Skip to content

Commit 47cc95d

Browse files
authored
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
1 parent a6153cf commit 47cc95d

File tree

2 files changed

+146
-1
lines changed

2 files changed

+146
-1
lines changed

workspaces/arborist/lib/edge.js

+5-1
Original file line numberDiff line numberDiff line change
@@ -169,7 +169,11 @@ class Edge {
169169
if (this.overrides && this.overrides.value && this.overrides.name === this.name) {
170170
if (this.overrides.value.startsWith('$')) {
171171
const ref = this.overrides.value.slice(1)
172-
const pkg = this.from.root.package
172+
// we may be a virtual root, if we are we want to resolve reference overrides
173+
// from the real root, not the virtual one
174+
const pkg = this.from.sourceReference
175+
? this.from.sourceReference.root.package
176+
: this.from.root.package
173177
const overrideSpec = (pkg.devDependencies && pkg.devDependencies[ref]) ||
174178
(pkg.optionalDependencies && pkg.optionalDependencies[ref]) ||
175179
(pkg.dependencies && pkg.dependencies[ref]) ||

workspaces/arborist/test/edge.js

+141
Original file line numberDiff line numberDiff line change
@@ -809,6 +809,147 @@ t.same(bundledEdge.explain(), {
809809
from: bundleParent.explain(),
810810
}, 'bundled edge.explain as expected')
811811

812+
t.test('override references find the correct root', (t) => {
813+
const overrides = new OverrideSet({
814+
overrides: {
815+
foo: '$foo',
816+
},
817+
})
818+
819+
const root = {
820+
name: 'root',
821+
packageName: 'root',
822+
edgesOut: new Map(),
823+
edgesIn: new Set(),
824+
explain: () => 'root node explanation',
825+
package: {
826+
name: 'root',
827+
version: '1.2.3',
828+
dependencies: {
829+
foo: '^1.0.0',
830+
},
831+
overrides: {
832+
foo: '$foo',
833+
},
834+
},
835+
get version () {
836+
return this.package.version
837+
},
838+
isTop: true,
839+
parent: null,
840+
overrides,
841+
resolve (n) {
842+
return n === 'foo' ? foo : null
843+
},
844+
addEdgeOut (edge) {
845+
this.edgesOut.set(edge.name, edge)
846+
},
847+
addEdgeIn (edge) {
848+
this.edgesIn.add(edge)
849+
},
850+
}
851+
852+
const foo = {
853+
name: 'foo',
854+
packageName: 'foo',
855+
edgesOut: new Map(),
856+
edgesIn: new Set(),
857+
explain: () => 'foo node explanation',
858+
package: {
859+
name: 'foo',
860+
version: '1.2.3',
861+
dependencies: {},
862+
},
863+
get version () {
864+
return this.package.version
865+
},
866+
parent: root,
867+
root: root,
868+
resolve (n) {
869+
return n === 'bar' ? bar : this.parent.resolve(n)
870+
},
871+
addEdgeOut (edge) {
872+
this.edgesOut.set(edge.name, edge)
873+
},
874+
addEdgeIn (edge) {
875+
this.edgesIn.add(edge)
876+
},
877+
}
878+
foo.overrides = overrides.getNodeRule(foo)
879+
880+
const bar = {
881+
name: 'bar',
882+
packageName: 'bar',
883+
edgesOut: new Map(),
884+
edgesIn: new Set(),
885+
explain: () => 'bar node explanation',
886+
package: {
887+
name: 'bar',
888+
version: '1.2.3',
889+
dependencies: {
890+
foo: '^2.0.0',
891+
},
892+
},
893+
get version () {
894+
return this.package.version
895+
},
896+
parent: foo,
897+
root: root,
898+
resolve (n) {
899+
return this.parent.resolve(n)
900+
},
901+
addEdgeOut (edge) {
902+
this.edgesOut.set(edge.name, edge)
903+
},
904+
addEdgeIn (edge) {
905+
this.edgesIn.add(edge)
906+
},
907+
}
908+
bar.overrides = foo.overrides.getNodeRule(bar)
909+
910+
const virtualBar = {
911+
name: 'bar',
912+
packageName: 'bar',
913+
edgesOut: new Map(),
914+
edgesIn: new Set(),
915+
explain: () => 'bar node explanation',
916+
package: {
917+
name: 'bar',
918+
version: '1.2.3',
919+
dependencies: {
920+
foo: '^2.0.0',
921+
},
922+
},
923+
parent: null,
924+
root: null,
925+
sourceReference: bar,
926+
get version () {
927+
return this.package.version
928+
},
929+
resolve (n) {
930+
return bar.resolve(n)
931+
},
932+
addEdgeOut (edge) {
933+
this.edgesOut.set(edge.name, edge)
934+
},
935+
addEdgeIn (edge) {
936+
this.edgesIn.add(edge)
937+
},
938+
}
939+
virtualBar.overrides = overrides
940+
941+
const edge = new Edge({
942+
from: virtualBar,
943+
type: 'prod',
944+
spec: '^2.0.0',
945+
name: 'foo',
946+
overrides: overrides.getEdgeRule({ name: 'foo', spec: '^2.0.0' }),
947+
})
948+
949+
t.ok(edge.valid, 'edge is valid')
950+
t.end()
951+
})
952+
812953
t.test('shrinkwrapped and bundled deps are not overridden and remain valid', (t) => {
813954
const overrides = new OverrideSet({
814955
overrides: {

0 commit comments

Comments
 (0)