Add support for non-standard package.json locations in npm tarballs #78
Conversation
artifactory/commands/npm/publish.go
Outdated
| return err | ||
| } | ||
|
|
||
| // findBestPackageJson scans the tarball and returns the best package.json file based on priority |
There was a problem hiding this comment.
a little more description on what exactly is best package and how it is finding will help readers later
artifactory/commands/npm/publish.go
Outdated
| priority := calculatePackageJsonPriority(hdr.Name) | ||
| candidate := &packageJsonCandidate{ | ||
| path: hdr.Name, | ||
| priority: priority, | ||
| content: content, | ||
| } |
There was a problem hiding this comment.
why do we need to have priority and search for? is it because of consideration that there can be multiple package.json ?
- Can we pack a npm package with multiple package.json? if so then npm should get confused with what package.json to use.
- Cant we limit the solution finding first package.json and returning it without adding any priority? Asking this since to pack you have to be in root folder of package and run npm pack this will have package.json as package/package.json but packing can also be done using other tools which might be reason for having something like node/package.json even in this scenario search in root folder for package.json should be good right?
There was a problem hiding this comment.
Yes, npm packages can absolutely contain multiple package.json files for several legitimate reasons.
One of the example can be a
- Monorepo structures where Packages with nested components each having their own
package.json. - Some packages include vendored dependencies with their own
package.json
The key issue is that the order of files in a tar archive is not guaranteed to be consistent. Simply taking the first package.json we find would be problematic some build tools might place package.json in different locations than the npm standard
How npm handles this situation
It has expectations about where package.json should be located but can handle non-standard structures. When publishing, npm normalizes the package structure, but when consuming packages, it needs to be flexible.
Can't we just use the first package.json we find?
What I felt was simply using the first package.json found would be problematic for several reasons:
- The order of files in a tarball is not guaranteed or standardized - it depends on how the archive was created
- A less relevant package.json (like from an example directory) might appear first in the tarball
- This would make our behavior inconsistent and potentially different from npm's own behavior
The current approach with package/package.json as highest priority matches npm's conventions, while providing fallbacks for non-standard packages. Using just "the first one found" would be unpredictable across different package structures.
We can discuss this tomorrow. If we want, we can go ahead with that approach. The implementation is what I had initially thought.
d866afd to
ff8275b
Compare
NPM Publish: Improve package.json Location Handling
Description
This PR improves how the npm publish command handles finding and reading package.json files in npm packages. Instead of using a complex priority-based approach, it implements a simpler and more predictable strategy that maintains compatibility with both standard and non-standard npm package structures.
Package.json Search Strategy
The command now uses a three-step strategy to locate package.json:
First checks standard locations in this order:
package/package.json(standard npm package location)node-pkg/package.json)If not found in standard locations, performs a second pass to scan the entire package for any
package.jsonfile, using the first one found.Only throws an error if no
package.jsonis found anywhere in the package.Changes
Testing Done
package/package.json)Impact
This change simplifies the code and makes the behavior more predictable while maintaining compatibility with different package structures. It should not break any existing functionality but makes the package.json selection process more transparent and easier to understand.