Skip to content

Commit

Permalink
fix(arborist): when reloading an edge, also refresh overrides
Browse files Browse the repository at this point in the history
this fixes some scenarios where overrides were not being properly
applied, especially those where a node_modules or package-lock.json
already exists
  • Loading branch information
nlf authored and fritzy committed Apr 12, 2022
1 parent 0c76487 commit 4d676e3
Show file tree
Hide file tree
Showing 4 changed files with 101 additions and 25 deletions.
5 changes: 5 additions & 0 deletions workspaces/arborist/lib/edge.js
Original file line number Diff line number Diff line change
Expand Up @@ -215,6 +215,11 @@ class Edge {

reload (hard = false) {
this[_explanation] = null
if (this[_from].overrides) {
this.overrides = this[_from].overrides.getEdgeRule(this)
} else {
delete this.overrides
}
const newTo = this[_from].resolve(this.name)
if (newTo !== this[_to]) {
if (this[_to]) {
Expand Down
3 changes: 3 additions & 0 deletions workspaces/arborist/lib/node.js
Original file line number Diff line number Diff line change
Expand Up @@ -792,6 +792,9 @@ class Node {
target.root = root
}

if (!this.overrides && this.parent && this.parent.overrides) {
this.overrides = this.parent.overrides.getNodeRule(this)
}
// tree should always be valid upon root setter completion.
treeCheck(this)
treeCheck(root)
Expand Down
62 changes: 37 additions & 25 deletions workspaces/arborist/test/edge.js
Original file line number Diff line number Diff line change
Expand Up @@ -244,16 +244,19 @@ t.not(new Edge({
}).satisfiedBy(b), 'b does not satisfy edge for c')
reset(a)

const overrideSet = new OverrideSet({
overrides: {
c: '2.x',
},
})

a.overrides = overrideSet
t.matchSnapshot(new Edge({
from: a,
type: 'prod',
name: 'c',
spec: '1.x',
overrides: new OverrideSet({
overrides: {
c: '2.x',
},
}).getEdgeRule({ name: 'c', spec: '1.x' }),
overrides: overrideSet.getEdgeRule({ name: 'c', spec: '1.x' }),
}).toJSON(), 'printableEdge shows overrides')
reset(a)

Expand All @@ -262,11 +265,7 @@ const overriddenExplanation = new Edge({
type: 'prod',
name: 'c',
spec: '1.x',
overrides: new OverrideSet({
overrides: {
c: '2.x',
},
}).getEdgeRule({ name: 'c', spec: '1.x' }),
overrides: overrideSet.getEdgeRule({ name: 'c', spec: '1.x' }),
}).explain()
t.equal(overriddenExplanation.rawSpec, '1.x', 'rawSpec has original spec')
t.equal(overriddenExplanation.spec, '2.x', 'spec has override spec')
Expand All @@ -278,38 +277,49 @@ t.ok(new Edge({
type: 'prod',
name: 'c',
spec: '1.x',
overrides: new OverrideSet({
overrides: {
c: '2.x',
},
}).getEdgeRule({ name: 'c', spec: '1.x' }),
overrides: overrideSet.getEdgeRule({ name: 'c', spec: '1.x' }),
}).satisfiedBy(c), 'c@2 satisfies spec:1.x, override:2.x')
reset(a)

const overrideSetB = new OverrideSet({
overrides: {
b: '1.x',
},
})
a.overrides = overrideSetB
t.matchSnapshot(new Edge({
from: a,
type: 'prod',
name: 'c',
spec: '2.x',
overrides: new OverrideSet({
overrides: {
b: '1.x',
},
}).getEdgeRule({ name: 'c', spec: '2.x' }),
overrides: overrideSetB.getEdgeRule({ name: 'c', spec: '2.x' }),
}).toJSON(), 'printableEdge does not show non-applicable override')

t.ok(new Edge({
from: a,
type: 'prod',
name: 'c',
spec: '2.x',
overrides: new OverrideSet({
overrides: {
b: '1.x',
},
}).getEdgeRule({ name: 'c', spec: '2.x' }),
overrides: overrideSetB.getEdgeRule({ name: 'c', spec: '2.x' }),
}).satisfiedBy(c), 'c@2 satisfies spec:1.x, no matching override')
reset(a)
delete a.overrides

const overrideEdge = new Edge({
from: a,
type: 'prod',
name: 'c',
spec: '2.x',
})
t.notOk(overrideEdge.overrides, 'edge has no overrides')
a.overrides = new OverrideSet({ overrides: { b: '1.x' } })
t.notOk(overrideEdge.overrides, 'edge has no overrides')
overrideEdge.reload()
t.ok(overrideEdge.overrides, 'edge has overrides after reload')
delete a.overrides
overrideEdge.reload()
t.notOk(overrideEdge.overrides, 'edge has no overrides after reload')
reset(a)

const referenceTop = {
name: 'referenceTop',
Expand Down Expand Up @@ -494,6 +504,7 @@ const overrides = new OverrideSet({
c: '1.x',
},
})
a.overrides = overrides
const overriddenEdge = new Edge({
from: a,
type: 'prod',
Expand All @@ -504,6 +515,7 @@ const overriddenEdge = new Edge({
t.equal(overriddenEdge.spec, '1.x', 'override spec takes priority')
t.equal(overriddenEdge.rawSpec, '2.x', 'rawSpec holds original spec')
reset(a)
delete a.overrides

const old = new Edge({
from: a,
Expand Down
56 changes: 56 additions & 0 deletions workspaces/arborist/test/node.js
Original file line number Diff line number Diff line change
Expand Up @@ -2781,6 +2781,62 @@ t.test('overrides', (t) => {
t.end()
})

t.test('setting root replaces overrides', async (t) => {
const root = new Node({
path: '/some/path',
loadOverrides: true,
pkg: {
name: 'root',
version: '1.0.0',
dependencies: {
foo: '^1.0.0',
},
overrides: {
bar: '^2.0.0',
},
},
})

const foo = new Node({
path: '/some/path/node_modules/foo',
pkg: {
name: 'foo',
version: '1.0.0',
dependencies: {
bar: '^1.0.0',
},
},
})

const bar = new Node({
path: '/some/path/node_modules/bar',
pkg: {
name: 'bar',
version: '2.0.0',
},
})

t.ok(root.overrides, 'root has overrides')
t.notOk(foo.overrides, 'foo does not have overrides')
t.notOk(bar.overrides, 'bar does not have overrides')
t.notOk(root.edgesOut.get('foo').valid, 'foo edge is not valid')
t.notOk(foo.edgesOut.get('bar').valid, 'bar edge is not valid')

// we add bar to the root first, this is deliberate so that we don't have a simple
// linear inheritance. we'll add foo later and make sure that both edges and nodes
// become valid after that

bar.root = root
t.ok(bar.overrides, 'bar now has overrides')
t.notOk(foo.edgesOut.get('bar').valid, 'bar edge is not valid yet')

foo.root = root
t.ok(foo.overrides, 'foo now has overrides')
t.ok(root.edgesOut.get('foo').valid, 'foo edge is now valid')
t.ok(bar.overrides, 'bar still has overrides')
t.ok(foo.edgesOut.get('bar').valid, 'bar edge is now valid')
})

t.test('canReplaceWith requires the same overrides', async (t) => {
const original = new Node({
loadOverrides: true,
Expand Down

0 comments on commit 4d676e3

Please sign in to comment.