fix: process deps for all link nodes

lib/arborist/build-ideal-tree.js

@@ -790,6 +790,11 @@ This is a one-time fix-up, please be patient...
     }
     await Promise.all(promises)
 
+    for (const { to } of node.edgesOut.values()) {
+      if (to && to.isLink)
+        this[_linkNodes].add(to)
+    }
+
     return this[_buildDepStep]()
   }
 
@@ -1539,9 +1544,14 @@ This is a one-time fix-up, please be patient...
         }
       }
 
-      // didn't find a parent for it, but we're filling in external
-      // link targets, so go ahead and process it.
-      if (this[_follow] && !link.target.parent && !link.target.fsParent) {
+      // didn't find a parent for it or it has not been seen yet
+      // so go ahead and process it.
+      const unseenLink = (link.target.parent || link.target.fsParent)
+        && !this[_depsSeen].has(link.target)
+      if (this[_follow]
+        && !link.target.parent
+        && !link.target.fsParent
+        || unseenLink) {
         this.addTracker('idealTree', link.target.name, link.target.location)
         this[_depsQueue].push(link.target)
       }

lib/arborist/load-virtual.js

@@ -185,29 +185,20 @@ module.exports = cls => class VirtualLoader extends cls {
   // links is the set of metadata, and nodes is the map of non-Link nodes
   // Set the targets to nodes in the set, if we have them (we might not)
   async [resolveLinks] (links, nodes) {
-    // now we've loaded the root, and all real nodes
-    // link up the links
-    const {meta} = this.virtualTree
-    const {loadedFromDisk, originalLockfileVersion} = meta
-    const oldLockfile = loadedFromDisk && !(originalLockfileVersion >= 2)
-
     for (const [location, meta] of links.entries()) {
       const targetPath = resolve(this.path, meta.resolved)
       const targetLoc = relpath(this.path, targetPath)
       const target = nodes.get(targetLoc)
       const link = this[loadLink](location, targetLoc, target, meta)
       nodes.set(location, link)
       nodes.set(targetLoc, link.target)
-      // legacy shrinkwraps do not store all the info we need for the target.
-      // if we're loading from disk, and have a link in place, we need to
-      // look in that actual folder (or at least try to) in order to get
-      // the dependencies of the link target and load it properly.
-      if (oldLockfile) {
-        const pj = link.realpath + '/package.json'
-        const pkg = await rpj(pj).catch(() => null)
-        if (pkg)
-          link.target.package = pkg
-      }
+
+      // we always need to read the package.json for link targets
+      // because they can be changed by the local user
+      const pj = link.realpath + '/package.json'
+      const pkg = await rpj(pj).catch(() => null)
+      if (pkg)
+        link.target.package = pkg
     }
   }
 

tap-snapshots/test-arborist-build-ideal-tree.js-TAP.test.js

@@ -32851,7 +32851,7 @@ Node {
       "name": "aaaaaa",
       "resolved": "file:abbrev",
       "target": Object {
-        "name": "aaaaaa",
+        "name": "zzzzzz",
         "parent": null,
       },
     },
@@ -33058,7 +33058,7 @@ Node {
       "name": "aaaaaa",
       "resolved": "file:abbrev",
       "target": Object {
-        "name": "aaaaaa",
+        "name": "zzzzzz",
         "parent": null,
       },
     },

tap-snapshots/test-arborist-reify.js-TAP.test.js

@@ -45654,6 +45654,50 @@ Node {
 }
 `
 
+exports[`test/arborist/reify.js TAP workspaces add new workspaces dep > should update package-lock with new added dep 1`] = `
+Object {
+  "dependencies": Object {
+    "a": Object {
+      "requires": Object {
+        "abbrev": "^1.1.1",
+      },
+      "version": "file:a",
+    },
+    "abbrev": Object {
+      "integrity": "sha512-nne9/IiQ/hzIhY6pdDnbBtz7DjPTKrY00P/zvPSm5pOFkl6xuGrGnXn/VtTNNfNtAfZ9/1RtehkszU9qcTii0Q==",
+      "resolved": "https://registry.npmjs.org/abbrev/-/abbrev-1.1.1.tgz",
+      "version": "1.1.1",
+    },
+  },
+  "lockfileVersion": 2,
+  "name": "workspaces-add-new-dep",
+  "packages": Object {
+    "": Object {
+      "name": "workspaces-add-new-dep",
+      "workspaces": Array [
+        "a",
+      ],
+    },
+    "a": Object {
+      "dependencies": Object {
+        "abbrev": "^1.1.1",
+      },
+      "version": "1.0.0",
+    },
+    "node_modules/a": Object {
+      "link": true,
+      "resolved": "a",
+    },
+    "node_modules/abbrev": Object {
+      "integrity": "sha512-nne9/IiQ/hzIhY6pdDnbBtz7DjPTKrY00P/zvPSm5pOFkl6xuGrGnXn/VtTNNfNtAfZ9/1RtehkszU9qcTii0Q==",
+      "resolved": "https://registry.npmjs.org/abbrev/-/abbrev-1.1.1.tgz",
+      "version": "1.1.1",
+    },
+  },
+  "requires": true,
+}
+`
+
 exports[`test/arborist/reify.js TAP workspaces reify from an actual loaded workspace env > should not clean up entire nm folder for no reason 1`] = `
 Node {
   "children": Map {

test/arborist/reify.js

@@ -1057,6 +1057,14 @@ t.test('workspaces', t => {
       'should not clean up entire nm folder for no reason'
     ))
 
+  t.test('add new workspaces dep', t => {
+    const path = fixture(t, 'workspaces-add-new-dep')
+    reify(path).then(() => {
+      t.matchSnapshot(require(path + '/package-lock.json'), 'should update package-lock with new added dep')
+      t.end()
+    })
+  })
+
   t.end()
 })
 

test/fixtures/reify-cases/workspaces-add-new-dep.js

@@ -0,0 +1,67 @@
+// generated from test/fixtures/workspaces-add-new-dep
+module.exports = t => {
+  const path = t.testdir({
+  "a": {
+    "package.json": JSON.stringify({
+      "name": "a",
+      "version": "1.0.0",
+      "dependencies": {
+        "abbrev": "^1.1.1"
+      }
+    })
+  },
+  "node_modules": {
+    ".package-lock.json": JSON.stringify({
+      "name": "workspaces-add-new-dep",
+      "lockfileVersion": 2,
+      "requires": true,
+      "packages": {
+        "a": {
+          "version": "1.0.0"
+        },
+        "node_modules/a": {
+          "resolved": "a",
+          "link": true
+        }
+      }
+    }),
+    "a": t.fixture('symlink', "../a")
+  },
+  "package-lock.json": JSON.stringify({
+    "name": "workspaces-add-new-dep",
+    "lockfileVersion": 2,
+    "requires": true,
+    "packages": {
+      "": {
+        "workspaces": [
+          "a"
+        ]
+      },
+      "a": {
+        "version": "1.0.0"
+      },
+      "node_modules/a": {
+        "resolved": "a",
+        "link": true
+      }
+    },
+    "dependencies": {
+      "a": {
+        "version": "file:a"
+      }
+    }
+  }),
+  "package.json": JSON.stringify({
+    "name": "workspaces-add-new-dep",
+    "workspaces": [
+      "a"
+    ]
+  })
+})
+  const {utimesSync} = require('fs')
+  const n = Date.now()
+  const {resolve} = require('path')
+
+  utimesSync(resolve(path, "node_modules/.package-lock.json"), new Date(n), new Date(n))
+  return path
+}

test/fixtures/workspaces-add-new-dep/a/package.json

@@ -0,0 +1,8 @@
+{
+  "name": "a",
+  "version": "1.0.0",
+  "//": "abbrev is a new dep, not yet installed:",
+  "dependencies": {
+    "abbrev": "^1.1.1"
+  }
+}

test/fixtures/workspaces-add-new-dep/package.json

@@ -0,0 +1,6 @@
+{
+  "name": "workspaces-add-new-dep",
+  "workspaces": [
+    "a"
+  ]
+}