Skip to content

fix(arborist): resolve sibling override sets via common ancestor#9110

Merged
wraithgar merged 2 commits intonpm:latestfrom
manzoorwanijk:fix/override-sibling-sets-silent-exit
Mar 16, 2026
Merged

fix(arborist): resolve sibling override sets via common ancestor#9110
wraithgar merged 2 commits intonpm:latestfrom
manzoorwanijk:fix/override-sibling-sets-silent-exit

Conversation

@manzoorwanijk
Copy link
Contributor

@manzoorwanijk manzoorwanijk commented Mar 12, 2026

Two bugs combine to cause npm install to silently exit 1 in large monorepos when multiple packages are listed in overrides and share a transitive dependency.

Bug 1 — Sibling override sets treated as conflicting: findSpecificOverrideSet walks parent chains to determine if one override set contains the other. Sibling sets (e.g. the react and react-dom children of the root override set) are never ancestors of each other, so it returned undefined. This left shared transitive deps like loose-envify with an inconsistent override state, eventually causing an ERESOLVE during buildDeps. Fixed by checking haveConflictingRules (which already existed but was not used in this path) and falling back to the nearest common ancestor when the rules are compatible.

Bug 2 — ERESOLVE report crashes with infinite depth: explain-eresolve.js generates the full report file with depth=Infinity. In large trees (especially linked strategy), cycles in the dependency graph (e.g. store nodes referencing each other) cause explainFrom to recurse infinitely, hitting RangeError: Invalid string length. That error is thrown inside the error formatter, which swallows the original ERESOLVE — so you get exit 1 with no message. Fixed by adding cycle detection in explain-dep.js using stack-based seen tracking (add on enter, delete on leave) so diamond dependencies still work but cycles are broken. Also capped report depth to 16. (reverted — depth=Infinity is intentional for the full report file)

Found while testing overrides in WordPress/gutenberg#75814.

References

Fixes #9109

Sibling override sets (e.g. react + react-dom children of root) were treated as conflicting because findSpecificOverrideSet only checked ancestor relationships. Now checks haveConflictingRules and falls back to the nearest common ancestor when rules are compatible.

Also caps ERESOLVE report depth to 16 and adds cycle detection in explain-dep to prevent RangeError: Invalid string length in large trees.
@manzoorwanijk manzoorwanijk force-pushed the fix/override-sibling-sets-silent-exit branch from 4166f55 to 9e08077 Compare March 12, 2026 20:02
@manzoorwanijk manzoorwanijk marked this pull request as ready for review March 12, 2026 20:02
@manzoorwanijk manzoorwanijk requested a review from a team as a code owner March 12, 2026 20:02

return {
explanation: `${explain(expl, chalk, 4)}\n\n${fix}`,
file: `# npm resolution error report\n\n${explain(expl, noColorChalk, Infinity)}\n\n${fix}`,
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is done on purpose so that the eresolve-report.txt has the full json output. See the comment on line 6.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You're right, reverted that change. The depth=Infinity is preserved.

The underlying issue was that in large linked-strategy trees, cycles in the dependency graph (e.g. store nodes referencing each other) caused explainFrom to recurse infinitely, eventually hitting RangeError: Invalid string length.

The fix is now in explain-dep.js instead - added stack-based cycle detection in explainFrom so it breaks out of recursive cycles while still fully expanding diamond dependencies (same node reached via different paths). This way, the full report is still generated as intended, just without infinite recursion.

@wraithgar wraithgar merged commit 21ea382 into npm:latest Mar 16, 2026
34 checks passed
@github-actions github-actions bot mentioned this pull request Mar 16, 2026
@manzoorwanijk manzoorwanijk deleted the fix/override-sibling-sets-silent-exit branch March 16, 2026 23:30
manzoorwanijk added a commit to manzoorwanijk/npm-cli that referenced this pull request Mar 17, 2026
wraithgar pushed a commit that referenced this pull request Mar 17, 2026
…fixes (#9120)

This is a cherry-pick of 

- 8e0a731 - #9108 
- 51365b1 - #9107
- 21ea382 - #9110 

and cf34e6f (the changes needed for
21ea382 in v10 branch)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[BUG] Overrides with shared transitive deps cause silent exit 1

2 participants