Refactor + display changes

.eslintrc.local.js

@@ -25,8 +25,8 @@ module.exports = {
   overrides: es6Files({
     'index.js': ['lib/cli.js'],
     'bin/npm-cli.js': ['lib/cli.js'],
-    'lib/cli.js': ['lib/es6/validate-engines.js'],
-    'lib/es6/validate-engines.js': ['package.json'],
+    'lib/cli.js': ['lib/cli/validate-engines.js'],
+    'lib/cli/validate-engines.js': ['package.json'],
     // TODO: This file should also have its requires restricted as well since it
     // is an entry point but it currently pulls in config definitions which have
     // a large require graph, so that is not currently feasible. A future config

lib/arborist-cmd.js

@@ -1,10 +1,9 @@
 const { log } = require('proc-log')
+const BaseCommand = require('./base-cmd.js')
 
 // This is the base for all commands whose execWorkspaces just gets
 // 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')
 class ArboristCmd extends BaseCommand {
   get isArboristCmd () {
     return true

lib/base-command.js → lib/base-cmd.js

@@ -167,7 +167,7 @@ class BaseCommand {
     const relativeFrom = prefixInsideCwd ? this.npm.localPrefix : process.cwd()
 
     const filters = this.npm.config.get('workspace')
-    const getWorkspaces = require('./workspaces/get-workspaces.js')
+    const getWorkspaces = require('./utils/get-workspaces.js')
     const ws = await getWorkspaces(filters, {
       path: this.npm.localPrefix,
       includeWorkspaceRoot,
@@ -179,4 +179,5 @@ class BaseCommand {
     this.workspacePaths = [...ws.values()]
   }
 }
+
 module.exports = BaseCommand

lib/cli.js

@@ -1,4 +1,4 @@
-const validateEngines = require('./es6/validate-engines.js')
-const cliEntry = require('node:path').resolve(__dirname, 'cli-entry.js')
+const validateEngines = require('./cli/validate-engines.js')
+const cliEntry = require('node:path').resolve(__dirname, 'cli/entry.js')
 
 module.exports = (process) => validateEngines(process, () => require(cliEntry))

lib/cli-entry.js → lib/cli/entry.js

@@ -11,9 +11,12 @@ module.exports = async (process, validateEngines) => {
     process.argv.splice(1, 1, 'npm', '-g')
   }
 
+  // Patch the global fs module here at the app level
+  require('graceful-fs').gracefulify(require('fs'))
+
   const satisfies = require('semver/functions/satisfies')
-  const exitHandler = require('./utils/exit-handler.js')
-  const Npm = require('./npm.js')
+  const exitHandler = require('./exit-handler.js')
+  const Npm = require('../npm.js')
   const npm = new Npm()
   exitHandler.setNpm(npm)
 
@@ -33,38 +36,51 @@ module.exports = async (process, validateEngines) => {
     log.warn('cli', validateEngines.unsupportedMessage)
   }
 
-  let cmd
   // Now actually fire up npm and run the command.
   // This is how to use npm programmatically:
   try {
-    await npm.load()
+    const { exec } = await npm.load()
 
-    // npm -v
-    if (npm.config.get('version', 'cli')) {
-      output.standard(npm.version)
+    if (!exec) {
       return exitHandler()
     }
 
-    // npm --versions
-    if (npm.config.get('versions', 'cli')) {
-      npm.argv = ['version']
-      npm.config.set('usage', false, 'cli')
-    }
+    const command = npm.argv.shift()
+    const args = npm.argv
 
-    cmd = npm.argv.shift()
-    if (!cmd) {
+    if (!command) {
       output.standard(npm.usage)
       process.exitCode = 1
       return exitHandler()
     }
 
-    await npm.exec(cmd)
+    // Options are prefixed by a hyphen-minus (-, \u2d).
+    // Other dash-type chars look similar but are invalid.
+    const nonDashArgs = npm.argv.filter(a => /^[\u2010-\u2015\u2212\uFE58\uFE63\uFF0D]/.test(a))
+    if (nonDashArgs.length) {
+      log.error(
+        'arg',
+        'Argument starts with non-ascii dash, this is probably invalid:',
+        require('@npmcli/redact').redactLog(nonDashArgs.join(', '))
+      )
+    }
+
+    const execPromise = npm.exec(command, args)
+
+    // this is async but we dont await it, since its ok if it doesnt
+    // finish before the command finishes running. it uses command and argv
+    // so it must be initiated here, after the command name is set
+    const updateNotifier = require('./update-notifier.js')
+    // eslint-disable-next-line promise/catch-or-return
+    updateNotifier(npm).then((msg) => (npm.updateNotification = msg))
+
+    await execPromise
     return exitHandler()
   } catch (err) {
     if (err.code === 'EUNKNOWNCOMMAND') {
-      const didYouMean = require('./utils/did-you-mean.js')
-      const suggestions = await didYouMean(npm.localPrefix, cmd)
-      output.standard(`Unknown command: "${cmd}"${suggestions}\n`)
+      const didYouMean = require('../utils/did-you-mean.js')
+      const suggestions = await didYouMean(npm.localPrefix, err.command)
+      output.standard(`Unknown command: "${err.command}"${suggestions}\n`)
       output.standard('To see a list of supported npm commands, run:\n  npm help')
       process.exitCode = 1
       return exitHandler()

lib/utils/exit-handler.js → lib/cli/exit-handler.js

@@ -1,25 +1,14 @@
-const { log, output, time } = require('proc-log')
-const errorMessage = require('./error-message.js')
+const { log, output, META } = require('proc-log')
+const errorMessage = require('../utils/error-message.js')
 const { redactLog: replaceInfo } = require('@npmcli/redact')
 
 let npm = null // set by the cli
 let exitHandlerCalled = false
 let showLogFileError = false
 
 process.on('exit', code => {
-  // process.emit is synchronous, so the timeEnd handler will run before the
-  // unfinished timer check below
-  time.end('npm')
-
   const hasLoadedNpm = npm?.config.loaded
 
-  // Unfinished timers can be read before config load
-  if (npm) {
-    for (const [name, timer] of npm.unfinishedTimers) {
-      log.silly('unfinished npm timer', name, timer)
-    }
-  }
-
   if (!code) {
     log.info('ok')
   } else {
@@ -31,64 +20,14 @@ process.on('exit', code => {
     log.error('', 'Exit handler never called!')
     log.error('', 'This is an error with npm itself. Please report this error at:')
     log.error('', '    <https://github.com/npm/cli/issues>')
-
-    // This gets logged regardless of silent mode
     // eslint-disable-next-line no-console
     console.error('')
-
     showLogFileError = true
   }
 
   // npm must be loaded to know where the log file was written
   if (hasLoadedNpm) {
-    // write the timing file now, this might do nothing based on the configs set.
-    // we need to call it here in case it errors so we dont tell the user
-    // about a timing file that doesn't exist
-    npm.writeTimingFile()
-
-    const logsDir = npm.logsDir
-    const logFiles = npm.logFiles
-
-    const timingDir = npm.timingDir
-    const timingFile = npm.timingFile
-
-    const timing = npm.config.get('timing')
-    const logsMax = npm.config.get('logs-max')
-
-    // Determine whether to show log file message and why it is
-    // being shown since in timing mode we always show the log file message
-    const logMethod = showLogFileError ? 'error' : timing ? 'info' : null
-
-    if (logMethod) {
-      // just a line break, will be ignored in silent mode
-      output.error('')
-
-      const message = []
-
-      if (timingFile) {
-        message.push(`Timing info written to: ${timingFile}`)
-      } else if (timing) {
-        message.push(
-          `The timing file was not written due to an error writing to the directory: ${timingDir}`
-        )
-      }
-
-      if (logFiles.length) {
-        message.push(`A complete log of this run can be found in: ${logFiles}`)
-      } else if (logsMax <= 0) {
-        // user specified no log file
-        message.push(`Log files were not written due to the config logs-max=${logsMax}`)
-      } else {
-        // could be an error writing to the directory
-        message.push(
-          `Log files were not written due to an error writing to the directory: ${logsDir}`,
-          'You can rerun the command with `--loglevel=verbose` to see the logs in your terminal'
-        )
-      }
-
-      log[logMethod]('', message.join('\n'))
-    }
-
+    npm.finish({ showLogFileError })
     // This removes any listeners npm setup, mostly for tests to avoid max listener warnings
     npm.unload()
   }
@@ -120,8 +59,7 @@ const exitHandler = err => {
 
   // only show the notification if it finished.
   if (typeof npm.updateNotification === 'string') {
-    // TODO: use proc-log to send `force: true` along with event
-    npm.forceLog('notice', '', npm.updateNotification)
+    log.notice('', npm.updateNotification, { [META]: true, force: true })
   }
 
   let exitCode = process.exitCode || 0
@@ -200,8 +138,7 @@ const exitHandler = err => {
   }
 
   if (hasLoadedNpm) {
-    // TODO: use proc-log.output.flush() once it exists
-    npm.flushOutput(jsonError)
+    output.flush({ [META]: true, jsonError })
   }
 
   log.verbose('exit', exitCode || 0)

lib/commands/access.js

@@ -3,10 +3,9 @@ const npa = require('npm-package-arg')
 const { output } = require('proc-log')
 const pkgJson = require('@npmcli/package-json')
 const localeCompare = require('@isaacs/string-locale-compare')('en')
-
 const otplease = require('../utils/otplease.js')
 const getIdentity = require('../utils/get-identity.js')
-const BaseCommand = require('../base-command.js')
+const BaseCommand = require('../base-cmd.js')
 
 const commands = [
   'get',

lib/commands/adduser.js

@@ -1,8 +1,7 @@
 const { log, output } = require('proc-log')
 const { redactLog: replaceInfo } = require('@npmcli/redact')
 const auth = require('../utils/auth.js')
-
-const BaseCommand = require('../base-command.js')
+const BaseCommand = require('../base-cmd.js')
 
 class AddUser extends BaseCommand {
   static description = 'Add a registry user account'
@@ -47,4 +46,5 @@ class AddUser extends BaseCommand {
     output.standard(message)
   }
 }
+
 module.exports = AddUser