From 891453129eeca1a06dc8ee84bc60cdc23b201c4c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Caleb=20=E3=83=84=20Everett?= Date: Tue, 14 Apr 2026 16:08:08 -0700 Subject: [PATCH] fix(arborist): fix infinite loop with bundledDependencies and overrides Fixes https://github.com/npm/cli/issues/9227 `npm install` hangs when a project uses `bundledDependencies` and `overrides` targeting a transitive dep shared by multiple bundled deps. In `edge.js` `satisfiedBy()`, the `inBundle` check (added in #4963) uses `rawSpec` for bundled nodes to prevent overrides from applying to pre-resolved deps inside a dependency's tarball. However, `inBundle` is also true for deps the root itself will bundle - these are freshly resolved from the registry and overrides should apply. The override was always applied at placement time (correct version installed), but the edge stayed invalid because `satisfiedBy` checked `rawSpec`. Two bundled deps sharing the overridden transitive dep would endlessly re-queue each other via REPLACE. The fix changes `inBundle` to `inDepBundle`, which is only true when the bundler is a non-root package. This preserves the #4963 behavior for deps pre-resolved inside a dependency's bundle/shrinkwrap while allowing the root's overrides to work. Note: it is unclear whether overrides _should_ be applied to deps that will be bundled or shrinkwrapped. The comment says that we explicitly don't, but I can't find supporting docs, and the existing behavior is that overrides are applied to dependencies that will be bundled/shrinkwrapped. I added tests asserting that behavior. These new tests passed without the change: - overrides do not apply inside a dependency that bundles - node bundled inside a dependency uses rawSpec - node inside a shrinkwrap uses rawSpec These new tests failed, they produced the same tree, but the edges were marked invalid: - node bundled by root uses overridden spec - overrides apply to deps the root will bundle and edges are valid This test hung forever: - does not infinite loop In both cases overrides that are 'baked into' dependnecies appear as 'invalid'. This happens because the root package doesn't read the bundler's overrides, and doesn't know why the shrinkwrap/bundle included the out-of-spec version. This commit doesn't affect that behavior. --- workspaces/arborist/lib/edge.js | 2 +- .../test/arborist/build-ideal-tree.js | 79 +++++++++++++++++++ workspaces/arborist/test/edge.js | 78 ++++++++++++++++++ 3 files changed, 158 insertions(+), 1 deletion(-) diff --git a/workspaces/arborist/lib/edge.js b/workspaces/arborist/lib/edge.js index 83a9c18cb6aac..8a5e060ec1427 100644 --- a/workspaces/arborist/lib/edge.js +++ b/workspaces/arborist/lib/edge.js @@ -110,7 +110,7 @@ class Edge { // NOTE: this condition means we explicitly do not support overriding // bundled or shrinkwrapped dependencies - if (node.hasShrinkwrap || node.inShrinkwrap || node.inBundle) { + if (node.hasShrinkwrap || node.inShrinkwrap || node.inDepBundle) { return depValid(node, this.rawSpec, this.#accept, this.#from) } diff --git a/workspaces/arborist/test/arborist/build-ideal-tree.js b/workspaces/arborist/test/arborist/build-ideal-tree.js index aa632426c03e1..7a87f3ea50826 100644 --- a/workspaces/arborist/test/arborist/build-ideal-tree.js +++ b/workspaces/arborist/test/arborist/build-ideal-tree.js @@ -4571,3 +4571,82 @@ t.test('skip invalid peerOptional edges in problemEdges when save=false (#8726)' 'shared stays at 1.1.0 - peerOptional mismatch is not treated as a problem') t.ok(tree.children.get('util'), 'util is in the tree') }) + +t.test('overrides with bundledDependencies', async t => { + t.test('does not infinite loop with bundledDependencies and overrides', async t => { + // https://github.com/npm/cli/issues/9227 + const registry = createRegistry(t, false) + + const bPacks = registry.packuments([ + { version: '1.0.0', dependencies: { bar: '1.0.0' } }, + ], 'b') + const cPacks = registry.packuments([ + { version: '1.0.0', dependencies: { bar: '1.0.0' } }, + ], 'c') + const barPacks = registry.packuments(['1.0.0', '2.0.0'], 'bar') + await registry.package({ manifest: registry.manifest({ name: 'b', packuments: bPacks }) }) + await registry.package({ manifest: registry.manifest({ name: 'c', packuments: cPacks }) }) + await registry.package({ manifest: registry.manifest({ name: 'bar', packuments: barPacks }), times: 2 }) + + const path = t.testdir({ + 'package.json': JSON.stringify({ + name: 'root', + dependencies: { b: '1.0.0', c: '1.0.0' }, + bundledDependencies: true, + overrides: { bar: '2.0.0' }, + }), + }) + + const tree = await buildIdeal(path) + t.equal(tree.children.get('bar').version, '2.0.0', 'override applied') + }) + + t.test('overrides apply to deps the root will bundle and edges are valid', async t => { + const registry = createRegistry(t, false) + + const fooPacks = registry.packuments([ + { version: '1.0.0', dependencies: { bar: '1.0.0' } }, + ], 'foo') + const barPacks = registry.packuments(['1.0.0', '2.0.0'], 'bar') + await registry.package({ manifest: registry.manifest({ name: 'foo', packuments: fooPacks }) }) + await registry.package({ manifest: registry.manifest({ name: 'bar', packuments: barPacks }) }) + + const path = t.testdir({ + 'package.json': JSON.stringify({ + name: 'root', + dependencies: { foo: '1.0.0' }, + bundledDependencies: ['foo'], + overrides: { bar: '2.0.0' }, + }), + }) + + const tree = await buildIdeal(path) + t.equal(tree.children.get('bar').version, '2.0.0', 'override installed correct version') + + const fooBarEdge = tree.edgesOut.get('foo').to.edgesOut.get('bar') + t.equal(fooBarEdge.valid, true, 'overridden edge is valid') + }) + + t.test('overrides do not apply inside a dependency that bundles', async t => { + const registry = createRegistry(t, false) + + const depPacks = registry.packuments([{ + version: '1.0.0', + dependencies: { bar: '1.0.0' }, + bundleDependencies: ['bar'], + }], 'dep') + await registry.package({ manifest: registry.manifest({ name: 'dep', packuments: depPacks }) }) + + const path = t.testdir({ + 'package.json': JSON.stringify({ + name: 'root', + dependencies: { dep: '1.0.0' }, + overrides: { bar: '2.0.0' }, + }), + }) + + const tree = await buildIdeal(path) + t.equal(tree.edgesOut.get('dep').valid, true, 'dep edge is valid') + t.notOk(tree.children.get('bar'), 'bar stays inside dep bundle') + }) +}) diff --git a/workspaces/arborist/test/edge.js b/workspaces/arborist/test/edge.js index e3f6204da9556..739adbbffe258 100644 --- a/workspaces/arborist/test/edge.js +++ b/workspaces/arborist/test/edge.js @@ -1332,3 +1332,81 @@ t.test('edge with overrides should not crash when target has no overrides', t => t.ok(edge.valid, 'edge should be valid') t.end() }) + +t.test('overrides and bundled/shrinkwrapped deps in satisfiedBy', t => { + const makeNode = (name, version, extras = {}) => ({ + name, + packageName: name, + version, + package: { name, version }, + edgesOut: new Map(), + edgesIn: new Set(), + explain: () => `${name}@${version}`, + isTop: false, + parent: top, + resolve: () => undefined, + addEdgeOut (edge) { + this.edgesOut.set(edge.name, edge) + }, + addEdgeIn (edge) { + this.edgesIn.add(edge) + }, + deleteEdgeIn (edge) { + this.edgesIn.delete(edge) + }, + hasShrinkwrap: false, + inShrinkwrap: false, + inBundle: false, + inDepBundle: false, + ...extras, + }) + + const overrides = new OverrideSet({ overrides: { bar: '2.x' } }) + a.overrides = overrides + + const makeOverriddenEdge = () => { + const edge = new Edge({ + from: a, + type: 'prod', + name: 'bar', + spec: '1.x', + overrides: overrides.getEdgeRule({ name: 'bar', spec: '1.x' }), + }) + reset(a) + a.overrides = overrides + return edge + } + + t.test('node bundled by root uses overridden spec', t => { + const edge = makeOverriddenEdge() + const node = makeNode('bar', '2.0.0', { inBundle: true, inDepBundle: false }) + t.ok(edge.satisfiedBy(node), 'bar@2.0.0 bundled by root satisfies override 2.x') + + const nodeOld = makeNode('bar', '1.0.0', { inBundle: true, inDepBundle: false }) + t.notOk(edge.satisfiedBy(nodeOld), 'bar@1.0.0 bundled by root does not satisfy override 2.x') + t.end() + }) + + t.test('node bundled inside a dependency uses rawSpec', t => { + const edge = makeOverriddenEdge() + const node = makeNode('bar', '1.0.0', { inBundle: true, inDepBundle: true }) + t.ok(edge.satisfiedBy(node), 'bar@1.0.0 in dep bundle satisfies rawSpec 1.x') + + const nodeNew = makeNode('bar', '2.0.0', { inBundle: true, inDepBundle: true }) + t.notOk(edge.satisfiedBy(nodeNew), 'bar@2.0.0 in dep bundle does not satisfy rawSpec 1.x') + t.end() + }) + + t.test('node inside a shrinkwrap uses rawSpec', t => { + const edge = makeOverriddenEdge() + const node = makeNode('bar', '1.0.0', { inShrinkwrap: true }) + t.ok(edge.satisfiedBy(node), 'bar@1.0.0 in shrinkwrap satisfies rawSpec 1.x') + + const nodeNew = makeNode('bar', '2.0.0', { inShrinkwrap: true }) + t.notOk(edge.satisfiedBy(nodeNew), 'bar@2.0.0 in shrinkwrap does not satisfy rawSpec 1.x') + t.end() + }) + + delete a.overrides + t.end() +})