diff --git a/workspaces/arborist/lib/arborist/build-ideal-tree.js b/workspaces/arborist/lib/arborist/build-ideal-tree.js index 121b20246731b..63157b9a550de 100644 --- a/workspaces/arborist/lib/arborist/build-ideal-tree.js +++ b/workspaces/arborist/lib/arborist/build-ideal-tree.js @@ -1595,6 +1595,10 @@ This is a one-time fix-up, please be patient... !link.target.parent && !link.target.fsParent || unseenLink) { + // Forward the link's overrides before its subtree resolves, so a root override reaches a transitive dep across the link boundary (npm/cli#9659). + if (link.overrides) { + link.target.updateOverridesEdgeInAdded(link.overrides) + } this.addTracker('idealTree', link.target.name, link.target.location) this.#depsQueue.push(link.target) } diff --git a/workspaces/arborist/lib/arborist/load-actual.js b/workspaces/arborist/lib/arborist/load-actual.js index bf73856ec2ddc..2158f6d32ae1a 100644 --- a/workspaces/arborist/lib/arborist/load-actual.js +++ b/workspaces/arborist/lib/arborist/load-actual.js @@ -70,6 +70,7 @@ module.exports = cls => class ActualLoader extends cls { // only reset root flags if we're not re-rooting, // otherwise leave as-is calcDepFlags(tree, !options.root) + this.#repropagateOverrides() this.actualTree = treeCheck(tree) return this.actualTree }) @@ -198,8 +199,6 @@ module.exports = cls => class ActualLoader extends cls { this.#transplant(root) - this.#repropagateOverrides() - if (global) { // need to depend on the children, or else all of them // will end up being flagged as extraneous, since the @@ -402,7 +401,7 @@ module.exports = cls => class ActualLoader extends cls { } } - // Re-forward overrides through links after the tree is complete, since a store Link may forward before its subtree resolves and miss a transitive match (npm/cli#9619). + // Re-forward overrides through links once all edges are resolved, since a Link may forward before its subtree resolves and miss a transitive match (npm/cli#9619, #9659). #repropagateOverrides () { if (!this.#actualTree.overrides) { return diff --git a/workspaces/arborist/test/arborist/build-ideal-tree.js b/workspaces/arborist/test/arborist/build-ideal-tree.js index 9bad026633823..6443f70fe58a8 100644 --- a/workspaces/arborist/test/arborist/build-ideal-tree.js +++ b/workspaces/arborist/test/arborist/build-ideal-tree.js @@ -3655,6 +3655,46 @@ t.test('overrides', async t => { t.equal(barEdge.to.version, '2.0.0') }) + t.test('overrides a nested dependency reached through a file: link — npm/cli#9659', async (t) => { + // A root override targeting a transitive dep must apply even when the path to that dep crosses a file:/workspace link boundary. + const registry = createRegistry(t, false) + const barPackuments = registry.packuments([ + { version: '1.0.0', dependencies: { baz: '^1.0.0' } }, + ], 'bar') + const barManifest = registry.manifest({ name: 'bar', packuments: barPackuments }) + const bazPackuments = registry.packuments(['1.0.0', '2.0.0'], 'baz') + const bazManifest = registry.manifest({ name: 'baz', packuments: bazPackuments }) + await registry.package({ manifest: barManifest }) + await registry.package({ manifest: bazManifest }) + + const path = t.testdir({ + 'package.json': JSON.stringify({ + name: 'root', + dependencies: { + a: 'file:./pkgs/a', + }, + overrides: { + baz: '2.0.0', + }, + }), + pkgs: { + a: { + 'package.json': JSON.stringify({ + name: 'a', + version: '1.0.0', + dependencies: { bar: '1.0.0' }, + }), + }, + }, + }) + + const tree = await buildIdeal(path) + + const barNode = tree.inventory.query('name', 'bar').values().next().value + const bazEdge = barNode.edgesOut.get('baz') + t.equal(bazEdge.to.version, '2.0.0', 'override applies across the file: link') + }) + t.test('does not override a nested dependency when parent spec does not match', async (t) => { const registry = createRegistry(t, false) const fooPackuments = registry.packuments([ diff --git a/workspaces/arborist/test/arborist/load-actual.js b/workspaces/arborist/test/arborist/load-actual.js index f699fa594f557..1c248bdba68ec 100644 --- a/workspaces/arborist/test/arborist/load-actual.js +++ b/workspaces/arborist/test/arborist/load-actual.js @@ -727,6 +727,72 @@ t.test('forwards a transitive override through a linked store link — npm/cli#9 t.equal(leafEdge.to.target.version, '2.0.0', 'edge resolves to the overridden package') }) +t.test('forwards a transitive override across a file: link boundary — npm/cli#9659', async t => { + // The override path crosses a file: link (root -> a) before entering the store chain (a -> b -> leaf). + // Loading from the hidden lockfile, the file link target's subtree resolves late, so override repropagation must run once all edges are resolved or `npm ls` reports the edge `invalid`. + const path = t.testdir({ + 'package.json': JSON.stringify({ + name: 'root', + version: '1.0.0', + dependencies: { a: 'file:./pkgs/a' }, + overrides: { leaf: '2.0.0' }, + }), + pkgs: { + a: { + 'package.json': JSON.stringify({ name: 'a', version: '1.0.0', dependencies: { b: '1.0.0' } }), + node_modules: { + b: t.fixture('symlink', '../../../node_modules/.store/b@1.0.0/node_modules/b'), + }, + }, + }, + node_modules: { + a: t.fixture('symlink', '../pkgs/a'), + '.store': { + 'b@1.0.0': { + node_modules: { + // leaf is declared ^1.0.0 but overridden to 2.0.0, outside that range + b: { 'package.json': JSON.stringify({ name: 'b', version: '1.0.0', dependencies: { leaf: '^1.0.0' } }) }, + leaf: t.fixture('symlink', '../../leaf@2.0.0/node_modules/leaf'), + }, + }, + 'leaf@2.0.0': { + node_modules: { + leaf: { 'package.json': JSON.stringify({ name: 'leaf', version: '2.0.0' }) }, + }, + }, + }, + '.package-lock.json': JSON.stringify({ + name: 'root', + lockfileVersion: 3, + requires: true, + packages: { + '': { name: 'root', version: '1.0.0', dependencies: { a: 'file:./pkgs/a' }, overrides: { leaf: '2.0.0' } }, + 'node_modules/a': { resolved: 'pkgs/a', link: true }, + 'pkgs/a': { version: '1.0.0', dependencies: { b: '1.0.0' } }, + 'pkgs/a/node_modules/b': { resolved: 'node_modules/.store/b@1.0.0/node_modules/b', link: true }, + 'node_modules/.store/b@1.0.0': {}, + 'node_modules/.store/b@1.0.0/node_modules/b': { version: '1.0.0', dependencies: { leaf: '^1.0.0' } }, + 'node_modules/.store/b@1.0.0/node_modules/leaf': { resolved: 'node_modules/.store/leaf@2.0.0/node_modules/leaf', link: true }, + 'node_modules/.store/leaf@2.0.0': {}, + 'node_modules/.store/leaf@2.0.0/node_modules/leaf': { version: '2.0.0' }, + }, + }), + }, + }) + // make the hidden lockfile the newest entry so loadActual loads from it + const hidden = resolve(path, 'node_modules/.package-lock.json') + const then = Date.now() + 10000 + fs.utimesSync(hidden, new Date(then), new Date(then)) + + const tree = await loadActual(path) + const b = tree.children.get('a').target.edgesOut.get('b').to.target + const leafEdge = b.edgesOut.get('leaf') + t.ok(leafEdge && !leafEdge.error, 'transitive overridden edge resolves without error') + t.ok(leafEdge.overrides, 'edge carries the override rule') + t.equal(leafEdge.spec, '2.0.0', 'edge spec is the overridden version') + t.equal(leafEdge.to.target.version, '2.0.0', 'edge resolves to the overridden package') +}) + t.test('store nodes do not load devDependencies as required edges', async t => { // A package in the linked store is structurally a tree top, so without the isInStore guard its devDependencies would load as required edges and surface as missing (e.g. npm sbom ESBOMPROBLEMS). const path = t.testdir({