Repository navigation
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
When a hoisted package is reached through two override scopes that say the same thing,
findSpecificOverrideSetfalls back to the scopes' common ancestor (added in #9110). The ancestor doesn't carry the nested rule, so the package's own dependencies are no longer overridden.In the reproduction from #10137,
node_modules/minimatchis reached fromtshy(edge override setroot > minimatch) and fromglob(root > tshy > sync-content > glob > minimatch). Both carrybrace-expansion: ^5.0.12, but the node ends up with the root set, so itsbrace-expansionedge keeps the plain^5.0.8spec,brace-expansion@5.0.9counts as valid,npm ciaccepts the stale lockfile andnpm installdoesn't fix it.This change keeps the common-ancestor fallback, with one exception: when both sets are the same rule (same key, value and nested rules) and one of them sits directly under the common ancestor, it uses that one. That set adds only the rules both paths agree on to the ancestor's ruleset, so nothing from one scope leaks into the other. Scoped copies that are both deeper (for example
a > minimatchandb > minimatch) still fall back to the ancestor, as before.Testing
test/override-set.jscases: the same rule in two scopes keeps the top-level one in either order, and itsbrace-expansionrule still applies. A rule whose nested overrides differ, and two scoped copies, still return the common ancestor. Without the fix, the first case fails (it returns the root andgetEdgeRulegives no^5.0.12).test/node.jscase that builds the tree from the issue: minimatch is reached from tshy and from glob, and itsbrace-expansionedge spec is^5.0.12and invalid against 5.0.9. Without the fix this fails withedge.spec^5.0.8.npx tapinworkspaces/arborist(full suite, Node 24.22.0): all passing, 100% coverage.npx eslinton the changed files is clean.node bin/npm-cli.js):npm cinow fails withInvalid: lock file's brace-expansion@5.0.9 does not satisfy brace-expansion@5.0.12, andnpm installupdates the tree tobrace-expansion@5.0.12. Onlatest(12.2.1) without the change, the same script letnpm ciinstall 5.0.9.npm testfor the whole CLI.AI assistance: I worked through this change with an AI coding assistant. I reviewed the code, the tests and this description, and ran the commands listed above.
References
Fixes #10137