Skip to content

Commit 15d7333

Browse files
isaacsdarcyclarke
isaacs
authored and
darcyclarke
committed
Use @npmcli/run-script for exec, explore; add interactive exec
This removes all other arg/shell escaping mechanisms, as they are no longer needed, and will be un-done by puka in @npmcli/run-script anyway. Adds an interactive shell mode when `npm exec` is run without any arguments, allowing users to interactively test out commands in an npm script environment. Previously, this would do nothing, and just exit. Prevent weird behavior from `npm explore ../blah`. `explore` now can _only_ be used to explore packages that are actually found in the relevant `node_modules` folder.
1 parent e9a440b commit 15d7333

File tree

9 files changed

+293
-220
lines changed

9 files changed

+293
-220
lines changed

‎docs/content/commands/npm-exec.md‎

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -16,9 +16,12 @@ npx <pkg>[@<specifier>] [args...]
1616
npx -p <pkg>[@<specifier>] <cmd> [args...]
1717
npx -c '<cmd> [args...]'
1818
npx -p <pkg>[@<specifier>] -c '<cmd> [args...]'
19+
Run without --call or positional args to open interactive subshell
20+
1921

2022
alias: npm x, npx
2123

24+
common options:
2225
--package=<pkg> (may be specified multiple times)
2326
-p is a shorthand for --package only when using npx executable
2427
-c <cmd> --call=<cmd> (may not be mixed with positional arguments)
@@ -30,6 +33,10 @@ This command allows you to run an arbitrary command from an npm package
3033
(either one installed locally, or fetched remotely), in a similar context
3134
as running it via `npm run`.
3235

36+
Run without positional arguments or `--call`, this allows you to
37+
interactively run commands in the same sort of shell environment that
38+
`package.json` scripts are run.
39+
3340
Whatever packages are specified by the `--package` option will be
3441
provided in the `PATH` of the executed command, along with any locally
3542
installed package executables. The `--package` option may be

‎lib/exec.js‎

Lines changed: 29 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,6 @@
11
const npm = require('./npm.js')
2-
2+
const output = require('./utils/output.js')
33
const usageUtil = require('./utils/usage.js')
4-
54
const usage = usageUtil('exec',
65
'Run a command from a local or remote npm package.\n\n' +
76

@@ -13,7 +12,9 @@ const usage = usageUtil('exec',
1312
'npx <pkg>[@<specifier>] [args...]\n' +
1413
'npx -p <pkg>[@<specifier>] <cmd> [args...]\n' +
1514
'npx -c \'<cmd> [args...]\'\n' +
16-
'npx -p <pkg>[@<specifier>] -c \'<cmd> [args...]\'',
15+
'npx -p <pkg>[@<specifier>] -c \'<cmd> [args...]\'' +
16+
'\n' +
17+
'Run without --call or positional args to open interactive subshell\n',
1718

1819
'\n--package=<pkg> (may be specified multiple times)\n' +
1920
'-p is a shorthand for --package only when using npx executable\n' +
@@ -59,15 +60,14 @@ const ciDetect = require('@npmcli/ci-detect')
5960
const crypto = require('crypto')
6061
const pacote = require('pacote')
6162
const npa = require('npm-package-arg')
62-
const escapeArg = require('./utils/escape-arg.js')
6363
const fileExists = require('./utils/file-exists.js')
6464
const PATH = require('./utils/path.js')
6565

6666
const cmd = (args, cb) => exec(args).then(() => cb()).catch(cb)
6767

68-
const run = async ({ args, call, pathArr }) => {
68+
const run = async ({ args, call, pathArr, shell }) => {
6969
// turn list of args into command string
70-
const script = call || args.map(escapeArg).join(' ').trim()
70+
const script = call || args.join(' ').trim() || shell
7171

7272
// do the fakey runScript dance
7373
// still should work if no package.json in cwd
@@ -84,6 +84,7 @@ const run = async ({ args, call, pathArr }) => {
8484
npm.log.disableProgress()
8585
try {
8686
return await runScript({
87+
...npm.flatOptions,
8788
pkg,
8889
banner: false,
8990
// we always run in cwd, not --prefix
@@ -101,13 +102,24 @@ const run = async ({ args, call, pathArr }) => {
101102
}
102103

103104
const exec = async args => {
104-
const { package: packages, call } = npm.flatOptions
105+
const { package: packages, call, shell } = npm.flatOptions
105106

106107
if (call && args.length)
107108
throw usage
108109

109110
const pathArr = [...PATH]
110111

112+
// nothing to maybe install, skip the arborist dance
113+
if (!call && !args.length && !packages.length) {
114+
output(`\nEntering npm script environment\nType 'exit' or ^D when finished\n`)
115+
return await run({
116+
args,
117+
call,
118+
shell,
119+
pathArr,
120+
})
121+
}
122+
111123
const needPackageCommandSwap = args.length && !packages.length
112124
// if there's an argument and no package has been explicitly asked for
113125
// check the local and global bin paths for a binary named the same as
@@ -126,8 +138,9 @@ const exec = async args => {
126138
if (binExists) {
127139
return await run({
128140
args,
129-
call: [args[0], ...args.slice(1).map(escapeArg)].join(' ').trim(),
141+
call: [args[0], ...args.slice(1)].join(' ').trim(),
130142
pathArr,
143+
shell,
131144
})
132145
}
133146

@@ -187,9 +200,13 @@ const exec = async args => {
187200
if (npm.flatOptions.yes === false)
188201
throw 'canceled'
189202

190-
if (!isTTY || ciDetect())
191-
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(', ')}`)
192-
else {
203+
if (!isTTY || ciDetect()) {
204+
npm.log.warn('exec', `The following package${
205+
add.length === 1 ? ' was' : 's were'
206+
} not found and will be installed: ${
207+
add.map((pkg) => pkg.replace(/@$/, '')).join(', ')
208+
}`)
209+
} else {
193210
const addList = add.map(a => ` ${a.replace(/@$/, '')}`)
194211
.join('\n') + '\n'
195212
const prompt = `Need to install the following packages:\n${
@@ -205,7 +222,7 @@ const exec = async args => {
205222
pathArr.unshift(resolve(installDir, 'node_modules/.bin'))
206223
}
207224

208-
return await run({ args, call, pathArr })
225+
return await run({ args, call, pathArr, shell })
209226
}
210227

211228
const manifestMissing = (tree, mani) => {

‎lib/explore.js‎

Lines changed: 41 additions & 35 deletions
Original file line numberDiff line numberDiff line change
@@ -4,59 +4,65 @@
44
const usageUtil = require('./utils/usage.js')
55
const completion = require('./utils/completion/installed-shallow.js')
66
const usage = usageUtil('explore', 'npm explore <pkg> [ -- <command>]')
7+
const rpj = require('read-package-json-fast')
78

89
const cmd = (args, cb) => explore(args).then(() => cb()).catch(cb)
910

1011
const output = require('./utils/output.js')
1112
const npm = require('./npm.js')
12-
const isWindows = require('./utils/is-windows.js')
13-
const escapeArg = require('./utils/escape-arg.js')
14-
const escapeExecPath = require('./utils/escape-exec-path.js')
15-
const log = require('npmlog')
1613

17-
const spawn = require('@npmcli/promise-spawn')
18-
19-
const { resolve } = require('path')
20-
const { promisify } = require('util')
21-
const stat = promisify(require('fs').stat)
14+
const runScript = require('@npmcli/run-script')
15+
const { join, resolve, relative } = require('path')
2216

2317
const explore = async args => {
2418
if (args.length < 1 || !args[0])
2519
throw usage
2620

27-
const pkg = args.shift()
28-
const cwd = resolve(npm.dir, pkg)
29-
const opts = { cwd, stdio: 'inherit', stdioString: true }
30-
31-
const shellArgs = []
32-
if (args.length) {
33-
if (isWindows) {
34-
const execCmd = escapeExecPath(args.shift())
35-
opts.windowsVerbatimArguments = true
36-
shellArgs.push('/d', '/s', '/c', execCmd, ...args.map(escapeArg))
37-
} else
38-
shellArgs.push('-c', args.map(escapeArg).join(' ').trim())
39-
}
21+
const pkgname = args.shift()
4022

41-
await stat(cwd).catch(er => {
42-
throw new Error(`It doesn't look like ${pkg} is installed.`)
43-
})
23+
// detect and prevent any .. shenanigans
24+
const path = join(npm.dir, join('/', pkgname))
25+
if (relative(path, npm.dir) === '')
26+
throw usage
4427

45-
const sh = npm.flatOptions.shell
46-
log.disableProgress()
28+
// run as if running a script named '_explore', which we set to either
29+
// the set of arguments, or the shell config, and let @npmcli/run-script
30+
// handle all the escaping and PATH setup stuff.
4731

48-
if (!shellArgs.length)
49-
output(`\nExploring ${cwd}\nType 'exit' or ^D when finished\n`)
32+
const pkg = await rpj(resolve(path, 'package.json')).catch(er => {
33+
npm.log.error('explore', `It doesn't look like ${pkgname} is installed.`)
34+
throw er
35+
})
5036

51-
log.silly('explore', { sh, shellArgs, opts })
37+
const { shell } = npm.flatOptions
38+
pkg.scripts = {
39+
...(pkg.scripts || {}),
40+
_explore: args.join(' ').trim() || shell,
41+
}
5242

53-
// only noisily fail if non-interactive, but still keep exit code intact
54-
const proc = spawn(sh, shellArgs, opts)
43+
if (!args.length)
44+
output(`\nExploring ${path}\nType 'exit' or ^D when finished\n`)
45+
npm.log.disableProgress()
5546
try {
56-
const res = await (shellArgs.length ? proc : proc.catch(er => er))
57-
process.exitCode = res.code
47+
return await runScript({
48+
...npm.flatOptions,
49+
pkg,
50+
banner: false,
51+
path,
52+
stdioString: true,
53+
event: '_explore',
54+
stdio: 'inherit',
55+
}).catch(er => {
56+
process.exitCode = typeof er.code === 'number' && er.code !== 0 ? er.code
57+
: 1
58+
// if it's not an exit error, or non-interactive, throw it
59+
const isProcExit = er.message === 'command failed' &&
60+
(typeof er.code === 'number' || /^SIG/.test(er.signal || ''))
61+
if (args.length || !isProcExit)
62+
throw er
63+
})
5864
} finally {
59-
log.enableProgress()
65+
npm.log.enableProgress()
6066
}
6167
}
6268

‎lib/utils/escape-arg.js‎

Lines changed: 0 additions & 18 deletions
This file was deleted.

‎lib/utils/escape-exec-path.js‎

Lines changed: 0 additions & 21 deletions
This file was deleted.

‎test/lib/exec.js‎

Lines changed: 33 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,8 @@
11
const t = require('tap')
22
const requireInject = require('require-inject')
33
const { resolve, delimiter } = require('path')
4+
const OUTPUT = []
5+
const output = (...msg) => OUTPUT.push(msg)
46

57
const ARB_CTOR = []
68
const ARB_ACTUAL_TREE = {}
@@ -29,6 +31,7 @@ const npm = {
2931
call: '',
3032
package: [],
3133
legacyPeerDeps: false,
34+
shell: 'shell-cmd',
3235
},
3336
localPrefix: 'local-prefix',
3437
localBin: 'local-bin',
@@ -91,6 +94,7 @@ const mocks = {
9194
pacote,
9295
read,
9396
'mkdirp-infer-owner': mkdirp,
97+
'../../lib/utils/output.js': output,
9498
}
9599
const exec = requireInject('../../lib/exec.js', mocks)
96100

@@ -123,7 +127,7 @@ t.test('npx foo, bin already exists locally', async t => {
123127
await exec(['foo'], er => {
124128
t.ifError(er, 'npm exec')
125129
})
126-
t.strictSame(RUN_SCRIPTS, [{
130+
t.match(RUN_SCRIPTS, [{
127131
pkg: { scripts: { npx: 'foo' }},
128132
banner: false,
129133
path: process.cwd(),
@@ -147,7 +151,7 @@ t.test('npx foo, bin already exists globally', async t => {
147151
await exec(['foo'], er => {
148152
t.ifError(er, 'npm exec')
149153
})
150-
t.strictSame(RUN_SCRIPTS, [{
154+
t.match(RUN_SCRIPTS, [{
151155
pkg: { scripts: { npx: 'foo' }},
152156
banner: false,
153157
path: process.cwd(),
@@ -193,6 +197,33 @@ t.test('npm exec foo, already present locally', async t => {
193197
}])
194198
})
195199

200+
t.test('npm exec <noargs>, run interactive shell', async t => {
201+
ARB_CTOR.length = 0
202+
MKDIRPS.length = 0
203+
ARB_REIFY.length = 0
204+
OUTPUT.length = 0
205+
await exec([], er => {
206+
if (er)
207+
throw er
208+
})
209+
t.strictSame(OUTPUT, [
210+
['\nEntering npm script environment\nType \'exit\' or ^D when finished\n'],
211+
], 'printed message about interactive shell')
212+
t.strictSame(MKDIRPS, [], 'no need to make any dirs')
213+
t.strictSame(ARB_CTOR, [], 'no need to instantiate arborist')
214+
t.strictSame(ARB_REIFY, [], 'no need to reify anything')
215+
t.equal(PROGRESS_ENABLED, true, 'progress re-enabled')
216+
t.match(RUN_SCRIPTS, [{
217+
pkg: { scripts: { npx: 'shell-cmd' } },
218+
banner: false,
219+
path: process.cwd(),
220+
stdioString: true,
221+
event: 'npx',
222+
env: { PATH: process.env.PATH },
223+
stdio: 'inherit',
224+
}])
225+
})
226+
196227
t.test('npm exec foo, not present locally or in central loc', async t => {
197228
const path = t.testdir()
198229
const installDir = resolve('cache-dir/_npx/f7fbba6e0636f890')

0 commit comments

Comments
 (0)