fix(diff): walk target children if root is a link

This expands the behavior that was previously only done for global
roots, but should really be done for _any_ root dir that is not accessed
via its realpath.

Fixes: npm/cli#3431

PR-URL: #294
Credit: @isaacs
Close: #294
Reviewed-by: @nlf

lib/diff.js

@@ -145,9 +145,9 @@ const allChildren = node => {
   if (!node)
     return new Map()
 
-  // if the node is a global root, and also a link, then what we really
+  // if the node is root, and also a link, then what we really
   // want is to traverse the target's children
-  if (node.global && node.isRoot && node.isLink)
+  if (node.isRoot && node.isLink)
     return allChildren(node.target)
 
   const kids = new Map()

tap-snapshots/test/diff.js.test.cjs

@@ -573,7 +573,47 @@ Diff {
 }
 `
 
-exports[`test/diff.js TAP when a global root is a link, traverse the target children > correctly removes the child node 1`] = `
+exports[`test/diff.js TAP when a root is a link, traverse the target children global=false > correctly removes the child node 1`] = `
+Diff {
+  "action": null,
+  "actual": Link {
+    "name": "path",
+    "path": "/some/linked/path",
+    "integrity": null,
+  },
+  "ideal": Node {
+    "name": "path",
+    "path": "/some/real/path",
+    "integrity": null,
+  },
+  "leaves": Array [
+    "/some/real/path/node_modules/child",
+  ],
+  "unchanged": Array [],
+  "removed": Array [
+    "/some/real/path/node_modules/child",
+  ],
+  "children": Array [
+    Diff {
+      "action": "REMOVE",
+      "actual": Node {
+        "name": "child",
+        "path": "/some/real/path/node_modules/child",
+        "integrity": null,
+      },
+      "ideal": undefined,
+      "leaves": Array [
+        "/some/real/path/node_modules/child",
+      ],
+      "unchanged": Array [],
+      "removed": Array [],
+      "children": Array [],
+    },
+  ],
+}
+`
+
+exports[`test/diff.js TAP when a root is a link, traverse the target children global=true > correctly removes the child node 1`] = `
 Diff {
   "action": null,
   "actual": Link {

test/diff.js

@@ -194,35 +194,40 @@ t.equal(d.parent, null, 'root has no parent')
 t.equal([...d.children][0].parent, d, 'parent of root child is root')
 t.equal(d.action, null, 'root has no action')
 
-t.test('when a global root is a link, traverse the target children', async (t) => {
-  const actual = new Link({
-    global: true,
-    path: '/some/linked/path',
-    realpath: '/some/real/path',
-  })
-
-  actual.target = new Node({
-    global: true,
-    path: '/some/real/path',
-    pkg: {},
-    children: [{
-      name: 'child',
-      pkg: {
-        name: 'child',
-        version: '1.2.3',
-      },
-    }],
-  })
-
-  const ideal = new Node({
-    path: '/some/real/path',
-    realpath: '/some/real/path',
-    children: [],
-  })
-
-  const diff = Diff.calculate({ actual, ideal })
-  t.matchSnapshot(diff, 'correctly removes the child node')
-  t.equal(diff.removed.length, 1, 'identifies the need to remove the child')
+t.test('when a root is a link, traverse the target children', t => {
+  t.plan(2)
+  for (const global of [true, false]) {
+    t.test(`global=${global}`, async t => {
+      const actual = new Link({
+        global,
+        path: '/some/linked/path',
+        realpath: '/some/real/path',
+      })
+
+      actual.target = new Node({
+        global,
+        path: '/some/real/path',
+        pkg: {},
+        children: [{
+          name: 'child',
+          pkg: {
+            name: 'child',
+            version: '1.2.3',
+          },
+        }],
+      })
+
+      const ideal = new Node({
+        path: '/some/real/path',
+        realpath: '/some/real/path',
+        children: [],
+      })
+
+      const diff = Diff.calculate({ actual, ideal })
+      t.matchSnapshot(diff, 'correctly removes the child node')
+      t.equal(diff.removed.length, 1, 'identifies the need to remove the child')
+    })
+  }
 })
 
 t.test('filtered diff', async t => {