Skip to content

Commit 2210d7a

Browse files
alexsch01alexsch01
authored
fix(powershell): use Invoke-Expression to pass args (#8278)
Continuation of #8267 @mbtools --- This fixes the command `npm test -- hello -p1 world -p2 "hello world" --q1=hello world --q2="hello world"` in Windows PowerShell and pwsh7 - where the "test" script prints all the arguments passed after the first "--" in the command above Before this change ``` PS> npm test -- hello -p1 world -p2 "hello world" --q1=hello world --q2="hello world" npm warn "world" is being parsed as a normal command line argument. npm warn "hello world" is being parsed as a normal command line argument. npm warn Unknown cli config "--p1". This will stop working in the next major version of npm. npm warn Unknown cli config "--p2". This will stop working in the next major version of npm. npm warn Unknown cli config "--q1". This will stop working in the next major version of npm. npm warn Unknown cli config "--q2". This will stop working in the next major version of npm. > [email protected] test > node args.js hello world hello world world hello world hello world world ``` With this change ``` PS> npm test -- hello -p1 world -p2 "hello world" --q1=hello world --q2="hello world" > [email protected] test > node args.js hello -p1 world -p2 hello world --q1=hello world --q2=hello world hello -p1 world -p2 hello world --q1=hello world --q2=hello world ``` --- Also, fixes comma-separated values in Windows PowerShell and pwsh7 Before this change ``` PS> npm help a=1,b=2,c=3 No matches in help for: a=1 b=2 c=3 ``` With this change ``` PS> npm help a=1,b=2,c=3 No matches in help for: a=1,b=2,c=3 ```
1 parent 4dd41c9 commit 2210d7a

File tree

3 files changed

+105
-18
lines changed

3 files changed

+105
-18
lines changed

‎bin/npm.ps1

Lines changed: 26 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -22,11 +22,32 @@ if (Test-Path $NPM_PREFIX_NPM_CLI_JS) {
2222
$NPM_CLI_JS=$NPM_PREFIX_NPM_CLI_JS
2323
}
2424

25-
# Support pipeline input
26-
if ($MyInvocation.ExpectingInput) {
27-
$input | & $NODE_EXE $NPM_CLI_JS $args
28-
} else {
29-
& $NODE_EXE $NPM_CLI_JS $args
25+
if ($MyInvocation.Line) { # used "-Command" argument
26+
if ($MyInvocation.Statement) {
27+
$NPM_ARGS = $MyInvocation.Statement.Substring($MyInvocation.InvocationName.Length).Trim()
28+
} else {
29+
$NPM_OG_COMMAND = (
30+
[System.Management.Automation.InvocationInfo].GetProperty('ScriptPosition', [System.Reflection.BindingFlags] 'Instance, NonPublic')
31+
).GetValue($MyInvocation).Text
32+
$NPM_ARGS = $NPM_OG_COMMAND.Substring($MyInvocation.InvocationName.Length).Trim()
33+
}
34+
35+
$NODE_EXE = $NODE_EXE.Replace("``", "````")
36+
$NPM_CLI_JS = $NPM_CLI_JS.Replace("``", "````")
37+
38+
# Support pipeline input
39+
if ($MyInvocation.ExpectingInput) {
40+
$input | Invoke-Expression "& `"$NODE_EXE`" `"$NPM_CLI_JS`" $NPM_ARGS"
41+
} else {
42+
Invoke-Expression "& `"$NODE_EXE`" `"$NPM_CLI_JS`" $NPM_ARGS"
43+
}
44+
} else { # used "-File" argument
45+
# Support pipeline input
46+
if ($MyInvocation.ExpectingInput) {
47+
$input | & $NODE_EXE $NPM_CLI_JS $args
48+
} else {
49+
& $NODE_EXE $NPM_CLI_JS $args
50+
}
3051
}
3152

3253
exit $LASTEXITCODE

‎bin/npx.ps1

Lines changed: 26 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -22,11 +22,32 @@ if (Test-Path $NPM_PREFIX_NPX_CLI_JS) {
2222
$NPX_CLI_JS=$NPM_PREFIX_NPX_CLI_JS
2323
}
2424

25-
# Support pipeline input
26-
if ($MyInvocation.ExpectingInput) {
27-
$input | & $NODE_EXE $NPX_CLI_JS $args
28-
} else {
29-
& $NODE_EXE $NPX_CLI_JS $args
25+
if ($MyInvocation.Line) { # used "-Command" argument
26+
if ($MyInvocation.Statement) {
27+
$NPX_ARGS = $MyInvocation.Statement.Substring($MyInvocation.InvocationName.Length).Trim()
28+
} else {
29+
$NPX_OG_COMMAND = (
30+
[System.Management.Automation.InvocationInfo].GetProperty('ScriptPosition', [System.Reflection.BindingFlags] 'Instance, NonPublic')
31+
).GetValue($MyInvocation).Text
32+
$NPX_ARGS = $NPX_OG_COMMAND.Substring($MyInvocation.InvocationName.Length).Trim()
33+
}
34+
35+
$NODE_EXE = $NODE_EXE.Replace("``", "````")
36+
$NPX_CLI_JS = $NPX_CLI_JS.Replace("``", "````")
37+
38+
# Support pipeline input
39+
if ($MyInvocation.ExpectingInput) {
40+
$input | Invoke-Expression "& `"$NODE_EXE`" `"$NPX_CLI_JS`" $NPX_ARGS"
41+
} else {
42+
Invoke-Expression "& `"$NODE_EXE`" `"$NPX_CLI_JS`" $NPX_ARGS"
43+
}
44+
} else { # used "-File" argument
45+
# Support pipeline input
46+
if ($MyInvocation.ExpectingInput) {
47+
$input | & $NODE_EXE $NPX_CLI_JS $args
48+
} else {
49+
& $NODE_EXE $NPX_CLI_JS $args
50+
}
3051
}
3152

3253
exit $LASTEXITCODE

‎test/bin/windows-shims.js

Lines changed: 53 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,9 @@ const BIN = join(ROOT, 'bin')
2020
const SHIMS = readNonJsFiles(BIN)
2121
const NODE_GYP = readNonJsFiles(join(BIN, 'node-gyp-bin'))
2222
const SHIM_EXTS = [...new Set(Object.keys(SHIMS).map(p => extname(p)))]
23+
const PACKAGE_NAME = 'test'
24+
const PACKAGE_VERSION = '1.0.0'
25+
const SCRIPT_NAME = 'args.js'
2326

2427
t.test('shim contents', t => {
2528
// these scripts should be kept in sync so this tests the contents of each
@@ -99,6 +102,18 @@ t.test('run shims', t => {
99102
},
100103
},
101104
},
105+
// test script returning all command line arguments
106+
[SCRIPT_NAME]: `#!/usr/bin/env node\n\nprocess.argv.slice(2).forEach((arg) => console.log(arg))`,
107+
// package.json for the test script
108+
'package.json': `
109+
{
110+
"name": "${PACKAGE_NAME}",
111+
"version": "${PACKAGE_VERSION}",
112+
"scripts": {
113+
"test": "node ${SCRIPT_NAME}"
114+
},
115+
"bin": "${SCRIPT_NAME}"
116+
}`,
102117
})
103118

104119
// The removal of this fixture causes this test to fail when done with
@@ -112,6 +127,12 @@ t.test('run shims', t => {
112127
// only cygwin *requires* the -l, but the others are ok with it
113128
args.unshift('-l')
114129
}
130+
if (cmd.toLowerCase().endsWith('powershell.exe') || cmd.toLowerCase().endsWith('pwsh.exe')) {
131+
// pwsh *requires* the -Command, Windows PowerShell defaults to it
132+
args.unshift('-Command')
133+
// powershell requires escaping double-quotes for this test
134+
args = args.map(elem => elem.replaceAll('"', '\\"'))
135+
}
115136
const result = spawnSync(`"${cmd}"`, args, {
116137
// don't hit the registry for the update check
117138
env: { PATH: path, npm_config_update_notifier: 'false' },
@@ -162,6 +183,7 @@ t.test('run shims', t => {
162183

163184
const shells = Object.entries({
164185
cmd: 'cmd',
186+
powershell: 'powershell',
165187
pwsh: 'pwsh',
166188
git: join(ProgramFiles, 'Git', 'bin', 'bash.exe'),
167189
'user git': join(ProgramFiles, 'Git', 'usr', 'bin', 'bash.exe'),
@@ -216,7 +238,7 @@ t.test('run shims', t => {
216238
}
217239
})
218240

219-
const matchCmd = (t, cmd, bin, match) => {
241+
const matchCmd = (t, cmd, bin, match, params, expected) => {
220242
const args = []
221243
const opts = {}
222244

@@ -227,25 +249,40 @@ t.test('run shims', t => {
227249
case 'bash.exe':
228250
args.push(bin)
229251
break
252+
case 'powershell.exe':
230253
case 'pwsh.exe':
231254
args.push(`${bin}.ps1`)
232255
break
233256
default:
234257
throw new Error('unknown shell')
235258
}
236259

237-
const isNpm = bin === 'npm'
238-
const result = spawnPath(cmd, [...args, isNpm ? 'help' : '--version'], opts)
260+
const result = spawnPath(cmd, [...args, ...params], opts)
261+
262+
// skip the first 3 lines of "npm test" to get the actual script output
263+
if (params[0].startsWith('test')) {
264+
result.stdout = result.stdout?.toString().split('\n').slice(3).join('\n').trim()
265+
}
239266

240267
t.match(result, {
241268
status: 0,
242269
signal: null,
243270
stderr: '',
244-
stdout: isNpm ? `npm@${version} ${ROOT}` : version,
271+
stdout: expected,
245272
...match,
246-
}, `${cmd} ${bin}`)
273+
}, `${cmd} ${bin} ${params[0]}`)
247274
}
248275

276+
// Array with command line parameters and expected output
277+
const tests = [
278+
{ bin: 'npm', params: ['help'], expected: `npm@${version} ${ROOT}` },
279+
{ bin: 'npx', params: ['--version'], expected: version },
280+
{ bin: 'npm', params: ['test'], expected: '' },
281+
{ bin: 'npm', params: [`test -- hello -p1 world -p2 "hello world" --q1=hello world --q2="hello world"`], expected: `hello\n-p1\nworld\n-p2\nhello world\n--q1=hello\nworld\n--q2=hello world` },
282+
{ bin: 'npm', params: ['test -- a=1,b=2,c=3'], expected: `a=1,b=2,c=3` },
283+
{ bin: 'npx', params: ['. -- a=1,b=2,c=3'], expected: `a=1,b=2,c=3` },
284+
]
285+
249286
// ensure that all tests are either run or skipped
250287
t.plan(shells.length)
251288

@@ -259,9 +296,17 @@ t.test('run shims', t => {
259296
}
260297
return t.end()
261298
}
262-
t.plan(2)
263-
matchCmd(t, cmd, 'npm', match)
264-
matchCmd(t, cmd, 'npx', match)
299+
t.plan(tests.length)
300+
for (const { bin, params, expected } of tests) {
301+
if (name === 'cygwin bash' && (
302+
(bin === 'npm' && params[0].startsWith('test')) ||
303+
(bin === 'npx' && params[0].startsWith('.'))
304+
)) {
305+
t.skip("`cygwin bash` doesn't respect option `{ cwd: path }` when calling `spawnSync`")
306+
} else {
307+
matchCmd(t, cmd, bin, match, params, expected)
308+
}
309+
}
265310
})
266311
}
267312
})

0 commit comments

Comments
 (0)