Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions workspaces/arborist/lib/arborist/build-ideal-tree.js
Original file line number Diff line number Diff line change
Expand Up @@ -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)
}
Expand Down
5 changes: 2 additions & 3 deletions workspaces/arborist/lib/arborist/load-actual.js
Original file line number Diff line number Diff line change
Expand Up @@ -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
})
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand Down
40 changes: 40 additions & 0 deletions workspaces/arborist/test/arborist/build-ideal-tree.js
Original file line number Diff line number Diff line change
Expand Up @@ -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([
Expand Down
66 changes: 66 additions & 0 deletions workspaces/arborist/test/arborist/load-actual.js
Original file line number Diff line number Diff line change
Expand Up @@ -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({
Expand Down
Loading