fix(ci): remove node_modules post-validation

docs/content/commands/npm-ci.md

@@ -1,7 +1,7 @@
 ---
 title: npm-ci
 section: 1
-description: Install a project with a clean slate
+description: Clean install a project
 ---
 
 ### Synopsis
@@ -28,12 +28,7 @@ it's meant to be used in automated environments such as test platforms,
 continuous integration, and deployment -- or any situation where you want
 to make sure you're doing a clean install of your dependencies.
 
-`npm ci` will be significantly faster when:
-
-- There is a `package-lock.json` or `npm-shrinkwrap.json` file.
-- The `node_modules` folder is missing or empty.
-
-In short, the main differences between using `npm install` and `npm ci` are:
+The main differences between using `npm install` and `npm ci` are:
 
 * The project **must** have an existing `package-lock.json` or
   `npm-shrinkwrap.json`.

lib/commands/ci.js

@@ -8,19 +8,10 @@ const readdir = util.promisify(fs.readdir)
 const log = require('../utils/log-shim.js')
 const validateLockfile = require('../utils/validate-lockfile.js')
 
-const removeNodeModules = async where => {
-  const rimrafOpts = { glob: false }
-  process.emit('time', 'npm-ci:rm')
-  const path = `${where}/node_modules`
-  // get the list of entries so we can skip the glob for performance
-  const entries = await readdir(path, null).catch(er => [])
-  await Promise.all(entries.map(f => rimraf(`${path}/${f}`, rimrafOpts)))
-  process.emit('timeEnd', 'npm-ci:rm')
-}
 const ArboristWorkspaceCmd = require('../arborist-cmd.js')
 
 class CI extends ArboristWorkspaceCmd {
-  static description = 'Install a project with a clean slate'
+  static description = 'Clean install a project'
   static name = 'ci'
   static params = [
     'audit',
@@ -31,9 +22,9 @@ class CI extends ArboristWorkspaceCmd {
 
   async exec () {
     if (this.npm.config.get('global')) {
-      const err = new Error('`npm ci` does not work for global packages')
-      err.code = 'ECIGLOBAL'
-      throw err
+      throw Object.assign(new Error('`npm ci` does not work for global packages'), {
+        code: 'ECIGLOBAL',
+      })
     }
 
     const where = this.npm.prefix
@@ -46,17 +37,14 @@ class CI extends ArboristWorkspaceCmd {
     }
 
     const arb = new Arborist(opts)
-    await Promise.all([
-      arb.loadVirtual().catch(er => {
-        log.verbose('loadVirtual', er.stack)
-        const msg =
-          'The `npm ci` command can only install with an existing package-lock.json or\n' +
-          'npm-shrinkwrap.json with lockfileVersion >= 1. Run an install with npm@5 or\n' +
-          'later to generate a package-lock.json file, then try again.'
-        throw new Error(msg)
-      }),
-      removeNodeModules(where),
-    ])
+    await arb.loadVirtual().catch(er => {
+      log.verbose('loadVirtual', er.stack)
+      const msg =
+        'The `npm ci` command can only install with an existing package-lock.json or\n' +
+        'npm-shrinkwrap.json with lockfileVersion >= 1. Run an install with npm@5 or\n' +
+        'later to generate a package-lock.json file, then try again.'
+      throw this.usageError(msg)
+    })
 
     // retrieves inventory of packages from loaded virtual tree (lock file)
     const virtualInventory = new Map(arb.virtualTree.inventory)
@@ -70,15 +58,24 @@ class CI extends ArboristWorkspaceCmd {
     // throws a validation error in case of mismatches
     const errors = validateLockfile(virtualInventory, arb.idealTree.inventory)
     if (errors.length) {
-      throw new Error(
+      throw this.usageError(
         '`npm ci` can only install packages when your package.json and ' +
         'package-lock.json or npm-shrinkwrap.json are in sync. Please ' +
         'update your lock file with `npm install` ' +
         'before continuing.\n\n' +
-        errors.join('\n') + '\n'
+        errors.join('\n')
       )
     }
 
+    // Only remove node_modules after we've successfully loaded the virtual
+    // tree and validated the lockfile
+    await this.npm.time('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 readdir(path, null).catch(er => [])
+      return Promise.all(entries.map(f => rimraf(`${path}/${f}`, { glob: false })))
+    })
+
     await arb.reify(opts)
 
     const ignoreScripts = this.npm.config.get('ignore-scripts')

smoke-tests/tap-snapshots/test/index.js.test.cjs

@@ -43,10 +43,24 @@ npm {CWD}
 `
 
 exports[`test/index.js TAP npm ci > should throw mismatch deps in lock file error 1`] = `
+npm ERR! code EUSAGE
+npm ERR! 
 npm ERR! \`npm ci\` can only install packages when your package.json and package-lock.json or npm-shrinkwrap.json are in sync. Please update your lock file with \`npm install\` before continuing.
 npm ERR! 
 npm ERR! Invalid: lock file's abbrev@1.0.4 does not satisfy abbrev@1.1.1
 npm ERR! 
+npm ERR! Clean install a project
+npm ERR! 
+npm ERR! Usage:
+npm ERR! npm ci
+npm ERR! 
+npm ERR! Options:
+npm ERR! [--no-audit] [--foreground-scripts] [--ignore-scripts]
+npm ERR! [--script-shell <script-shell>]
+npm ERR! 
+npm ERR! aliases: clean-install, ic, install-clean, isntall-clean
+npm ERR! 
+npm ERR! Run "npm help ci" for more info
 
 npm ERR! A complete log of this run can be found in:
 

tap-snapshots/test/lib/load-all-commands.js.test.cjs

@@ -113,7 +113,7 @@ Run "npm help cache" for more info
 `
 
 exports[`test/lib/load-all-commands.js TAP load each command ci > must match snapshot 1`] = `
-Install a project with a clean slate
+Clean install a project
 
 Usage:
 npm ci

tap-snapshots/test/lib/utils/npm-usage.js.test.cjs

@@ -251,7 +251,7 @@ All commands:
 
                     Run "npm help cache" for more info
 
-    ci              Install a project with a clean slate
+    ci              Clean install a project
 
                     Usage:
                     npm ci

test/fixtures/mock-registry.js

@@ -202,16 +202,21 @@ class MockRegistry {
     nock = nock.reply(200, manifest)
     if (tarballs) {
       for (const version in tarballs) {
-      // for (const version in manifest.versions) {
-        const packument = manifest.versions[version]
-        const dist = new URL(packument.dist.tarball)
-        const tarball = await pacote.tarball(tarballs[version])
-        nock.get(dist.pathname).reply(200, tarball)
+        const m = manifest.versions[version]
+        nock = await this.tarball({ manifest: m, tarball: tarballs[version] })
       }
     }
     this.nock = nock
   }
 
+  async tarball ({ manifest, tarball }) {
+    const nock = this.nock
+    const dist = new URL(manifest.dist.tarball)
+    const tar = await pacote.tarball(tarball)
+    nock.get(dist.pathname).reply(200, tar)
+    return nock
+  }
+
   // either pass in packuments if you need to set specific attributes besides version,
   // or an array of versions
   // the last packument in the packuments or versions array will be tagged latest