chore: refactor commands

This is the first phase of refactoring the internal structure of the npm
commands to set us up for future changes.  This iteration changes the
function signature of `exec` for all the commands to be a async (no more
callbacks), and also groups all the commands into their own
subdirectory.

It also removes the Proxy `npm.commands` object, in favor of an
`npm.cmd` and `npm.exec` function that breaks up the two things that
proxy was doing. Namely, getting to the attributes of a given command
(`npm.cmd` now does this), and actually running the command `npm.exec`
does this.

PR-URL: #3959
Credit: @wraithgar
Close: #3959
Reviewed-by: @lukekarrys

lib/workspaces/arborist-cmd.js → lib/arborist-cmd.js

@@ -2,7 +2,7 @@
 // a list of workspace names and passes it on to new Arborist() to
 // be able to run a filtered Arborist.reify() at some point.
 
-const BaseCommand = require('../base-command.js')
+const BaseCommand = require('./base-command.js')
 class ArboristCmd extends BaseCommand {
   get isArboristCmd () {
     return true
@@ -17,12 +17,9 @@ class ArboristCmd extends BaseCommand {
     ]
   }
 
-  execWorkspaces (args, filters, cb) {
-    this.setWorkspaces(filters, true)
-      .then(() => {
-        this.exec(args, cb)
-      })
-      .catch(er => cb(er))
+  async execWorkspaces (args, filters) {
+    await this.setWorkspaces(filters)
+    return this.exec(args)
   }
 }
 

lib/base-command.js

@@ -65,7 +65,7 @@ class BaseCommand {
     })
   }
 
-  execWorkspaces (args, filters, cb) {
+  async execWorkspaces (args, filters) {
     throw Object.assign(
       new Error('This command does not support workspaces.'),
       { code: 'ENOWORKSPACES' }

lib/cli.js

@@ -18,7 +18,8 @@ module.exports = async (process) => {
 
   checkForUnsupportedNode()
 
-  const npm = require('../lib/npm.js')
+  const Npm = require('../lib/npm.js')
+  const npm = new Npm()
   const exitHandler = require('../lib/utils/exit-handler.js')
   exitHandler.setNpm(npm)
 
@@ -38,6 +39,7 @@ module.exports = async (process) => {
 
   const updateNotifier = require('../lib/utils/update-notifier.js')
 
+  let cmd
   // now actually fire up npm and run the command.
   // this is how to use npm programmatically:
   try {
@@ -55,24 +57,23 @@ module.exports = async (process) => {
 
     updateNotifier(npm)
 
-    const cmd = npm.argv.shift()
+    cmd = npm.argv.shift()
     if (!cmd) {
-      npm.output(npm.usage)
+      npm.output(await npm.usage)
       process.exitCode = 1
       return exitHandler()
     }
 
-    const impl = npm.commands[cmd]
-    if (!impl) {
+    await npm.exec(cmd, npm.argv)
+    exitHandler()
+  } catch (err) {
+    if (err.code === 'EUNKNOWNCOMMAND') {
       const didYouMean = require('./utils/did-you-mean.js')
       const suggestions = await didYouMean(npm, npm.localPrefix, cmd)
       npm.output(`Unknown command: "${cmd}"${suggestions}\n\nTo see a list of supported npm commands, run:\n  npm help`)
       process.exitCode = 1
       return exitHandler()
     }
-
-    impl(npm.argv, exitHandler)
-  } catch (err) {
     return exitHandler(err)
   }
 }

lib/access.js → lib/commands/access.js

@@ -3,9 +3,9 @@ const path = require('path')
 const libaccess = require('libnpmaccess')
 const readPackageJson = require('read-package-json-fast')
 
-const otplease = require('./utils/otplease.js')
-const getIdentity = require('./utils/get-identity.js')
-const BaseCommand = require('./base-command.js')
+const otplease = require('../utils/otplease.js')
+const getIdentity = require('../utils/get-identity.js')
+const BaseCommand = require('../base-command.js')
 
 const subcommands = [
   'public',
@@ -76,11 +76,7 @@ class Access extends BaseCommand {
     }
   }
 
-  exec (args, cb) {
-    this.access(args).then(() => cb()).catch(cb)
-  }
-
-  async access ([cmd, ...args]) {
+  async exec ([cmd, ...args]) {
     if (!cmd)
       throw this.usageError('Subcommand is required.')
 

lib/adduser.js → lib/commands/adduser.js

@@ -1,11 +1,11 @@
 const log = require('npmlog')
-const replaceInfo = require('./utils/replace-info.js')
-const BaseCommand = require('./base-command.js')
+const replaceInfo = require('../utils/replace-info.js')
+const BaseCommand = require('../base-command.js')
 const authTypes = {
-  legacy: require('./auth/legacy.js'),
-  oauth: require('./auth/oauth.js'),
-  saml: require('./auth/saml.js'),
-  sso: require('./auth/sso.js'),
+  legacy: require('../auth/legacy.js'),
+  oauth: require('../auth/oauth.js'),
+  saml: require('../auth/saml.js'),
+  sso: require('../auth/sso.js'),
 }
 
 class AddUser extends BaseCommand {
@@ -24,11 +24,7 @@ class AddUser extends BaseCommand {
     ]
   }
 
-  exec (args, cb) {
-    this.adduser(args).then(() => cb()).catch(cb)
-  }
-
-  async adduser (args) {
+  async exec (args) {
     const { scope } = this.npm.flatOptions
     const registry = this.getRegistry(this.npm.flatOptions)
     const auth = this.getAuthType(this.npm.flatOptions)

lib/audit.js → lib/commands/audit.js

@@ -1,8 +1,8 @@
 const Arborist = require('@npmcli/arborist')
 const auditReport = require('npm-audit-report')
-const reifyFinish = require('./utils/reify-finish.js')
-const auditError = require('./utils/audit-error.js')
-const ArboristWorkspaceCmd = require('./workspaces/arborist-cmd.js')
+const reifyFinish = require('../utils/reify-finish.js')
+const auditError = require('../utils/audit-error.js')
+const ArboristWorkspaceCmd = require('../arborist-cmd.js')
 
 class Audit extends ArboristWorkspaceCmd {
   /* istanbul ignore next - see test/lib/load-all-commands.js */
@@ -47,11 +47,7 @@ class Audit extends ArboristWorkspaceCmd {
     }
   }
 
-  exec (args, cb) {
-    this.audit(args).then(() => cb()).catch(cb)
-  }
-
-  async audit (args) {
+  async exec (args) {
     const reporter = this.npm.config.get('json') ? 'json' : 'detail'
     const opts = {
       ...this.npm.flatOptions,

lib/bin.js → lib/commands/bin.js

@@ -1,5 +1,5 @@
-const envPath = require('./utils/path.js')
-const BaseCommand = require('./base-command.js')
+const envPath = require('../utils/path.js')
+const BaseCommand = require('../base-command.js')
 
 class Bin extends BaseCommand {
   static get description () {
@@ -14,11 +14,7 @@ class Bin extends BaseCommand {
     return ['global']
   }
 
-  exec (args, cb) {
-    this.bin(args).then(() => cb()).catch(cb)
-  }
-
-  async bin (args) {
+  async exec (args) {
     const b = this.npm.bin
     this.npm.output(b)
     if (this.npm.config.get('global') && !envPath.includes(b))

lib/birthday.js → lib/commands/birthday.js

@@ -1,10 +1,11 @@
-const BaseCommand = require('./base-command.js')
+const BaseCommand = require('../base-command.js')
 
 class Birthday extends BaseCommand {
-  exec (args, cb) {
+  async exec () {
     this.npm.config.set('package', ['@npmcli/npm-birthday'])
     this.npm.config.set('yes', true)
-    return this.npm.commands.exec(['npm-birthday'], cb)
+    const exec = await this.npm.cmd('exec')
+    return exec.exec(['npm-birthday'])
   }
 }
 

lib/bugs.js → lib/commands/bugs.js

@@ -1,8 +1,8 @@
 const log = require('npmlog')
 const pacote = require('pacote')
-const openUrl = require('./utils/open-url.js')
-const hostedFromMani = require('./utils/hosted-git-info-from-manifest.js')
-const BaseCommand = require('./base-command.js')
+const openUrl = require('../utils/open-url.js')
+const hostedFromMani = require('../utils/hosted-git-info-from-manifest.js')
+const BaseCommand = require('../base-command.js')
 
 class Bugs extends BaseCommand {
   static get description () {
@@ -22,11 +22,7 @@ class Bugs extends BaseCommand {
     return ['browser', 'registry']
   }
 
-  exec (args, cb) {
-    this.bugs(args).then(() => cb()).catch(cb)
-  }
-
-  async bugs (args) {
+  async exec (args) {
     if (!args || !args.length)
       args = ['.']
 

lib/cache.js → lib/commands/cache.js

@@ -5,7 +5,7 @@ const pacote = require('pacote')
 const path = require('path')
 const rimraf = promisify(require('rimraf'))
 const semver = require('semver')
-const BaseCommand = require('./base-command.js')
+const BaseCommand = require('../base-command.js')
 const npa = require('npm-package-arg')
 const jsonParse = require('json-parse-even-better-errors')
 const localeCompare = require('@isaacs/string-locale-compare')('en')
@@ -104,11 +104,7 @@ class Cache extends BaseCommand {
     }
   }
 
-  exec (args, cb) {
-    this.cache(args).then(() => cb()).catch(cb)
-  }
-
-  async cache (args) {
+  async exec (args) {
     const cmd = args.shift()
     switch (cmd) {
       case 'rm': case 'clear': case 'clean':

lib/ci.js → lib/commands/ci.js

@@ -1,7 +1,7 @@
 const util = require('util')
 const Arborist = require('@npmcli/arborist')
 const rimraf = util.promisify(require('rimraf'))
-const reifyFinish = require('./utils/reify-finish.js')
+const reifyFinish = require('../utils/reify-finish.js')
 const runScript = require('@npmcli/run-script')
 const fs = require('fs')
 const readdir = util.promisify(fs.readdir)
@@ -17,7 +17,7 @@ const removeNodeModules = async where => {
   await Promise.all(entries.map(f => rimraf(`${path}/${f}`, rimrafOpts)))
   process.emit('timeEnd', 'npm-ci:rm')
 }
-const ArboristWorkspaceCmd = require('./workspaces/arborist-cmd.js')
+const ArboristWorkspaceCmd = require('../arborist-cmd.js')
 
 class CI extends ArboristWorkspaceCmd {
   /* istanbul ignore next - see test/lib/load-all-commands.js */
@@ -39,11 +39,7 @@ class CI extends ArboristWorkspaceCmd {
     ]
   }
 
-  exec (args, cb) {
-    this.ci().then(() => cb()).catch(cb)
-  }
-
-  async ci () {
+  async exec () {
     if (this.npm.config.get('global')) {
       const err = new Error('`npm ci` does not work for global packages')
       err.code = 'ECIGLOBAL'

lib/completion.js → lib/commands/completion.js

@@ -29,20 +29,20 @@
 // as an array.
 //
 
-const { definitions, shorthands } = require('./utils/config/index.js')
-const deref = require('./utils/deref-command.js')
-const { aliases, cmdList, plumbing } = require('./utils/cmd-list.js')
+const { definitions, shorthands } = require('../utils/config/index.js')
+const deref = require('../utils/deref-command.js')
+const { aliases, cmdList, plumbing } = require('../utils/cmd-list.js')
 const aliasNames = Object.keys(aliases)
 const fullList = cmdList.concat(aliasNames).filter(c => !plumbing.includes(c))
 const nopt = require('nopt')
 const configNames = Object.keys(definitions)
 const shorthandNames = Object.keys(shorthands)
 const allConfs = configNames.concat(shorthandNames)
-const isWindowsShell = require('./utils/is-windows-shell.js')
-const fileExists = require('./utils/file-exists.js')
+const isWindowsShell = require('../utils/is-windows-shell.js')
+const fileExists = require('../utils/file-exists.js')
 
 const { promisify } = require('util')
-const BaseCommand = require('./base-command.js')
+const BaseCommand = require('../base-command.js')
 
 class Completion extends BaseCommand {
   /* istanbul ignore next - see test/lib/load-all-commands.js */
@@ -75,11 +75,7 @@ class Completion extends BaseCommand {
     return out
   }
 
-  exec (args, cb) {
-    this.compl(args).then(() => cb()).catch(cb)
-  }
-
-  async compl (args) {
+  async exec (args) {
     if (isWindowsShell) {
       const msg = 'npm completion supported only in MINGW / Git bash on Windows'
       throw Object.assign(new Error(msg), {
@@ -163,8 +159,8 @@ class Completion extends BaseCommand {
     // at this point, if words[1] is some kind of npm command,
     // then complete on it.
     // otherwise, do nothing
-    const impl = this.npm.commands[cmd]
-    if (impl && impl.completion) {
+    const impl = this.npm.cmd(cmd)
+    if (impl.completion) {
       const comps = await impl.completion(opts)
       return this.wrap(opts, comps)
     }
@@ -195,19 +191,11 @@ const dumpScript = async () => {
   const fs = require('fs')
   const readFile = promisify(fs.readFile)
   const { resolve } = require('path')
-  const p = resolve(__dirname, 'utils/completion.sh')
+  const p = resolve(__dirname, '..', 'utils', 'completion.sh')
 
   const d = (await readFile(p, 'utf8')).replace(/^#!.*?\n/, '')
   await new Promise((res, rej) => {
     let done = false
-    process.stdout.write(d, () => {
-      if (done)
-        return
-
-      done = true
-      res()
-    })
-
     process.stdout.on('error', er => {
       if (done)
         return
@@ -224,11 +212,21 @@ const dumpScript = async () => {
       // Really, one should not be tossing away EPIPE errors, or any
       // errors, so casually.  But, without this, `. <(npm completion)`
       // can never ever work on OS X.
+      // TODO Ignoring coverage, see 'non EPIPE errors cause failures' test.
+      /* istanbul ignore next */
       if (er.errno === 'EPIPE')
         res()
       else
         rej(er)
     })
+
+    process.stdout.write(d, () => {
+      if (done)
+        return
+
+      done = true
+      res()
+    })
   })
 }