fix(npm) pass npm context everywhere

Instead of files randomly requiring the npm singleton,
we pass it where it needs to go so that tests don't need
to do so much require mocking everywhere

PR-URL: https://github.com/npm/cli/pull/2772
Credit: @wraithgar
Close: #2772
Reviewed-by: @ruyadorno

1 files changed, 231 insertions, 220 deletions

diff --git a/lib/exec.js b/lib/exec.js
index dab65c23a..d1db49128 100644
--- a/lib/exec.js
+++ b/lib/exec.js
@@ -1,28 +1,18 @@
-const npm = require('./npm.js')
 const output = require('./utils/output.js')
 const usageUtil = require('./utils/usage.js')
-const usage = usageUtil('exec',
-  'Run a command from a local or remote npm package.\n\n' +
-
-  'npm exec -- <pkg>[@<version>] [args...]\n' +
-  'npm exec --package=<pkg>[@<version>] -- <cmd> [args...]\n' +
-  'npm exec -c \'<cmd> [args...]\'\n' +
-  'npm exec --package=foo -c \'<cmd> [args...]\'\n' +
-  '\n' +
-  'npx <pkg>[@<specifier>] [args...]\n' +
-  'npx -p <pkg>[@<specifier>] <cmd> [args...]\n' +
-  'npx -c \'<cmd> [args...]\'\n' +
-  'npx -p <pkg>[@<specifier>] -c \'<cmd> [args...]\'' +
-  '\n' +
-  'Run without --call or positional args to open interactive subshell\n',
-
-  '\n--package=<pkg> (may be specified multiple times)\n' +
-  '-p is a shorthand for --package only when using npx executable\n' +
-  '-c <cmd> --call=<cmd> (may not be mixed with positional arguments)'
-)
-
 const { promisify } = require('util')
 const read = promisify(require('read'))
+const mkdirp = require('mkdirp-infer-owner')
+const readPackageJson = require('read-package-json-fast')
+const Arborist = require('@npmcli/arborist')
+const runScript = require('@npmcli/run-script')
+const { resolve, delimiter } = require('path')
+const ciDetect = require('@npmcli/ci-detect')
+const crypto = require('crypto')
+const pacote = require('pacote')
+const npa = require('npm-package-arg')
+const fileExists = require('./utils/file-exists.js')
+const PATH = require('./utils/path.js')
 
 // it's like this:
 //
@@ -49,237 +39,258 @@ const read = promisify(require('read'))
 // runScript({ pkg, event: 'npx', ... })
 // process.env.npm_lifecycle_event = 'npx'
 
-const mkdirp = require('mkdirp-infer-owner')
-const readPackageJson = require('read-package-json-fast')
-const Arborist = require('@npmcli/arborist')
-const runScript = require('@npmcli/run-script')
-const { resolve, delimiter } = require('path')
-const ciDetect = require('@npmcli/ci-detect')
-const crypto = require('crypto')
-const pacote = require('pacote')
-const npa = require('npm-package-arg')
-const fileExists = require('./utils/file-exists.js')
-const PATH = require('./utils/path.js')
-
-const cmd = (args, cb) => exec(args).then(() => cb()).catch(cb)
-
-const run = async ({ args, call, pathArr, shell }) => {
-  // turn list of args into command string
-  const script = call || args.shift() || shell
-
-  // do the fakey runScript dance
-  // still should work if no package.json in cwd
-  const realPkg = await readPackageJson(`${npm.localPrefix}/package.json`)
-    .catch(() => ({}))
-  const pkg = {
-    ...realPkg,
-    scripts: {
-      ...(realPkg.scripts || {}),
-      npx: script,
-    },
+class Exec {
+  constructor (npm) {
+    this.npm = npm
   }
 
-  npm.log.disableProgress()
-  try {
-    if (script === shell) {
-      if (process.stdin.isTTY) {
-        if (ciDetect())
-          return npm.log.warn('exec', 'Interactive mode disabled in CI environment')
-        output(`\nEntering npm script environment\nType 'exit' or ^D when finished\n`)
-      }
-    }
-    return await runScript({
-      ...npm.flatOptions,
-      pkg,
-      banner: false,
-      // we always run in cwd, not --prefix
-      path: process.cwd(),
-      stdioString: true,
-      event: 'npx',
-      args,
-      env: {
-        PATH: pathArr.join(delimiter),
-      },
-      stdio: 'inherit',
-    })
-  } finally {
-    npm.log.enableProgress()
+  get usage () {
+    return usageUtil('exec',
+      'Run a command from a local or remote npm package.\n\n' +
+
+      'npm exec -- <pkg>[@<version>] [args...]\n' +
+      'npm exec --package=<pkg>[@<version>] -- <cmd> [args...]\n' +
+      'npm exec -c \'<cmd> [args...]\'\n' +
+      'npm exec --package=foo -c \'<cmd> [args...]\'\n' +
+      '\n' +
+      'npx <pkg>[@<specifier>] [args...]\n' +
+      'npx -p <pkg>[@<specifier>] <cmd> [args...]\n' +
+      'npx -c \'<cmd> [args...]\'\n' +
+      'npx -p <pkg>[@<specifier>] -c \'<cmd> [args...]\'' +
+      '\n' +
+      'Run without --call or positional args to open interactive subshell\n',
+
+      '\n--package=<pkg> (may be specified multiple times)\n' +
+      '-p is a shorthand for --package only when using npx executable\n' +
+      '-c <cmd> --call=<cmd> (may not be mixed with positional arguments)'
+    )
   }
-}
-
-const exec = async args => {
-  const { package: packages, call, shell } = npm.flatOptions
 
-  if (call && args.length)
-    throw usage
+  exec (args, cb) {
+    this._exec(args).then(() => cb()).catch(cb)
+  }
 
-  const pathArr = [...PATH]
+  // When commands go async and we can dump the boilerplate exec methods this
+  // can be named correctly
+  async _exec (args) {
+    const { package: packages, call, shell } = this.npm.flatOptions
 
-  // nothing to maybe install, skip the arborist dance
-  if (!call && !args.length && !packages.length) {
-    return await run({
-      args,
-      call,
-      shell,
-      pathArr,
-    })
-  }
+    if (call && args.length)
+      throw this.usage
 
-  const needPackageCommandSwap = args.length && !packages.length
-  // if there's an argument and no package has been explicitly asked for
-  // check the local and global bin paths for a binary named the same as
-  // the argument and run it if it exists, otherwise fall through to
-  // the behavior of treating the single argument as a package name
-  if (needPackageCommandSwap) {
-    let binExists = false
-    if (await fileExists(`${npm.localBin}/${args[0]}`)) {
-      pathArr.unshift(npm.localBin)
-      binExists = true
-    } else if (await fileExists(`${npm.globalBin}/${args[0]}`)) {
-      pathArr.unshift(npm.globalBin)
-      binExists = true
-    }
+    const pathArr = [...PATH]
 
-    if (binExists) {
-      return await run({
+    // nothing to maybe install, skip the arborist dance
+    if (!call && !args.length && !packages.length) {
+      return await this.run({
         args,
         call,
-        pathArr,
         shell,
+        pathArr,
       })
     }
 
-    packages.push(args[0])
-  }
+    const needPackageCommandSwap = args.length && !packages.length
+    // if there's an argument and no package has been explicitly asked for
+    // check the local and global bin paths for a binary named the same as
+    // the argument and run it if it exists, otherwise fall through to
+    // the behavior of treating the single argument as a package name
+    if (needPackageCommandSwap) {
+      let binExists = false
+      if (await fileExists(`${this.npm.localBin}/${args[0]}`)) {
+        pathArr.unshift(this.npm.localBin)
+        binExists = true
+      } else if (await fileExists(`${this.npm.globalBin}/${args[0]}`)) {
+        pathArr.unshift(this.npm.globalBin)
+        binExists = true
+      }
 
-  // If we do `npm exec foo`, and have a `foo` locally, then we'll
-  // always use that, so we don't really need to fetch the manifest.
-  // So: run npa on each packages entry, and if it is a name with a
-  // rawSpec==='', then try to readPackageJson at
-  // node_modules/${name}/package.json, and only pacote fetch if
-  // that fails.
-  const manis = await Promise.all(packages.map(async p => {
-    const spec = npa(p, npm.localPrefix)
-    if (spec.type === 'tag' && spec.rawSpec === '') {
-      // fall through to the pacote.manifest() approach
-      try {
-        const pj = resolve(npm.localPrefix, 'node_modules', spec.name)
-        return await readPackageJson(pj)
-      } catch (er) {}
+      if (binExists) {
+        return await this.run({
+          args,
+          call,
+          pathArr,
+          shell,
+        })
+      }
+
+      packages.push(args[0])
     }
-    // Force preferOnline to true so we are making sure to pull in the latest
-    // This is especially useful if the user didn't give us a version, and
-    // they expect to be running @latest
-    return await pacote.manifest(p, {
-      ...npm.flatOptions,
-      preferOnline: true,
-    })
-  }))
-
-  if (needPackageCommandSwap)
-    args[0] = getBinFromManifest(manis[0])
-
-  // figure out whether we need to install stuff, or if local is fine
-  const localArb = new Arborist({
-    ...npm.flatOptions,
-    path: npm.localPrefix,
-  })
-  const tree = await localArb.loadActual()
-
-  // do we have all the packages in manifest list?
-  const needInstall = manis.some(mani => manifestMissing(tree, mani))
-
-  if (needInstall) {
-    const installDir = cacheInstallDir(packages)
-    await mkdirp(installDir)
-    const arb = new Arborist({ ...npm.flatOptions, path: installDir })
-    const tree = await arb.loadActual()
-
-    // at this point, we have to ensure that we get the exact same
-    // version, because it's something that has only ever been installed
-    // by npm exec in the cache install directory
-    const add = manis.filter(mani => manifestMissing(tree, {
-      ...mani,
-      _from: `${mani.name}@${mani.version}`,
+
+    // If we do `npm exec foo`, and have a `foo` locally, then we'll
+    // always use that, so we don't really need to fetch the manifest.
+    // So: run npa on each packages entry, and if it is a name with a
+    // rawSpec==='', then try to readPackageJson at
+    // node_modules/${name}/package.json, and only pacote fetch if
+    // that fails.
+    const manis = await Promise.all(packages.map(async p => {
+      const spec = npa(p, this.npm.localPrefix)
+      if (spec.type === 'tag' && spec.rawSpec === '') {
+        // fall through to the pacote.manifest() approach
+        try {
+          const pj = resolve(this.npm.localPrefix, 'node_modules', spec.name)
+          return await readPackageJson(pj)
+        } catch (er) {}
+      }
+      // Force preferOnline to true so we are making sure to pull in the latest
+      // This is especially useful if the user didn't give us a version, and
+      // they expect to be running @latest
+      return await pacote.manifest(p, {
+        ...this.npm.flatOptions,
+        preferOnline: true,
+      })
     }))
-      .map(mani => mani._from)
-      .sort((a, b) => a.localeCompare(b))
-
-    // no need to install if already present
-    if (add.length) {
-      if (!npm.flatOptions.yes) {
-        // set -n to always say no
-        if (npm.flatOptions.yes === false)
-          throw 'canceled'
-
-        if (!process.stdin.isTTY || ciDetect()) {
-          npm.log.warn('exec', `The following package${
+
+    if (needPackageCommandSwap)
+      args[0] = this.getBinFromManifest(manis[0])
+
+    // figure out whether we need to install stuff, or if local is fine
+    const localArb = new Arborist({
+      ...this.npm.flatOptions,
+      path: this.npm.localPrefix,
+    })
+    const tree = await localArb.loadActual()
+
+    // do we have all the packages in manifest list?
+    const needInstall = manis.some(mani => this.manifestMissing(tree, mani))
+
+    if (needInstall) {
+      const installDir = this.cacheInstallDir(packages)
+      await mkdirp(installDir)
+      const arb = new Arborist({ ...this.npm.flatOptions, path: installDir })
+      const tree = await arb.loadActual()
+
+      // at this point, we have to ensure that we get the exact same
+      // version, because it's something that has only ever been installed
+      // by npm exec in the cache install directory
+      const add = manis.filter(mani => this.manifestMissing(tree, {
+        ...mani,
+        _from: `${mani.name}@${mani.version}`,
+      }))
+        .map(mani => mani._from)
+        .sort((a, b) => a.localeCompare(b))
+
+      // no need to install if already present
+      if (add.length) {
+        if (!this.npm.flatOptions.yes) {
+          // set -n to always say no
+          if (this.npm.flatOptions.yes === false)
+            throw 'canceled'
+
+          if (!process.stdin.isTTY || ciDetect()) {
+            this.npm.log.warn('exec', `The following package${
             add.length === 1 ? ' was' : 's were'
           } not found and will be installed: ${
             add.map((pkg) => pkg.replace(/@$/, '')).join(', ')
           }`)
-        } else {
-          const addList = add.map(a => `  ${a.replace(/@$/, '')}`)
-            .join('\n') + '\n'
-          const prompt = `Need to install the following packages:\n${
+          } else {
+            const addList = add.map(a => `  ${a.replace(/@$/, '')}`)
+              .join('\n') + '\n'
+            const prompt = `Need to install the following packages:\n${
             addList
           }Ok to proceed? `
-          const confirm = await read({ prompt, default: 'y' })
-          if (confirm.trim().toLowerCase().charAt(0) !== 'y')
-            throw 'canceled'
+            const confirm = await read({ prompt, default: 'y' })
+            if (confirm.trim().toLowerCase().charAt(0) !== 'y')
+              throw 'canceled'
+          }
         }
+        await arb.reify({ ...this.npm.flatOptions, add })
       }
-      await arb.reify({ ...npm.flatOptions, add })
+      pathArr.unshift(resolve(installDir, 'node_modules/.bin'))
     }
-    pathArr.unshift(resolve(installDir, 'node_modules/.bin'))
+
+    return await this.run({ args, call, pathArr, shell })
   }
 
-  return await run({ args, call, pathArr, shell })
-}
+  async run ({ args, call, pathArr, shell }) {
+    // turn list of args into command string
+    const script = call || args.shift() || shell
+
+    // do the fakey runScript dance
+    // still should work if no package.json in cwd
+    const realPkg = await readPackageJson(`${this.npm.localPrefix}/package.json`)
+      .catch(() => ({}))
+    const pkg = {
+      ...realPkg,
+      scripts: {
+        ...(realPkg.scripts || {}),
+        npx: script,
+      },
+    }
 
-const manifestMissing = (tree, mani) => {
-  // if the tree doesn't have a child by that name/version, return true
-  // true means we need to install it
-  const child = tree.children.get(mani.name)
-  // if no child, we have to load it
-  if (!child)
-    return true
+    this.npm.log.disableProgress()
+    try {
+      if (script === shell) {
+        if (process.stdin.isTTY) {
+          if (ciDetect())
+            return this.npm.log.warn('exec', 'Interactive mode disabled in CI environment')
+          output(`\nEntering npm script environment\nType 'exit' or ^D when finished\n`)
+        }
+      }
+      return await runScript({
+        ...this.npm.flatOptions,
+        pkg,
+        banner: false,
+        // we always run in cwd, not --prefix
+        path: process.cwd(),
+        stdioString: true,
+        event: 'npx',
+        args,
+        env: {
+          PATH: pathArr.join(delimiter),
+        },
+        stdio: 'inherit',
+      })
+    } finally {
+      this.npm.log.enableProgress()
+    }
+  }
 
-  // if no version/tag specified, allow whatever's there
-  if (mani._from === `${mani.name}@`)
-    return false
+  manifestMissing (tree, mani) {
+    // if the tree doesn't have a child by that name/version, return true
+    // true means we need to install it
+    const child = tree.children.get(mani.name)
+    // if no child, we have to load it
+    if (!child)
+      return true
 
-  // otherwise the version has to match what we WOULD get
-  return child.version !== mani.version
-}
+    // if no version/tag specified, allow whatever's there
+    if (mani._from === `${mani.name}@`)
+      return false
 
-const getBinFromManifest = mani => {
-  // if we have a bin matching (unscoped portion of) packagename, use that
-  // otherwise if there's 1 bin or all bin value is the same (alias), use that,
-  // otherwise fail
-  const bin = mani.bin || {}
-  if (new Set(Object.values(bin)).size === 1)
-    return Object.keys(bin)[0]
-
-  // XXX probably a util to parse this better?
-  const name = mani.name.replace(/^@[^/]+\//, '')
-  if (bin[name])
-    return name
-
-  // XXX need better error message
-  throw Object.assign(new Error('could not determine executable to run'), {
-    pkgid: mani._id,
-  })
-}
+    // otherwise the version has to match what we WOULD get
+    return child.version !== mani.version
+  }
 
-// only packages not found in ${prefix}/node_modules
-const cacheInstallDir = packages =>
-  resolve(npm.config.get('cache'), '_npx', getHash(packages))
+  getBinFromManifest (mani) {
+    // if we have a bin matching (unscoped portion of) packagename, use that
+    // otherwise if there's 1 bin or all bin value is the same (alias), use
+    // that, otherwise fail
+    const bin = mani.bin || {}
+    if (new Set(Object.values(bin)).size === 1)
+      return Object.keys(bin)[0]
+
+    // XXX probably a util to parse this better?
+    const name = mani.name.replace(/^@[^/]+\//, '')
+    if (bin[name])
+      return name
+
+    // XXX need better error message
+    throw Object.assign(new Error('could not determine executable to run'), {
+      pkgid: mani._id,
+    })
+  }
 
-const getHash = packages =>
-  crypto.createHash('sha512')
-    .update(packages.sort((a, b) => a.localeCompare(b)).join('\n'))
-    .digest('hex')
-    .slice(0, 16)
+  cacheInstallDir (packages) {
+    // only packages not found in ${prefix}/node_modules
+    return resolve(this.npm.config.get('cache'), '_npx', this.getHash(packages))
+  }
 
-module.exports = Object.assign(cmd, { usage })
+  getHash (packages) {
+    return crypto.createHash('sha512')
+      .update(packages.sort((a, b) => a.localeCompare(b)).join('\n'))
+      .digest('hex')
+      .slice(0, 16)
+  }
+}
+module.exports = Exec