fix(arborist): omit failed optional dependencies from installed deps,…

workspaces/arborist/lib/arborist/build-ideal-tree.js

@@ -809,7 +809,8 @@ This is a one-time fix-up, please be patient...
     const crackOpen = this.#complete &&
       node !== this.idealTree &&
       node.resolved &&
-      (hasBundle || hasShrinkwrap)
+      (hasBundle || hasShrinkwrap) &&
+      !node.ideallyInert
     if (crackOpen) {
       const Arborist = this.constructor
       const opt = { ...this.options }
@@ -1527,7 +1528,7 @@ This is a one-time fix-up, please be patient...
 
       const set = optionalSet(node)
       for (const node of set) {
-        node.parent = null
+        node.ideallyInert = true
       }
     }
   }
@@ -1548,6 +1549,7 @@ This is a one-time fix-up, please be patient...
           node.parent !== null
           && !node.isProjectRoot
           && !excludeNodes.has(node)
+          && !node.ideallyInert
         ) {
           this[_addNodeToTrashList](node)
         }

workspaces/arborist/lib/arborist/isolated-reifier.js

@@ -100,6 +100,9 @@ module.exports = cls => class IsolatedReifier extends cls {
   }
 
   async externalProxy (result, node) {
+    if (node.ideallyInert) {
+      return
+    }
     await this.assignCommonProperties(node, result)
     if (node.hasShrinkwrap) {
       const dir = join(

workspaces/arborist/lib/arborist/load-virtual.js

@@ -267,6 +267,7 @@ module.exports = cls => class VirtualLoader extends cls {
       integrity: sw.integrity,
       resolved: consistentResolve(sw.resolved, this.path, path),
       pkg: sw,
+      ideallyInert: sw.ideallyInert,
       hasShrinkwrap: sw.hasShrinkwrap,
       dev,
       optional,

workspaces/arborist/lib/arborist/reify.js

@@ -149,7 +149,7 @@ module.exports = cls => class Reifier extends cls {
     for (const path of this[_trashList]) {
       const loc = relpath(this.idealTree.realpath, path)
       const node = this.idealTree.inventory.get(loc)
-      if (node && node.root === this.idealTree) {
+      if (node && node.root === this.idealTree && !node.ideallyInert) {
         node.parent = null
       }
     }
@@ -237,6 +237,18 @@ module.exports = cls => class Reifier extends cls {
     this.idealTree.meta.hiddenLockfile = true
     this.idealTree.meta.lockfileVersion = defaultLockfileVersion
 
+    // Preserve inertness for failed stuff.
+    if (this.actualTree) {
+      for (const [loc, actual] of this.actualTree.inventory.entries()) {
+        if (actual.ideallyInert) {
+          const ideal = this.idealTree.inventory.get(loc)
+          if (ideal) {
+            ideal.ideallyInert = true
+          }
+        }
+      }
+    }
+
     this.actualTree = this.idealTree
     this.idealTree = null
 
@@ -599,6 +611,9 @@ module.exports = cls => class Reifier extends cls {
     // retire the same path at the same time.
     const dirsChecked = new Set()
     return promiseAllRejectLate(leaves.map(async node => {
+      if (node.ideallyInert) {
+        return
+      }
       for (const d of walkUp(node.path)) {
         if (d === node.top.path) {
           break
@@ -743,6 +758,10 @@ module.exports = cls => class Reifier extends cls {
   }
 
   async #extractOrLink (node) {
+    if (node.ideallyInert) {
+      return
+    }
+
     const nm = resolve(node.parent.path, 'node_modules')
     await this.#validateNodeModules(nm)
 
@@ -818,6 +837,7 @@ module.exports = cls => class Reifier extends cls {
       const set = optionalSet(node)
       for (node of set) {
         log.verbose('reify', 'failed optional dependency', node.path)
+        node.ideallyInert = true
         this[_addNodeToTrashList](node)
       }
     }) : p).then(() => node)
@@ -1152,6 +1172,9 @@ module.exports = cls => class Reifier extends cls {
 
       this.#retiredUnchanged[retireFolder] = []
       return promiseAllRejectLate(diff.unchanged.map(node => {
+        if (node.ideallyInert) {
+          return
+        }
         // no need to roll back links, since we'll just delete them anyway
         if (node.isLink) {
           return mkdir(dirname(node.path), { recursive: true, force: true })
@@ -1231,7 +1254,7 @@ module.exports = cls => class Reifier extends cls {
       // skip links that only live within node_modules as they are most
       // likely managed by packages we installed, we only want to rebuild
       // unchanged links we directly manage
-      const linkedFromRoot = node.parent === tree || node.target.fsTop === tree
+      const linkedFromRoot = (node.parent === tree && !node.ideallyInert) || node.target.fsTop === tree
       if (node.isLink && linkedFromRoot) {
         nodes.push(node)
       }

workspaces/arborist/lib/node.js

@@ -103,6 +103,7 @@ class Node {
       global = false,
       dummy = false,
       sourceReference = null,
+      ideallyInert = false,
     } = options
     // this object gives querySelectorAll somewhere to stash context about a node
     // while processing a query
@@ -197,6 +198,8 @@ class Node {
       this.extraneous = false
     }
 
+    this.ideallyInert = ideallyInert
+
     this.edgesIn = new Set()
     this.edgesOut = new CaseInsensitiveMap()
 

workspaces/arborist/lib/shrinkwrap.js

@@ -109,6 +109,7 @@ const nodeMetaKeys = [
   'inBundle',
   'hasShrinkwrap',
   'hasInstallScript',
+  'ideallyInert',
 ]
 
 const metaFieldFromPkg = (pkg, key) => {
@@ -135,6 +136,10 @@ const assertNoNewer = async (path, data, lockTime, dir, seen) => {
 
   const parent = isParent ? dir : resolve(dir, 'node_modules')
   const rel = relpath(path, dir)
+  const inert = data.packages[rel]?.ideallyInert
+  if (inert) {
+    return
+  }
   seen.add(rel)
   let entries
   if (dir === path) {
@@ -173,7 +178,7 @@ const assertNoNewer = async (path, data, lockTime, dir, seen) => {
 
   // assert that all the entries in the lockfile were seen
   for (const loc in data.packages) {
-    if (!seen.has(loc)) {
+    if (!seen.has(loc) && !data.packages[loc].ideallyInert) {
       throw new Error(`missing from node_modules: ${loc}`)
     }
   }
@@ -783,6 +788,10 @@ class Shrinkwrap {
       // ok, I did my best!  good luck!
     }
 
+    if (lock.ideallyInert) {
+      meta.ideallyInert = true
+    }
+
     if (lock.bundled) {
       meta.inBundle = true
     }
@@ -953,6 +962,12 @@ class Shrinkwrap {
       this.#buildLegacyLockfile(this.tree, this.data)
     }
 
+    if (!this.hiddenLockfile) {
+      for (const node of Object.values(this.data.packages)) {
+        delete node.ideallyInert
+      }
+    }
+
     // lf version 1 = dependencies only
     // lf version 2 = dependencies and packages
     // lf version 3 = packages only

workspaces/arborist/tap-snapshots/test/arborist/build-ideal-tree.js.test.cjs

@@ -77868,11 +77868,34 @@ ArboristNode {
 
 exports[`test/arborist/build-ideal-tree.js TAP optional dependency failures > optional-dep-enotarget 1`] = `
 ArboristNode {
+  "children": Map {
+    "tap" => ArboristNode {
+      "edgesIn": Set {
+        EdgeIn {
+          "error": "INVALID",
+          "from": "",
+          "name": "tap",
+          "spec": "9999.0000.9999",
+          "type": "optional",
+        },
+      },
+      "errors": Array [
+        Object {
+          "code": "ETARGET",
+        },
+      ],
+      "location": "node_modules/tap",
+      "name": "tap",
+      "optional": true,
+      "path": "{CWD}/test/fixtures/optional-dep-enotarget/node_modules/tap",
+    },
+  },
   "edgesOut": Map {
     "tap" => EdgeOut {
+      "error": "INVALID",
       "name": "tap",
       "spec": "9999.0000.9999",
-      "to": null,
+      "to": "node_modules/tap",
       "type": "optional",
     },
   },
@@ -77887,11 +77910,32 @@ ArboristNode {
 
 exports[`test/arborist/build-ideal-tree.js TAP optional dependency failures > optional-dep-missing 1`] = `
 ArboristNode {
+  "children": Map {
+    "@isaacs/this-does-not-exist-at-all" => ArboristNode {
+      "edgesIn": Set {
+        EdgeIn {
+          "from": "",
+          "name": "@isaacs/this-does-not-exist-at-all",
+          "spec": "*",
+          "type": "optional",
+        },
+      },
+      "errors": Array [
+        Object {
+          "code": "E404",
+        },
+      ],
+      "location": "node_modules/@isaacs/this-does-not-exist-at-all",
+      "name": "@isaacs/this-does-not-exist-at-all",
+      "optional": true,
+      "path": "{CWD}/test/fixtures/optional-dep-missing/node_modules/@isaacs/this-does-not-exist-at-all",
+    },
+  },
   "edgesOut": Map {
     "@isaacs/this-does-not-exist-at-all" => EdgeOut {
       "name": "@isaacs/this-does-not-exist-at-all",
       "spec": "*",
-      "to": null,
+      "to": "node_modules/@isaacs/this-does-not-exist-at-all",
       "type": "optional",
     },
   },
@@ -77906,11 +77950,60 @@ ArboristNode {
 
 exports[`test/arborist/build-ideal-tree.js TAP optional dependency failures > optional-metadep-enotarget 1`] = `
 ArboristNode {
+  "children": Map {
+    "@isaacs/prod-dep-enotarget" => ArboristNode {
+      "children": Map {
+        "tap" => ArboristNode {
+          "edgesIn": Set {
+            EdgeIn {
+              "error": "INVALID",
+              "from": "node_modules/@isaacs/prod-dep-enotarget",
+              "name": "tap",
+              "spec": "9999.0000.9999",
+              "type": "prod",
+            },
+          },
+          "errors": Array [
+            Object {
+              "code": "ETARGET",
+            },
+          ],
+          "location": "node_modules/@isaacs/prod-dep-enotarget/node_modules/tap",
+          "name": "tap",
+          "optional": true,
+          "path": "{CWD}/test/fixtures/optional-metadep-enotarget/node_modules/@isaacs/prod-dep-enotarget/node_modules/tap",
+        },
+      },
+      "edgesIn": Set {
+        EdgeIn {
+          "from": "",
+          "name": "@isaacs/prod-dep-enotarget",
+          "spec": "*",
+          "type": "optional",
+        },
+      },
+      "edgesOut": Map {
+        "tap" => EdgeOut {
+          "error": "INVALID",
+          "name": "tap",
+          "spec": "9999.0000.9999",
+          "to": "node_modules/@isaacs/prod-dep-enotarget/node_modules/tap",
+          "type": "prod",
+        },
+      },
+      "location": "node_modules/@isaacs/prod-dep-enotarget",
+      "name": "@isaacs/prod-dep-enotarget",
+      "optional": true,
+      "path": "{CWD}/test/fixtures/optional-metadep-enotarget/node_modules/@isaacs/prod-dep-enotarget",
+      "resolved": "https://registry.npmjs.org/@isaacs/prod-dep-enotarget/-/prod-dep-enotarget-1.0.0.tgz",
+      "version": "1.0.0",
+    },
+  },
   "edgesOut": Map {
     "@isaacs/prod-dep-enotarget" => EdgeOut {
       "name": "@isaacs/prod-dep-enotarget",
       "spec": "*",
-      "to": null,
+      "to": "node_modules/@isaacs/prod-dep-enotarget",
       "type": "optional",
     },
   },
@@ -77924,11 +78017,58 @@ ArboristNode {
 
 exports[`test/arborist/build-ideal-tree.js TAP optional dependency failures > optional-metadep-missing 1`] = `
 ArboristNode {
+  "children": Map {
+    "@isaacs/testing-prod-dep-metadata-missing" => ArboristNode {
+      "children": Map {
+        "@isaacs/this-does-not-exist-at-all" => ArboristNode {
+          "edgesIn": Set {
+            EdgeIn {
+              "from": "node_modules/@isaacs/testing-prod-dep-metadata-missing",
+              "name": "@isaacs/this-does-not-exist-at-all",
+              "spec": "*",
+              "type": "prod",
+            },
+          },
+          "errors": Array [
+            Object {
+              "code": "E404",
+            },
+          ],
+          "location": "node_modules/@isaacs/testing-prod-dep-metadata-missing/node_modules/@isaacs/this-does-not-exist-at-all",
+          "name": "@isaacs/this-does-not-exist-at-all",
+          "optional": true,
+          "path": "{CWD}/test/fixtures/optional-metadep-missing/node_modules/@isaacs/testing-prod-dep-metadata-missing/node_modules/@isaacs/this-does-not-exist-at-all",
+        },
+      },
+      "edgesIn": Set {
+        EdgeIn {
+          "from": "",
+          "name": "@isaacs/testing-prod-dep-metadata-missing",
+          "spec": "*",
+          "type": "optional",
+        },
+      },
+      "edgesOut": Map {
+        "@isaacs/this-does-not-exist-at-all" => EdgeOut {
+          "name": "@isaacs/this-does-not-exist-at-all",
+          "spec": "*",
+          "to": "node_modules/@isaacs/testing-prod-dep-metadata-missing/node_modules/@isaacs/this-does-not-exist-at-all",
+          "type": "prod",
+        },
+      },
+      "location": "node_modules/@isaacs/testing-prod-dep-metadata-missing",
+      "name": "@isaacs/testing-prod-dep-metadata-missing",
+      "optional": true,
+      "path": "{CWD}/test/fixtures/optional-metadep-missing/node_modules/@isaacs/testing-prod-dep-metadata-missing",
+      "resolved": "https://registry.npmjs.org/@isaacs/testing-prod-dep-metadata-missing/-/testing-prod-dep-metadata-missing-1.0.0.tgz",
+      "version": "1.0.0",
+    },
+  },
   "edgesOut": Map {
     "@isaacs/testing-prod-dep-metadata-missing" => EdgeOut {
       "name": "@isaacs/testing-prod-dep-metadata-missing",
       "spec": "*",
-      "to": null,
+      "to": "node_modules/@isaacs/testing-prod-dep-metadata-missing",
       "type": "optional",
     },
   },