fix(arborist): prune removed-workspace entries from package-lock.json

Author: manzoorwanijk

workspaces/arborist/lib/shrinkwrap.js

@@ -905,6 +905,10 @@ class Shrinkwrap {
           continue
         }
         const loc = relpath(this.path, node.path)
+        // Drop lockfile entries for extraneous nodes outside node_modules. These are stale workspace entries: the workspace was removed from package.json or its directory was deleted, so it should not be tracked in package-lock.json.
+        if (node.extraneous && !/(^|\/)node_modules\//.test(loc) && loc !== 'node_modules') {
+          continue
+        }
         this.data.packages[loc] = Shrinkwrap.metaFromNode(
           node,
           this.path,

workspaces/arborist/tap-snapshots/test/arborist/reify.js.test.cjs

@@ -3183,10 +3183,6 @@ exports[`test/arborist/reify.js TAP filtered reification in workspaces > hidden
     "apps/x": {
       "version": "1.2.3"
     },
-    "foo/x": {
-      "version": "1.2.3",
-      "extraneous": true
-    },
     "node_modules/c": {
       "resolved": "packages/c",
       "link": true
@@ -32370,9 +32366,6 @@ Object {
         "e",
       ],
     },
-    "e": Object {
-      "extraneous": true,
-    },
     "node_modules/a": Object {
       "extraneous": true,
       "inBundle": true,

workspaces/arborist/tap-snapshots/test/shrinkwrap.js.test.cjs

@@ -2734,14 +2734,6 @@ Object {
       "name": "example",
       "version": "1.0.0",
     },
-    "../bar": Object {
-      "extraneous": true,
-      "version": "1.0.0",
-    },
-    "../linked-node-modules/foo": Object {
-      "extraneous": true,
-      "version": "1.0.0",
-    },
     "node_modules/bar": Object {
       "link": true,
       "resolved": "../bar",
@@ -9755,16 +9747,6 @@ Object {
     "": Object {
       "name": "workspace3",
     },
-    "app": Object {
-      "dependencies": Object {
-        "a": "",
-        "b": "",
-        "c": "",
-        "i": "",
-      },
-      "extraneous": true,
-      "version": "1.2.3",
-    },
     "app/node_modules/i": Object {
       "extraneous": true,
       "version": "1.2.3",
@@ -9785,41 +9767,14 @@ Object {
       "link": true,
       "resolved": "packages/c",
     },
-    "packages/a": Object {
-      "dependencies": Object {
-        "b": "",
-        "c": "",
-        "x": "",
-      },
-      "extraneous": true,
-      "version": "1.2.3",
-    },
     "packages/a/node_modules/x": Object {
       "extraneous": true,
       "version": "1.2.3",
     },
-    "packages/b": Object {
-      "dependencies": Object {
-        "a": "",
-        "c": "",
-        "y": "",
-      },
-      "extraneous": true,
-      "version": "1.2.3",
-    },
     "packages/b/node_modules/y": Object {
       "extraneous": true,
       "version": "1.2.3",
     },
-    "packages/c": Object {
-      "dependencies": Object {
-        "a": "",
-        "b": "",
-        "z": "",
-      },
-      "extraneous": true,
-      "version": "1.2.3",
-    },
     "packages/c/node_modules/z": Object {
       "extraneous": true,
       "version": "1.2.3",

workspaces/arborist/test/arborist/reify.js

@@ -1960,6 +1960,65 @@ t.test('filtered reification in workspaces', async t => {
     'hidden lockfile - foo/x linked, c, old x, removed a')
 })
 
+// Regression for https://github.com/npm/cli/issues/5463: a workspace whose directory has been deleted should not leave behind an extraneous entry (or a lingering reference in the root's workspaces array) in package-lock.json after `npm install`.
+t.test('removed workspace is pruned from package-lock.json', async t => {
+  const setup = () => {
+    const path = t.testdir({
+      'package.json': JSON.stringify({
+        name: 'remove-ws',
+        version: '1.0.0',
+        workspaces: ['packages/a', 'packages/b'],
+      }),
+      packages: {
+        a: {
+          'package.json': JSON.stringify({ name: 'a', version: '1.0.0' }),
+        },
+        b: {
+          'package.json': JSON.stringify({ name: 'b', version: '1.0.0' }),
+        },
+      },
+    })
+    return path
+  }
+
+  // The lockfile's root.workspaces array mirrors package.json verbatim and is intentionally not mutated here, so we only assert that orphan package/link entries are dropped.
+  const assertClean = (t, path, label) => {
+    const lock = JSON.parse(fs.readFileSync(`${path}/package-lock.json`, 'utf8'))
+    t.notOk(lock.packages['packages/b'],
+      `${label}: packages/b entry removed from lockfile`)
+    t.notOk(lock.packages['node_modules/b'],
+      `${label}: node_modules/b link removed from lockfile`)
+    t.notOk(lock.dependencies && lock.dependencies.b,
+      `${label}: dependencies.b removed from legacy lockfile`)
+  }
+
+  for (const strategy of ['hoisted', 'linked']) {
+    t.test(`${strategy} strategy, package.json kept stale`, async t => {
+      const path = setup()
+      createRegistry(t, false)
+      await reify(path, { installStrategy: strategy })
+      // Remove only the directory, leave package.json's workspaces array alone.
+      fs.rmSync(`${path}/packages/b`, { recursive: true, force: true })
+      await reify(path, { installStrategy: strategy })
+      assertClean(t, path, `${strategy}/keep-pj`)
+    })
+
+    t.test(`${strategy} strategy, package.json updated`, async t => {
+      const path = setup(strategy)
+      createRegistry(t, false)
+      await reify(path, { installStrategy: strategy })
+      fs.rmSync(`${path}/packages/b`, { recursive: true, force: true })
+      fs.writeFileSync(`${path}/package.json`, JSON.stringify({
+        name: 'remove-ws',
+        version: '1.0.0',
+        workspaces: ['packages/a'],
+      }))
+      await reify(path, { installStrategy: strategy })
+      assertClean(t, path, `${strategy}/clean-pj`)
+    })
+  }
+})
+
 t.test('project with bundled deps and a link dep on itself', async t => {
   const pkg = {
     name: '@isaacs/testing-bundle-self-link',