fix(ci): rm workspace node_modules

lib/commands/ci.js

@@ -4,6 +4,7 @@ const fs = require('fs/promises')
 const { log, time } = require('proc-log')
 const validateLockfile = require('../utils/validate-lockfile.js')
 const ArboristWorkspaceCmd = require('../arborist-cmd.js')
+const getWorkspaces = require('../utils/get-workspaces.js')
 
 class CI extends ArboristWorkspaceCmd {
   static description = 'Clean install a project'
@@ -74,16 +75,24 @@ class CI extends ArboristWorkspaceCmd {
       )
     }
 
+    const workspacePaths = await getWorkspaces([], {
+      path: this.npm.localPrefix,
+      includeWorkspaceRoot: true,
+    })
+
     const dryRun = this.npm.config.get('dry-run')
     if (!dryRun) {
       // Only remove node_modules after we've successfully loaded the virtual
       // tree and validated the lockfile
       await time.start('npm-ci:rm', async () => {
-        const path = `${where}/node_modules`
-        // get the list of entries so we can skip the glob for performance
-        const entries = await fs.readdir(path, null).catch(() => [])
-        return Promise.all(entries.map(f => fs.rm(`${path}/${f}`,
-          { force: true, recursive: true })))
+        return await Promise.all([...workspacePaths.values()].map(async modulePath => {
+          const path = `${modulePath}/node_modules`
+          // get the list of entries so we can skip the glob for performance
+          const entries = await fs.readdir(path, null).catch(() => [])
+          return Promise.all(entries.map(f => {
+            return fs.rm(`${path}/${f}`, { force: true, recursive: true })
+          }))
+        }))
       })
     }
 

test/lib/commands/ci.js

@@ -12,6 +12,7 @@ const loadMockNpm = async (t, opts) => {
   const registry = new MockRegistry({
     tap: t,
     registry: mock.npm.config.get('registry'),
+    strict: true,
   })
   return { registry, ...mock }
 }
@@ -113,6 +114,11 @@ t.test('reifies, audits, removes node_modules on repeat run', async t => {
     manifest: manifest.versions['1.0.0'],
     tarball: path.join(npm.prefix, 'abbrev'),
   })
+  await registry.tarball({
+    manifest: manifest.versions['1.0.0'],
+    tarball: path.join(npm.prefix, 'abbrev'),
+  })
+  registry.nock.post('/-/npm/v1/security/advisories/bulk').reply(200, {})
   registry.nock.post('/-/npm/v1/security/advisories/bulk').reply(200, {})
   await npm.exec('ci', [])
   await npm.exec('ci', [])
@@ -142,9 +148,6 @@ t.test('--no-audit and --ignore-scripts', async t => {
       'package-lock.json': JSON.stringify(packageLock),
     },
   })
-  require('nock').emitter.on('no match', () => {
-    t.fail('Should not audit')
-  })
   const manifest = registry.manifest({ name: 'abbrev' })
   await registry.tarball({
     manifest: manifest.versions['1.0.0'],
@@ -230,3 +233,142 @@ t.test('should throw error when ideal inventory mismatches virtual', async t =>
   const nmTestFile = path.join(npm.prefix, 'node_modules', 'test-file')
   t.equal(fs.existsSync(nmTestFile), true, 'does not remove node_modules')
 })
+
+t.test('should remove node_modules within workspaces', async t => {
+  const { npm, registry } = await loadMockNpm(t, {
+    prefixDir: {
+      tarballs: {
+        oneOneZero: {
+          'package.json': JSON.stringify({ name: 'abbrev', version: '1.1.0' }),
+          'index.js': 'module.exports = "hello world"',
+        },
+        oneOneOne: {
+          'package.json': JSON.stringify({ name: 'abbrev', version: '1.1.1' }),
+          'index.js': 'module.exports = "hello world"',
+        },
+      },
+      node_modules: {
+        abbrev: {
+          'foo.txt': '',
+          'package.json': JSON.stringify({
+            name: 'abbrev',
+            version: '1.1.0',
+          }),
+        },
+        'workspace-a': t.fixture('symlink', '../workspace-a'),
+        'workspace-b': t.fixture('symlink', '../workspace-b'),
+      },
+      'package-lock.json': JSON.stringify({
+        name: 'workspace-test-3',
+        version: '1.0.0',
+        lockfileVersion: 3,
+        requires: true,
+        packages: {
+          '': {
+            name: 'workspace-test-3',
+            version: '1.0.0',
+            workspaces: [
+              'workspace-a',
+              'workspace-b',
+            ],
+          },
+          'node_modules/abbrev': {
+            version: '1.1.0',
+            resolved: 'https://registry.npmjs.org/abbrev/-/abbrev-1.1.0.tgz',
+          },
+          'node_modules/workspace-a': {
+            resolved: 'workspace-a',
+            link: true,
+          },
+          'node_modules/workspace-b': {
+            resolved: 'workspace-b',
+            link: true,
+          },
+          'workspace-a': {
+            version: '1.0.0',
+            dependencies: {
+              abbrev: '1.1.0',
+            },
+          },
+          'workspace-b': {
+            version: '1.0.0',
+            dependencies: {
+              abbrev: '1.1.1',
+            },
+            devDependencies: {},
+          },
+          'workspace-b/node_modules/abbrev': {
+            version: '1.1.1',
+            resolved: 'https://registry.npmjs.org/abbrev/-/abbrev-1.1.1.tgz',
+          },
+        },
+      }),
+      'package.json': JSON.stringify({
+        name: 'workspace-test-3',
+        version: '1.0.0',
+        workspaces: [
+          'workspace-a',
+          'workspace-b',
+        ],
+      }),
+      'workspace-a': {
+        'package.json': JSON.stringify({
+          name: 'workspace-a',
+          version: '1.0.0',
+          dependencies: {
+            abbrev: '1.1.0',
+          },
+        }),
+      },
+      'workspace-b': {
+        node_modules: {
+          abbrev: {
+            'bar.txt': '',
+            'package.json': JSON.stringify({
+              name: 'abbrev',
+              version: '1.1.1',
+            }),
+          },
+        },
+        'package.json': JSON.stringify({
+          name: 'workspace-b',
+          version: '1.0.0',
+          dependencies: {
+            abbrev: '1.1.1',
+          },
+        }),
+      },
+    },
+  })
+
+  const manifest = registry.manifest({
+    name: 'abbrev',
+    versions: ['1.1.0', '1.1.1'],
+  })
+
+  await registry.tarball({
+    manifest: manifest.versions['1.1.0'],
+    tarball: path.join(npm.prefix, 'tarballs/oneOneZero'),
+  })
+
+  await registry.tarball({
+    manifest: manifest.versions['1.1.1'],
+    tarball: path.join(npm.prefix, 'tarballs/oneOneOne'),
+  })
+
+  registry.nock.post('/-/npm/v1/security/advisories/bulk').reply(200, {})
+
+  await npm.exec('ci', [])
+
+  const rmFooTest = path.join(npm.prefix, 'node_modules/abbrev/foo.txt')
+  t.equal(fs.existsSync(rmFooTest), false, 'existing node_modules is removed')
+
+  const rmBarTest = path.join(npm.prefix, 'workspace-b/node_modules/abbrev/bar.txt')
+  t.equal(fs.existsSync(rmBarTest), false, 'existing ws node_modules is removed')
+
+  const checkNmAbbrev = path.join(npm.prefix, 'node_modules/abbrev')
+  t.equal(fs.existsSync(checkNmAbbrev), true, 'installs abbrev')
+
+  const checkWsAbbrev = path.join(npm.prefix, 'workspace-b/node_modules/abbrev')
+  t.equal(fs.existsSync(checkWsAbbrev), true, 'installs abbrev')
+})