From 47cc95d9ffb37fc8ff62a1d5554eab16d303aa43 Mon Sep 17 00:00:00 2001 From: nlf Date: Mon, 1 Aug 2022 11:11:55 -0700 Subject: [PATCH] fix(arborist): use the sourceReference root rather than the node root for overrides (#5227) when we examine override references, if we look at only `this.from.root.package` the root could actually be a virtual one. in order to ensure we resolve references from the real root, we instead need to look at `this.from.sourceReference.root.package` to get the correct value. closes #4395 --- workspaces/arborist/lib/edge.js | 6 +- workspaces/arborist/test/edge.js | 141 +++++++++++++++++++++++++++++++ 2 files changed, 146 insertions(+), 1 deletion(-) diff --git a/workspaces/arborist/lib/edge.js b/workspaces/arborist/lib/edge.js index a04404f226563..5b248b9166208 100644 --- a/workspaces/arborist/lib/edge.js +++ b/workspaces/arborist/lib/edge.js @@ -169,7 +169,11 @@ class Edge { if (this.overrides && this.overrides.value && this.overrides.name === this.name) { if (this.overrides.value.startsWith('$')) { const ref = this.overrides.value.slice(1) - const pkg = this.from.root.package + // we may be a virtual root, if we are we want to resolve reference overrides + // from the real root, not the virtual one + const pkg = this.from.sourceReference + ? this.from.sourceReference.root.package + : this.from.root.package const overrideSpec = (pkg.devDependencies && pkg.devDependencies[ref]) || (pkg.optionalDependencies && pkg.optionalDependencies[ref]) || (pkg.dependencies && pkg.dependencies[ref]) || diff --git a/workspaces/arborist/test/edge.js b/workspaces/arborist/test/edge.js index 8bd12bbc1da91..9d1352351741e 100644 --- a/workspaces/arborist/test/edge.js +++ b/workspaces/arborist/test/edge.js @@ -809,6 +809,147 @@ t.same(bundledEdge.explain(), { from: bundleParent.explain(), }, 'bundled edge.explain as expected') +t.test('override references find the correct root', (t) => { + const overrides = new OverrideSet({ + overrides: { + foo: '$foo', + }, + }) + + const root = { + name: 'root', + packageName: 'root', + edgesOut: new Map(), + edgesIn: new Set(), + explain: () => 'root node explanation', + package: { + name: 'root', + version: '1.2.3', + dependencies: { + foo: '^1.0.0', + }, + overrides: { + foo: '$foo', + }, + }, + get version () { + return this.package.version + }, + isTop: true, + parent: null, + overrides, + resolve (n) { + return n === 'foo' ? foo : null + }, + addEdgeOut (edge) { + this.edgesOut.set(edge.name, edge) + }, + addEdgeIn (edge) { + this.edgesIn.add(edge) + }, + } + + const foo = { + name: 'foo', + packageName: 'foo', + edgesOut: new Map(), + edgesIn: new Set(), + explain: () => 'foo node explanation', + package: { + name: 'foo', + version: '1.2.3', + dependencies: {}, + }, + get version () { + return this.package.version + }, + parent: root, + root: root, + resolve (n) { + return n === 'bar' ? bar : this.parent.resolve(n) + }, + addEdgeOut (edge) { + this.edgesOut.set(edge.name, edge) + }, + addEdgeIn (edge) { + this.edgesIn.add(edge) + }, + } + foo.overrides = overrides.getNodeRule(foo) + + const bar = { + name: 'bar', + packageName: 'bar', + edgesOut: new Map(), + edgesIn: new Set(), + explain: () => 'bar node explanation', + package: { + name: 'bar', + version: '1.2.3', + dependencies: { + foo: '^2.0.0', + }, + }, + get version () { + return this.package.version + }, + parent: foo, + root: root, + resolve (n) { + return this.parent.resolve(n) + }, + addEdgeOut (edge) { + this.edgesOut.set(edge.name, edge) + }, + addEdgeIn (edge) { + this.edgesIn.add(edge) + }, + } + bar.overrides = foo.overrides.getNodeRule(bar) + + const virtualBar = { + name: 'bar', + packageName: 'bar', + edgesOut: new Map(), + edgesIn: new Set(), + explain: () => 'bar node explanation', + package: { + name: 'bar', + version: '1.2.3', + dependencies: { + foo: '^2.0.0', + }, + }, + parent: null, + root: null, + sourceReference: bar, + get version () { + return this.package.version + }, + resolve (n) { + return bar.resolve(n) + }, + addEdgeOut (edge) { + this.edgesOut.set(edge.name, edge) + }, + addEdgeIn (edge) { + this.edgesIn.add(edge) + }, + } + virtualBar.overrides = overrides + + const edge = new Edge({ + from: virtualBar, + type: 'prod', + spec: '^2.0.0', + name: 'foo', + overrides: overrides.getEdgeRule({ name: 'foo', spec: '^2.0.0' }), + }) + + t.ok(edge.valid, 'edge is valid') + t.end() +}) + t.test('shrinkwrapped and bundled deps are not overridden and remain valid', (t) => { const overrides = new OverrideSet({ overrides: {