diff --git a/.eslintrc.local.js b/.eslintrc.local.js index 6d94f64614fb8..2dce9d2badc08 100644 --- a/.eslintrc.local.js +++ b/.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 diff --git a/lib/cli.js b/lib/cli.js index 84d5471ad5e91..e11729fe3205b 100644 --- a/lib/cli.js +++ b/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)) diff --git a/lib/cli-entry.js b/lib/cli/entry.js similarity index 71% rename from lib/cli-entry.js rename to lib/cli/entry.js index 9a5994aec19a3..1c5f7e0dc1545 100644 --- a/lib/cli-entry.js +++ b/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) @@ -58,11 +61,29 @@ module.exports = async (process, validateEngines) => { return exitHandler() } + // 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)) + + // 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:', + nonDashArgs.join(', ') + ) + } + await npm.exec(cmd) return exitHandler() } catch (err) { if (err.code === 'EUNKNOWNCOMMAND') { - const didYouMean = require('./utils/did-you-mean.js') + const didYouMean = require('../utils/did-you-mean.js') const suggestions = await didYouMean(npm.localPrefix, cmd) output.standard(`Unknown command: "${cmd}"${suggestions}\n`) output.standard('To see a list of supported npm commands, run:\n npm help') diff --git a/lib/utils/exit-handler.js b/lib/cli/exit-handler.js similarity index 99% rename from lib/utils/exit-handler.js rename to lib/cli/exit-handler.js index 76d7d6036e072..6dc44de1f003a 100644 --- a/lib/utils/exit-handler.js +++ b/lib/cli/exit-handler.js @@ -1,5 +1,5 @@ const { log, output, time } = require('proc-log') -const errorMessage = require('./error-message.js') +const errorMessage = require('../utils/error-message.js') const { redactLog: replaceInfo } = require('@npmcli/redact') let npm = null // set by the cli diff --git a/lib/utils/update-notifier.js b/lib/cli/update-notifier.js similarity index 100% rename from lib/utils/update-notifier.js rename to lib/cli/update-notifier.js diff --git a/lib/es6/validate-engines.js b/lib/cli/validate-engines.js similarity index 100% rename from lib/es6/validate-engines.js rename to lib/cli/validate-engines.js diff --git a/lib/npm.js b/lib/npm.js index e76317e905ea3..d52f8bdc1c480 100644 --- a/lib/npm.js +++ b/lib/npm.js @@ -2,10 +2,6 @@ const { resolve, dirname, join } = require('node:path') const Config = require('@npmcli/config') const which = require('which') const fs = require('node:fs/promises') - -// Patch the global fs module here at the app level -require('graceful-fs').gracefulify(require('fs')) - const { definitions, flatten, shorthands } = require('@npmcli/config/lib/definitions') const usage = require('./utils/npm-usage.js') const LogFile = require('./utils/log-file.js') @@ -13,7 +9,6 @@ const Timers = require('./utils/timers.js') const Display = require('./utils/display.js') const { log, time } = require('proc-log') const { redactLog: replaceInfo } = require('@npmcli/redact') -const updateNotifier = require('./utils/update-notifier.js') const pkg = require('../package.json') const { deref } = require('./utils/cmd-list.js') @@ -42,7 +37,6 @@ class Npm { #title = 'npm' #argvClean = [] #npmRoot = null - #warnedNonDashArg = false #chalk = null #logChalk = null @@ -109,30 +103,7 @@ class Npm { // passed in directly. async exec (cmd, args = this.argv) { const command = this.setCmd(cmd) - - const timeEnd = time.start(`command:${cmd}`) - - // 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 - // eslint-disable-next-line promise/catch-or-return - updateNotifier(this).then((msg) => (this.updateNotification = msg)) - - // Options are prefixed by a hyphen-minus (-, \u2d). - // Other dash-type chars look similar but are invalid. - if (!this.#warnedNonDashArg) { - const nonDashArgs = args.filter(a => /^[\u2010-\u2015\u2212\uFE58\uFE63\uFF0D]/.test(a)) - if (nonDashArgs.length) { - this.#warnedNonDashArg = true - log.error( - 'arg', - 'Argument starts with non-ascii dash, this is probably invalid:', - nonDashArgs.join(', ') - ) - } - } - - return command.cmdExec(args).finally(timeEnd) + return time.start(`command:${cmd}`, () => command.cmdExec(args)) } async load () { diff --git a/smoke-tests/test/npm-replace-global.js b/smoke-tests/test/npm-replace-global.js index 5b84daf356ce2..c0accf6ba53b5 100644 --- a/smoke-tests/test/npm-replace-global.js +++ b/smoke-tests/test/npm-replace-global.js @@ -190,7 +190,7 @@ t.test('fail when updating with lazy require', async t => { // exit-handler is the last thing called in the code // so an uncached lazy require within the exit handler will always throw await fs.writeFile( - join(paths.globalNodeModules, 'npm/lib/utils/exit-handler.js'), + join(paths.globalNodeModules, 'npm/lib/cli/exit-handler.js'), `module.exports = () => require('./LAZY_REQUIRE_CANARY');module.exports.setNpm = () => {}`, 'utf-8' ) diff --git a/tap-snapshots/test/lib/utils/update-notifier.js.test.cjs b/tap-snapshots/test/lib/cli/update-notifier.js.test.cjs similarity index 64% rename from tap-snapshots/test/lib/utils/update-notifier.js.test.cjs rename to tap-snapshots/test/lib/cli/update-notifier.js.test.cjs index 5693b3f3a9097..244d5216340f8 100644 --- a/tap-snapshots/test/lib/utils/update-notifier.js.test.cjs +++ b/tap-snapshots/test/lib/cli/update-notifier.js.test.cjs @@ -5,7 +5,7 @@ * Make sure to inspect the output below. Do not ignore changes! */ 'use strict' -exports[`test/lib/utils/update-notifier.js TAP notification situations 122.420.69 - color=always > must match snapshot 1`] = ` +exports[`test/lib/cli/update-notifier.js TAP notification situations 122.420.69 - color=always > must match snapshot 1`] = ` New major version of npm available! 122.420.69 -> 123.420.69 Changelog: https://github.com/npm/cli/releases/tag/v123.420.69 @@ -13,7 +13,7 @@ To update run: npm install -g npm@123.420.69 ` -exports[`test/lib/utils/update-notifier.js TAP notification situations 122.420.69 - color=false > must match snapshot 1`] = ` +exports[`test/lib/cli/update-notifier.js TAP notification situations 122.420.69 - color=false > must match snapshot 1`] = ` New major version of npm available! 122.420.69 -> 123.420.69 Changelog: https://github.com/npm/cli/releases/tag/v123.420.69 @@ -21,7 +21,7 @@ To update run: npm install -g npm@123.420.69 ` -exports[`test/lib/utils/update-notifier.js TAP notification situations 123.419.69 - color=always > must match snapshot 1`] = ` +exports[`test/lib/cli/update-notifier.js TAP notification situations 123.419.69 - color=always > must match snapshot 1`] = ` New minor version of npm available! 123.419.69 -> 123.420.69 Changelog: https://github.com/npm/cli/releases/tag/v123.420.69 @@ -29,7 +29,7 @@ To update run: npm install -g npm@123.420.69 ` -exports[`test/lib/utils/update-notifier.js TAP notification situations 123.419.69 - color=false > must match snapshot 1`] = ` +exports[`test/lib/cli/update-notifier.js TAP notification situations 123.419.69 - color=false > must match snapshot 1`] = ` New minor version of npm available! 123.419.69 -> 123.420.69 Changelog: https://github.com/npm/cli/releases/tag/v123.420.69 @@ -37,7 +37,7 @@ To update run: npm install -g npm@123.420.69 ` -exports[`test/lib/utils/update-notifier.js TAP notification situations 123.420.68 - color=always > must match snapshot 1`] = ` +exports[`test/lib/cli/update-notifier.js TAP notification situations 123.420.68 - color=always > must match snapshot 1`] = ` New patch version of npm available! 123.420.68 -> 123.420.69 Changelog: https://github.com/npm/cli/releases/tag/v123.420.69 @@ -45,7 +45,7 @@ To update run: npm install -g npm@123.420.69 ` -exports[`test/lib/utils/update-notifier.js TAP notification situations 123.420.68 - color=false > must match snapshot 1`] = ` +exports[`test/lib/cli/update-notifier.js TAP notification situations 123.420.68 - color=false > must match snapshot 1`] = ` New patch version of npm available! 123.420.68 -> 123.420.69 Changelog: https://github.com/npm/cli/releases/tag/v123.420.69 @@ -53,7 +53,7 @@ To update run: npm install -g npm@123.420.69 ` -exports[`test/lib/utils/update-notifier.js TAP notification situations 123.420.70 - color=always > must match snapshot 1`] = ` +exports[`test/lib/cli/update-notifier.js TAP notification situations 123.420.70 - color=always > must match snapshot 1`] = ` New minor version of npm available! 123.420.70 -> 123.421.70 Changelog: https://github.com/npm/cli/releases/tag/v123.421.70 @@ -61,7 +61,7 @@ To update run: npm install -g npm@123.421.70 ` -exports[`test/lib/utils/update-notifier.js TAP notification situations 123.420.70 - color=false > must match snapshot 1`] = ` +exports[`test/lib/cli/update-notifier.js TAP notification situations 123.420.70 - color=false > must match snapshot 1`] = ` New minor version of npm available! 123.420.70 -> 123.421.70 Changelog: https://github.com/npm/cli/releases/tag/v123.421.70 @@ -69,7 +69,7 @@ To update run: npm install -g npm@123.421.70 ` -exports[`test/lib/utils/update-notifier.js TAP notification situations 123.421.69 - color=always > must match snapshot 1`] = ` +exports[`test/lib/cli/update-notifier.js TAP notification situations 123.421.69 - color=always > must match snapshot 1`] = ` New patch version of npm available! 123.421.69 -> 123.421.70 Changelog: https://github.com/npm/cli/releases/tag/v123.421.70 @@ -77,7 +77,7 @@ To update run: npm install -g npm@123.421.70 ` -exports[`test/lib/utils/update-notifier.js TAP notification situations 123.421.69 - color=false > must match snapshot 1`] = ` +exports[`test/lib/cli/update-notifier.js TAP notification situations 123.421.69 - color=false > must match snapshot 1`] = ` New patch version of npm available! 123.421.69 -> 123.421.70 Changelog: https://github.com/npm/cli/releases/tag/v123.421.70 @@ -85,7 +85,7 @@ To update run: npm install -g npm@123.421.70 ` -exports[`test/lib/utils/update-notifier.js TAP notification situations 124.0.0-beta.0 - color=always > must match snapshot 1`] = ` +exports[`test/lib/cli/update-notifier.js TAP notification situations 124.0.0-beta.0 - color=always > must match snapshot 1`] = ` New prerelease version of npm available! 124.0.0-beta.0 -> 124.0.0-beta.99999 Changelog: https://github.com/npm/cli/releases/tag/v124.0.0-beta.99999 @@ -93,7 +93,7 @@ To update run: npm install -g npm@124.0.0-beta.99999 ` -exports[`test/lib/utils/update-notifier.js TAP notification situations 124.0.0-beta.0 - color=false > must match snapshot 1`] = ` +exports[`test/lib/cli/update-notifier.js TAP notification situations 124.0.0-beta.0 - color=false > must match snapshot 1`] = ` New prerelease version of npm available! 124.0.0-beta.0 -> 124.0.0-beta.99999 Changelog: https://github.com/npm/cli/releases/tag/v124.0.0-beta.99999 diff --git a/test/fixtures/mock-npm.js b/test/fixtures/mock-npm.js index 6bb3123d2b091..466b9b2a0ba46 100644 --- a/test/fixtures/mock-npm.js +++ b/test/fixtures/mock-npm.js @@ -48,17 +48,13 @@ const setGlobalNodeModules = (globalDir) => { } const buildMocks = (t, mocks) => { - const allMocks = { - '{LIB}/utils/update-notifier.js': async () => {}, - ...mocks, - } + const allMocks = { ...mocks } // The definitions must be mocked since they are a singleton that reads from // process and environs to build defaults in order to break the requiure // cache. We also need to mock them with any mocks that were passed in for the // test in case those mocks are for things like ci-info which is used there. const definitions = '@npmcli/config/lib/definitions' allMocks[definitions] = tmock(t, definitions, allMocks) - return allMocks } diff --git a/test/lib/cli.js b/test/lib/cli.js index a6cb576e886ee..6c3079b8efe31 100644 --- a/test/lib/cli.js +++ b/test/lib/cli.js @@ -3,7 +3,7 @@ const tmock = require('../fixtures/tmock') t.test('returns cli-entry function', async t => { const cli = tmock(t, '{LIB}/cli.js', { - '{LIB}/cli-entry.js': () => 'ENTRY', + '{LIB}/cli/entry.js': () => 'ENTRY', }) t.equal(cli(process), 'ENTRY') diff --git a/test/lib/cli-entry.js b/test/lib/cli/entry.js similarity index 88% rename from test/lib/cli-entry.js rename to test/lib/cli/entry.js index 3a8d3321c4b64..66a417e7d269a 100644 --- a/test/lib/cli-entry.js +++ b/test/lib/cli/entry.js @@ -1,7 +1,7 @@ const t = require('tap') -const { load: loadMockNpm } = require('../fixtures/mock-npm.js') -const tmock = require('../fixtures/tmock.js') -const validateEngines = require('../../lib/es6/validate-engines.js') +const { load: loadMockNpm } = require('../../fixtures/mock-npm.js') +const tmock = require('../../fixtures/tmock.js') +const validateEngines = require('../../../lib/cli/validate-engines.js') const cliMock = async (t, opts) => { let exitHandlerArgs = null @@ -13,9 +13,9 @@ const cliMock = async (t, opts) => { exitHandlerMock.setNpm = _npm => npm = _npm const { Npm, ...mock } = await loadMockNpm(t, { ...opts, init: false }) - const cli = tmock(t, '{LIB}/cli-entry.js', { + const cli = tmock(t, '{LIB}/cli/entry.js', { '{LIB}/npm.js': Npm, - '{LIB}/utils/exit-handler.js': exitHandlerMock, + '{LIB}/cli/exit-handler.js': exitHandlerMock, }) return { @@ -159,3 +159,16 @@ t.test('unsupported node version', async t => { /npm v.* does not support Node\.js 12\.6\.0\./ ) }) + +t.test('non-ascii dash', async t => { + const { cli, logs } = await cliMock(t, { + globals: { + 'process.argv': ['node', 'npm', 'scope', '\u2010not-a-dash'], + }, + }) + await cli(process) + t.equal( + logs.error[0], + 'arg Argument starts with non-ascii dash, this is probably invalid: \u2010not-a-dash' + ) +}) diff --git a/test/lib/utils/update-notifier.js b/test/lib/cli/update-notifier.js similarity index 99% rename from test/lib/utils/update-notifier.js rename to test/lib/cli/update-notifier.js index 052367c60dadb..2d29868b792a1 100644 --- a/test/lib/utils/update-notifier.js +++ b/test/lib/cli/update-notifier.js @@ -83,7 +83,7 @@ const runUpdateNotifier = async (t, { prefixDir, argv, }) - const updateNotifier = tmock(t, '{LIB}/utils/update-notifier.js', mocks) + const updateNotifier = tmock(t, '{LIB}/cli/update-notifier.js', mocks) const result = await updateNotifier(mock.npm) diff --git a/test/lib/es6/validate-engines.js b/test/lib/cli/validate-engines.js similarity index 94% rename from test/lib/es6/validate-engines.js rename to test/lib/cli/validate-engines.js index 0e6bce726af96..1c0b59700a773 100644 --- a/test/lib/es6/validate-engines.js +++ b/test/lib/cli/validate-engines.js @@ -3,7 +3,7 @@ const mockGlobals = require('@npmcli/mock-globals') const tmock = require('../../fixtures/tmock') const mockValidateEngines = (t) => { - const validateEngines = tmock(t, '{LIB}/es6/validate-engines.js', { + const validateEngines = tmock(t, '{LIB}/cli/validate-engines.js', { '{ROOT}/package.json': { version: '1.2.3', engines: { node: '>=0' } }, }) mockGlobals(t, { 'process.version': 'v4.5.6' }) diff --git a/test/lib/npm.js b/test/lib/npm.js index a0dec04caa137..bc9042365abea 100644 --- a/test/lib/npm.js +++ b/test/lib/npm.js @@ -178,17 +178,16 @@ t.test('npm.load', async t => { outputs.length = 0 logs.length = 0 - await npm.exec('get', ['scope', '\u2010not-a-dash']) + await npm.exec('get', ['scope', 'usage']) t.strictSame([npm.command, npm.flatOptions.npmCommand], ['ll', 'll'], 'does not change npm.command when another command is called') t.match(logs, [ - 'error arg Argument starts with non-ascii dash, this is probably invalid: \u2010not-a-dash', /timing command:config Completed in [0-9.]+ms/, /timing command:get Completed in [0-9.]+ms/, ]) - t.same(outputs, ['scope=@foo\n\u2010not-a-dash=undefined']) + t.same(outputs, ['scope=@foo\nusage=false']) }) await t.test('--no-workspaces with --workspace', async t => { diff --git a/test/lib/utils/exit-handler.js b/test/lib/utils/exit-handler.js index af92611de0594..b1f4d9c272533 100644 --- a/test/lib/utils/exit-handler.js +++ b/test/lib/utils/exit-handler.js @@ -72,7 +72,7 @@ const mockExitHandler = async (t, { config, mocks, files, ...opts } = {}) => { }, }) - const exitHandler = tmock(t, '{LIB}/utils/exit-handler.js', { + const exitHandler = tmock(t, '{LIB}/cli/exit-handler.js', { '{LIB}/utils/error-message.js': (err) => ({ summary: [['ERR SUMMARY', err.message]], detail: [['ERR DETAIL', err.message]], diff --git a/test/lib/utils/installed-deep.js b/test/lib/utils/installed-deep.js index 4d212746cba89..20e001aaec751 100644 --- a/test/lib/utils/installed-deep.js +++ b/test/lib/utils/installed-deep.js @@ -1,6 +1,6 @@ const t = require('tap') const installedDeep = require('../../../lib/utils/installed-deep.js') -const mockNpm = require('../../../fixtures/mock-npm') +const mockNpm = require('../../fixtures/mock-npm') const fixture = { 'package.json': JSON.stringify({ diff --git a/test/lib/utils/installed-shallow.js b/test/lib/utils/installed-shallow.js index 43ccda4daea69..67b49292a64c7 100644 --- a/test/lib/utils/installed-shallow.js +++ b/test/lib/utils/installed-shallow.js @@ -1,6 +1,6 @@ const t = require('tap') const installed = require('../../../lib/utils/installed-shallow.js') -const mockNpm = require('../../../fixtures/mock-npm') +const mockNpm = require('../../fixtures/mock-npm') const mockShallow = async (t, config) => { const res = await mockNpm(t, {