fix(arborist): when reloading an edge, also refresh overrides

this fixes some scenarios where overrides were not being properly
applied, especially those where a node_modules or package-lock.json
already exists

Author: nlf

workspaces/arborist/lib/edge.js

@@ -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]) {

workspaces/arborist/lib/node.js

@@ -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)

workspaces/arborist/test/edge.js

@@ -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)
 
@@ -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')
@@ -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',
@@ -494,6 +504,7 @@ const overrides = new OverrideSet({
     c: '1.x',
   },
 })
+a.overrides = overrides
 const overriddenEdge = new Edge({
   from: a,
   type: 'prod',
@@ -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,

workspaces/arborist/test/node.js

@@ -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,