diff options
-rw-r--r-- | bin/npx-cli.js | 106 | ||||
-rw-r--r-- | docs/content/cli-commands/npm-exec.md | 33 | ||||
-rw-r--r-- | docs/content/cli-commands/npx.md | 33 | ||||
-rw-r--r-- | lib/config/defaults.js | 6 | ||||
-rw-r--r-- | lib/config/flat-options.js | 3 | ||||
-rw-r--r-- | lib/exec.js | 26 | ||||
-rw-r--r-- | tap-snapshots/test-lib-config-flat-options.js-TAP.test.js | 1 | ||||
-rw-r--r-- | test/bin/npx-cli.js | 68 | ||||
-rw-r--r-- | test/coverage-map.js | 2 | ||||
-rw-r--r-- | test/lib/exec.js | 212 |
10 files changed, 470 insertions, 20 deletions
diff --git a/bin/npx-cli.js b/bin/npx-cli.js index dc2072ffd..716dc958e 100644 --- a/bin/npx-cli.js +++ b/bin/npx-cli.js @@ -6,20 +6,114 @@ const cli = require('../lib/cli.js') process.argv[1] = require.resolve('./npm-cli.js') process.argv.splice(2, 0, 'exec') +// TODO: remove the affordances for removed items in npm v9 +const removedSwitches = new Set([ + 'always-spawn', + 'ignore-existing', + 'shell-auto-fallback' +]) + +const removedOpts = new Set([ + 'npm', + 'node-arg', + 'n' +]) + +const removed = new Set([ + ...removedSwitches, + ...removedOpts +]) + +const { types, shorthands } = require('../lib/config/defaults.js') +const npmSwitches = Object.entries(types) + .filter(([key, type]) => type === Boolean || + (Array.isArray(type) && type.includes(Boolean))) + .map(([key, type]) => key) + +// things that don't take a value +const switches = new Set([ + ...removedSwitches, + ...npmSwitches, + 'no-install', + 'quiet', + 'q', + 'version', + 'v', + 'help', + 'h' +]) + +// things that do take a value +const opts = new Set([ + ...removedOpts, + 'package', + 'p', + 'cache', + 'userconfig', + 'call', + 'c', + 'shell', + 'npm', + 'node-arg', + 'n' +]) + // break out of loop when we find a positional argument or -- // If we find a positional arg, we shove -- in front of it, and // let the normal npm cli handle the rest. let i +let sawRemovedFlags = false for (i = 3; i < process.argv.length; i++) { const arg = process.argv[i] if (arg === '--') { break } else if (/^-/.test(arg)) { - // `--package foo` treated the same as `--package=foo` - if (!arg.includes('=')) { - i++ + const [key, ...v] = arg.replace(/^-+/, '').split('=') + + switch (key) { + case 'p': + process.argv[i] = ['--package', ...v].join('=') + break + + case 'shell': + process.argv[i] = ['--script-shell', ...v].join('=') + break + + case 'no-install': + process.argv[i] = '--yes=false' + break + + default: + // resolve shorthands and run again + if (shorthands[key] && !removed.has(key)) { + const a = [...shorthands[key]] + if (v.length) { + a.push(v.join('=')) + } + process.argv.splice(i, 1, ...a) + i-- + continue + } + break + } + + if (removed.has(key)) { + console.error(`npx: the --${key} argument has been removed.`) + sawRemovedFlags = true + process.argv.splice(i, 1) + i-- + } + + if (v.length === 0 && !switches.has(key) && + (opts.has(key) || !/^-/.test(process.argv[i + 1]))) { + // value will be next argument, skip over it. + if (removed.has(key)) { + // also remove the value for the cut key. + process.argv.splice(i + 1, 1) + } else { + i++ + } } - continue } else { // found a positional arg, put -- in front of it, and we're done process.argv.splice(i, 0, '--') @@ -27,4 +121,8 @@ for (i = 3; i < process.argv.length; i++) { } } +if (sawRemovedFlags) { + console.error('See `npm help exec` for more information') +} + cli(process) diff --git a/docs/content/cli-commands/npm-exec.md b/docs/content/cli-commands/npm-exec.md index 1537c915b..29b7a06e5 100644 --- a/docs/content/cli-commands/npm-exec.md +++ b/docs/content/cli-commands/npm-exec.md @@ -41,7 +41,10 @@ where all specified packages are available. If any requested packages are not present in the local project dependencies, then they are installed to a folder in the npm cache, which -is added to the `PATH` environment variable in the executed process. +is added to the `PATH` environment variable in the executed process. A +prompt is printed (which can be suppressed by providing either `--yes` or +`--no`). + Package names provided without a specifier will be matched with whatever version exists in the local project. Package names with a specifier will only be considered a match if they have the exact same name and version as @@ -137,6 +140,34 @@ $ npm x -c 'eslint && say "hooray, lint passed"' $ npx -c 'eslint && say "hooray, lint passed"' ``` +### Compatibility with Older npx Versions + +The `npx` binary was rewritten in npm v7.0.0, and the standalone `npx` +package deprecated at that time. `npx` uses the `npm exec` +command instead of a separate argument parser and install process, with +some affordances to maintain backwards compatibility with the arguments it +accepted in previous versions. + +This resulted in some shifts in its functionality: + +- Any `npm` config value may be provided. +- To prevent security and user-experience problems from mistyping package + names, `npx` prompts before installing anything. Suppress this + prompt with the `-y` or `--yes` option. +- The `--no-install` option is deprecated, and will be converted to `--no`. +- Shell fallback functionality is removed, as it is not advisable. +- The `-p` argument is a shorthand for `--parseable` in npm, but shorthand + for `--package` in npx. This is maintained, but only for the `npx` + executable. +- The `--ignore-existing` option is removed. Locally installed bins are + always present in the executed process `PATH`. +- The `--npm` option is removed. `npx` will always use the `npm` it ships + with. +- The `--node-arg` and `-n` options are removed. +- The `--always-spawn` option is redundant, and thus removed. +- The `--shell` option is replaced with `--script-shell`, but maintained + in the `npx` executable for backwards compatibility. + ### See Also * [npm run-script](/cli-commands/run-script) diff --git a/docs/content/cli-commands/npx.md b/docs/content/cli-commands/npx.md index ae19dc4ff..6568a28b1 100644 --- a/docs/content/cli-commands/npx.md +++ b/docs/content/cli-commands/npx.md @@ -41,7 +41,10 @@ where all specified packages are available. If any requested packages are not present in the local project dependencies, then they are installed to a folder in the npm cache, which -is added to the `PATH` environment variable in the executed process. +is added to the `PATH` environment variable in the executed process. A +prompt is printed (which can be suppressed by providing either `--yes` or +`--no`). + Package names provided without a specifier will be matched with whatever version exists in the local project. Package names with a specifier will only be considered a match if they have the exact same name and version as @@ -137,6 +140,34 @@ $ npm x -c 'eslint && say "hooray, lint passed"' $ npx -c 'eslint && say "hooray, lint passed"' ``` +### Compatibility with Older npx Versions + +The `npx` binary was rewritten in npm v7.0.0, and the standalone `npx` +package deprecated at that time. `npx` uses the `npm exec` +command instead of a separate argument parser and install process, with +some affordances to maintain backwards compatibility with the arguments it +accepted in previous versions. + +This resulted in some shifts in its functionality: + +- Any `npm` config value may be provided. +- To prevent security and user-experience problems from mistyping package + names, `npx` prompts before installing anything. Suppress this + prompt with the `-y` or `--yes` option. +- The `--no-install` option is deprecated, and will be converted to `--no`. +- Shell fallback functionality is removed, as it is not advisable. +- The `-p` argument is a shorthand for `--parseable` in npm, but shorthand + for `--package` in npx. This is maintained, but only for the `npx` + executable. +- The `--ignore-existing` option is removed. Locally installed bins are + always present in the executed process `PATH`. +- The `--npm` option is removed. `npx` will always use the `npm` it ships + with. +- The `--node-arg` and `-n` options are removed. +- The `--always-spawn` option is redundant, and thus removed. +- The `--shell` option is replaced with `--script-shell`, but maintained + in the `npx` executable for backwards compatibility. + ### See Also * [npm run-script](/cli-commands/run-script) diff --git a/lib/config/defaults.js b/lib/config/defaults.js index 926ca9ad4..13e3c0c11 100644 --- a/lib/config/defaults.js +++ b/lib/config/defaults.js @@ -311,7 +311,7 @@ exports.types = { 'ham-it-up': Boolean, heading: String, 'if-present': Boolean, - include: [Array, 'dev', 'optional', 'peer'], + include: [Array, 'prod', 'dev', 'optional', 'peer'], 'include-staged': Boolean, 'ignore-prepublish': Boolean, 'ignore-scripts': Boolean, @@ -365,7 +365,7 @@ exports.types = { 'save-prod': Boolean, scope: String, 'script-shell': [null, String], - 'scripts-prepend-node-path': [false, true, 'auto', 'warn-only'], + 'scripts-prepend-node-path': [Boolean, 'auto', 'warn-only'], searchopts: String, searchexclude: [null, String], searchlimit: Number, @@ -412,7 +412,7 @@ function getLocalAddresses () { } exports.shorthands = { - before: ['--enjoy-by'], + 'enjoy-by': ['--before'], c: ['--call'], s: ['--loglevel', 'silent'], d: ['--loglevel', 'info'], diff --git a/lib/config/flat-options.js b/lib/config/flat-options.js index 3358f0c5e..9fbdfdbb5 100644 --- a/lib/config/flat-options.js +++ b/lib/config/flat-options.js @@ -198,6 +198,9 @@ const flatOptions = npm => npm.flatOptions || Object.freeze({ }, userAgent: npm.config.get('user-agent'), + // yes, it's fine, just do it, jeez, stop asking + yes: npm.config.get('yes'), + ...getScopesAndAuths(npm), // npm fund exclusive option to select an item from a funding list diff --git a/lib/exec.js b/lib/exec.js index be19137c2..7bd864cc6 100644 --- a/lib/exec.js +++ b/lib/exec.js @@ -6,21 +6,25 @@ const usage = usageUtil('exec', 'Run a command from a local or remote npm package.\n\n' + 'npm exec -- <pkg>[@<version>] [args...]\n' + - 'npm exec -p <pkg>[@<version>] -- <cmd> [args...]\n' + + 'npm exec --package=<pkg>[@<version>] -- <cmd> [args...]\n' + 'npm exec -c \'<cmd> [args...]\'\n' + - 'npm exec -p foo -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-p <pkg> --package=<pkg> (may be specified multiple times)\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 completion = require('./utils/completion/installed-shallow.js') +const { promisify } = require('util') +const read = promisify(require('read')) + // it's like this: // // npm x pkg@version <-- runs the bin named "pkg" or the only bin if only 1 @@ -118,9 +122,25 @@ const exec = async args => { // add installDir/node_modules/.bin to pathArr const add = manis.filter(mani => manifestMissing(tree, mani)) .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' + } + 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' + } + } await arb.reify({ add }) } pathArr.unshift(resolve(installDir, 'node_modules/.bin')) diff --git a/tap-snapshots/test-lib-config-flat-options.js-TAP.test.js b/tap-snapshots/test-lib-config-flat-options.js-TAP.test.js index 08ae5075d..b360c5ac8 100644 --- a/tap-snapshots/test-lib-config-flat-options.js-TAP.test.js +++ b/tap-snapshots/test-lib-config-flat-options.js-TAP.test.js @@ -120,5 +120,6 @@ Object { "userAgent": "user-agent", "viewer": "viewer", "which": undefined, + "yes": undefined, } ` diff --git a/test/bin/npx-cli.js b/test/bin/npx-cli.js index 8995fb76a..fc85f6366 100644 --- a/test/bin/npx-cli.js +++ b/test/bin/npx-cli.js @@ -4,6 +4,14 @@ const npx = require.resolve('../../bin/npx-cli.js') const cli = require.resolve('../../lib/cli.js') const npm = require.resolve('../../bin/npm-cli.js') +const logs = [] +console.error = (...msg) => logs.push(msg) + +t.afterEach(cb => { + logs.length = 0 + cb() +}) + t.test('npx foo -> npm exec -- foo', t => { process.argv = ['node', npx, 'foo'] requireInject(npx, { [cli]: () => {} }) @@ -25,9 +33,63 @@ t.test('npx -x y foo -z -> npm exec -x y -- foo -z', t => { t.end() }) -t.test('npx --x=y foo -z -> npm exec --x=y -- foo -z', t => { - process.argv = ['node', npx, '--x=y', 'foo', '-z'] +t.test('npx --x=y --no-install foo -z -> npm exec --x=y -- foo -z', t => { + process.argv = ['node', npx, '--x=y', '--no-install', 'foo', '-z'] + requireInject(npx, { [cli]: () => {} }) + t.strictSame(process.argv, ['node', npm, 'exec', '--x=y', '--yes=false', '--', 'foo', '-z']) + t.end() +}) + +t.test('transform renamed options into proper values', t => { + process.argv = ['node', npx, '-y', '--shell=bash', '-p', 'foo', '-c', 'asdf'] + requireInject(npx, { [cli]: () => {} }) + t.strictSame(process.argv, ['node', npm, 'exec', '--yes', '--script-shell=bash', '--package', 'foo', '--call', 'asdf']) + t.end() +}) + +// warn if deprecated switches/options are used +t.test('use a bunch of deprecated switches and options', t => { + process.argv = [ + 'node', + npx, + '--npm', + '/some/npm/bin', + '--node-arg=--harmony', + '-n', + '--require=foobar', + '--reg=http://localhost:12345/', + '-p', + 'foo', + '--always-spawn', + '--shell-auto-fallback', + '--ignore-existing', + '-q', + 'foobar' + ] + + const expect = [ + 'node', + npm, + 'exec', + '--registry', + 'http://localhost:12345/', + '--package', + 'foo', + '--loglevel', + 'warn', + '--', + 'foobar' + ] requireInject(npx, { [cli]: () => {} }) - t.strictSame(process.argv, ['node', npm, 'exec', '--x=y', '--', 'foo', '-z']) + t.strictSame(process.argv, expect) + t.strictSame(logs, [ + [ 'npx: the --npm argument has been removed.' ], + [ 'npx: the --node-arg argument has been removed.' ], + [ 'npx: the --n argument has been removed.' ], + [ 'npx: the --always-spawn argument has been removed.' ], + [ 'npx: the --shell-auto-fallback argument has been removed.' ], + [ 'npx: the --ignore-existing argument has been removed.' ], + [ 'See `npm help exec` for more information' ] + ]) t.end() }) diff --git a/test/coverage-map.js b/test/coverage-map.js index 70b1b13dd..9c9dde834 100644 --- a/test/coverage-map.js +++ b/test/coverage-map.js @@ -1,7 +1,7 @@ 'use strict' const coverageMap = (filename) => { - if (/^test\/lib\//.test(filename)) { + if (/^test\/(lib|bin)\//.test(filename)) { return filename.replace(/^test\//, '') } return [] diff --git a/test/lib/exec.js b/test/lib/exec.js index dda06e7a8..bd5964cfe 100644 --- a/test/lib/exec.js +++ b/test/lib/exec.js @@ -21,6 +21,7 @@ class Arborist { let PROGRESS_ENABLED = true const npm = { flatOptions: { + yes: true, call: '', package: [] }, @@ -61,11 +62,20 @@ const pacote = { const MKDIRPS = [] const mkdirp = async path => MKDIRPS.push(path) +let READ_RESULT = '' +let READ_ERROR = null +const READ = [] +const read = (options, cb) => { + READ.push(options) + process.nextTick(() => cb(READ_ERROR, READ_RESULT)) +} + const exec = requireInject('../../lib/exec.js', { '@npmcli/arborist': Arborist, '@npmcli/run-script': runScript, '../../lib/npm.js': npm, pacote, + read, 'mkdirp-infer-owner': mkdirp }) @@ -74,6 +84,9 @@ t.afterEach(cb => { ARB_CTOR.length = 0 ARB_REIFY.length = 0 RUN_SCRIPTS.length = 0 + READ.length = 0 + READ_RESULT = '' + READ_ERROR = null npm.flatOptions.package = [] npm.flatOptions.call = '' cb() @@ -308,14 +321,14 @@ t.test('npm exec @foo/bar -- --some=arg, locally installed', async t => { t.test('run command with 2 packages, need install, verify sort', t => { // test both directions, should use same install dir both times + // also test the read() call here, verify that the prompts match const cases = [['foo', 'bar'], ['bar', 'foo']] t.plan(cases.length) for (const packages of cases) { t.test(packages.join(', '), async t => { npm.flatOptions.package = packages - const add = packages.map(p => `${p}@`) + const add = packages.map(p => `${p}@`).sort((a, b) => a.localeCompare(b)) const path = t.testdir() - // XXX const installDir = resolve('cache-dir/_npx/07de77790e5f40f2') npm.localPrefix = path ARB_ACTUAL_TREE[path] = { @@ -446,9 +459,200 @@ t.test('npm exec -p foo -c "ls -laF"', async t => { t.test('positional args and --call together is an error', t => { npm.flatOptions.call = 'true' - return t.rejects(exec(['foo'], er => { + return exec(['foo'], er => t.equal(er, exec.usage)) +}) + +t.test('prompt when installs are needed if not already present', async t => { + const packages = ['foo', 'bar'] + READ_RESULT = 'yolo' + + npm.flatOptions.package = packages + npm.flatOptions.yes = undefined + + const add = packages.map(p => `${p}@`).sort((a, b) => a.localeCompare(b)) + const path = t.testdir() + const installDir = resolve('cache-dir/_npx/07de77790e5f40f2') + npm.localPrefix = path + ARB_ACTUAL_TREE[path] = { + children: new Map() + } + ARB_ACTUAL_TREE[installDir] = { + children: new Map() + } + MANIFESTS.foo = { + name: 'foo', + version: '1.2.3', + bin: { + foo: 'foo' + }, + _from: 'foo@' + } + MANIFESTS.bar = { + name: 'bar', + version: '1.2.3', + bin: { + bar: 'bar' + }, + _from: 'bar@' + } + await exec(['foobar'], er => { if (er) { throw er } - }), exec.usage) + }) + t.strictSame(MKDIRPS, [installDir], 'need to make install dir') + t.match(ARB_CTOR, [ { package: packages, path } ]) + t.strictSame(ARB_REIFY, [{add}], 'need to install both packages') + t.equal(PROGRESS_ENABLED, true, 'progress re-enabled') + const PATH = `${resolve(installDir, 'node_modules', '.bin')}${delimiter}${process.env.PATH}` + t.match(RUN_SCRIPTS, [{ + pkg: { scripts: { npx: 'foobar' } }, + banner: false, + path: process.cwd(), + stdioString: true, + event: 'npx', + env: { PATH }, + stdio: 'inherit' + }]) + t.strictSame(READ, [{ + prompt: 'Need to install the following packages:\n bar\n foo\nOk to proceed? ', + default: 'y' + }]) +}) + +t.test('abort if prompt rejected', async t => { + const packages = ['foo', 'bar'] + READ_RESULT = 'no, why would I want such a thing??' + + npm.flatOptions.package = packages + npm.flatOptions.yes = undefined + + const add = packages.map(p => `${p}@`).sort((a, b) => a.localeCompare(b)) + const path = t.testdir() + const installDir = resolve('cache-dir/_npx/07de77790e5f40f2') + npm.localPrefix = path + ARB_ACTUAL_TREE[path] = { + children: new Map() + } + ARB_ACTUAL_TREE[installDir] = { + children: new Map() + } + MANIFESTS.foo = { + name: 'foo', + version: '1.2.3', + bin: { + foo: 'foo' + }, + _from: 'foo@' + } + MANIFESTS.bar = { + name: 'bar', + version: '1.2.3', + bin: { + bar: 'bar' + }, + _from: 'bar@' + } + await exec(['foobar'], er => { + t.equal(er, 'canceled', 'should be canceled') + }) + t.strictSame(MKDIRPS, [installDir], 'need to make install dir') + t.match(ARB_CTOR, [ { package: packages, path } ]) + t.strictSame(ARB_REIFY, [], 'no install performed') + t.equal(PROGRESS_ENABLED, true, 'progress re-enabled') + t.strictSame(RUN_SCRIPTS, []) + t.strictSame(READ, [{ + prompt: 'Need to install the following packages:\n bar\n foo\nOk to proceed? ', + default: 'y' + }]) +}) + +t.test('abort if prompt false', async t => { + const packages = ['foo', 'bar'] + READ_ERROR = 'canceled' + + npm.flatOptions.package = packages + npm.flatOptions.yes = undefined + + const add = packages.map(p => `${p}@`).sort((a, b) => a.localeCompare(b)) + const path = t.testdir() + const installDir = resolve('cache-dir/_npx/07de77790e5f40f2') + npm.localPrefix = path + ARB_ACTUAL_TREE[path] = { + children: new Map() + } + ARB_ACTUAL_TREE[installDir] = { + children: new Map() + } + MANIFESTS.foo = { + name: 'foo', + version: '1.2.3', + bin: { + foo: 'foo' + }, + _from: 'foo@' + } + MANIFESTS.bar = { + name: 'bar', + version: '1.2.3', + bin: { + bar: 'bar' + }, + _from: 'bar@' + } + await exec(['foobar'], er => { + t.equal(er, 'canceled', 'should be canceled') + }) + t.strictSame(MKDIRPS, [installDir], 'need to make install dir') + t.match(ARB_CTOR, [ { package: packages, path } ]) + t.strictSame(ARB_REIFY, [], 'no install performed') + t.equal(PROGRESS_ENABLED, true, 'progress re-enabled') + t.strictSame(RUN_SCRIPTS, []) + t.strictSame(READ, [{ + prompt: 'Need to install the following packages:\n bar\n foo\nOk to proceed? ', + default: 'y' + }]) +}) + +t.test('abort if -n provided', async t => { + const packages = ['foo', 'bar'] + + npm.flatOptions.package = packages + npm.flatOptions.yes = false + + const add = packages.map(p => `${p}@`).sort((a, b) => a.localeCompare(b)) + const path = t.testdir() + const installDir = resolve('cache-dir/_npx/07de77790e5f40f2') + npm.localPrefix = path + ARB_ACTUAL_TREE[path] = { + children: new Map() + } + ARB_ACTUAL_TREE[installDir] = { + children: new Map() + } + MANIFESTS.foo = { + name: 'foo', + version: '1.2.3', + bin: { + foo: 'foo' + }, + _from: 'foo@' + } + MANIFESTS.bar = { + name: 'bar', + version: '1.2.3', + bin: { + bar: 'bar' + }, + _from: 'bar@' + } + await exec(['foobar'], er => { + t.equal(er, 'canceled', 'should be canceled') + }) + t.strictSame(MKDIRPS, [installDir], 'need to make install dir') + t.match(ARB_CTOR, [ { package: packages, path } ]) + t.strictSame(ARB_REIFY, [], 'no install performed') + t.equal(PROGRESS_ENABLED, true, 'progress re-enabled') + t.strictSame(RUN_SCRIPTS, []) + t.strictSame(READ, []) }) |