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

2 files changed, 93 insertions, 25 deletions

diff --git a/workspaces/arborist/test/edge.js b/workspaces/arborist/test/edge.js
index c90b20278..2df989eb8 100644
--- a/workspaces/arborist/test/edge.js
+++ b/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,24 +277,22 @@ 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({
@@ -303,13 +300,26 @@ t.ok(new Edge({
   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,
diff --git a/workspaces/arborist/test/node.js b/workspaces/arborist/test/node.js
index ecdf72c22..80bc21559 100644
--- a/workspaces/arborist/test/node.js
+++ b/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,