fix(powershell): use Invoke-Expression to pass args #8267

alexsch01 · 2025-05-01T17:10:05Z

Reopening of #7458

Co-authored-by: noseratio <[email protected]>

mbtools · 2025-05-01T20:02:55Z

Hi Alex,

Any change here should go along with new tests that prove what's not working (before the change) but working after. This includes executing npm run xxx from the command line and npm run xxx in a script in package.json.

I have updated test/bin/windows-shims.js to cover passing regular and named parameters to scripts.

windows-shims.txt

Please include this in the PR.

On my machine (pwsh 7.5.0, npm 10.9.2), all parameters are passed correctly with the current npm.ps1.

Do you have other tests that fail? Please add them.

@mbtools

…ipts Co-authored-by: @mbtools

alexsch01 · 2025-05-02T11:08:49Z

@mbtools thanks! I'll work on new tests for this PR later today

alexsch01 · 2025-05-02T15:01:40Z

@mbtools

On my machine (pwsh 7.5.0, npm 10.9.2), all parameters are passed correctly with the current npm.ps1.

npm test -- --param1 -p doesn't work in Windows PowerShell and PowerShell 7.5.1 (pwsh) using npm 10.9.2

it doesn't pass --param1 and -p
this PR fixes that

alexsch01 · 2025-05-02T15:36:17Z

This PR is ready for review

alexsch01 · 2025-05-03T12:49:40Z

Below "Output" is from running the windows-shims.js from upstream WITHOUT my PR

This should pass before adding my PR changes

@mbtools Ok I have an issue running the tests on my Windows machine

Without my PR, running node .\test\bin\windows-shims.js gives 2 failures

line number 240 and line number 253 from https://github.com/npm/cli/blob/latest/test/bin/windows-shims.js

commit 2d1d8d0 if this "latest" file changes

Output

PS C:\Users\FakeUser\Downloads\cli> node .\test\bin\windows-shims.js
TAP version 14

Subtest: shim contents

1..3
# Subtest: bash
    ok 1 - should be equivalent strictly
    ok 2 - all other changes are m->x
    1..2
ok 1 - bash # time=4.053ms

# Subtest: cmd
    ok 1 - should be equivalent strictly
    ok 2 - all other changes are m->x
    1..2
ok 2 - cmd # time=1.276ms

# Subtest: pwsh
    ok 1 - should be equivalent strictly
    ok 2 - all other changes are m->x
    1..2
ok 3 - pwsh # time=2.412ms

ok 1 - shim contents # time=12.605ms

Subtest: node-gyp

ok 1 - node-gyp contains env var
ok 2 - node-gyp contains path
ok 3 - node-gyp.cmd contains env var
ok 4 - node-gyp.cmd contains path
1..4

ok 2 - node-gyp # time=0.827ms

Subtest: run shims

1..6
# Subtest: cmd
    1..2
    not ok 1 - npm.cmd npm
      ---
      diff: |
        --- expected
        +++ actual
        @@ -1,4 +1,4 @@
         Object {
        -  "status": 0,
        -  "stdout": "[email protected] C:\\Users\\FakeUser\\Downloads\\cli",
        +  "status": 1,
        +  "stdout": "",
         }
      at:
        fileName: test\bin\windows-shims.js
        lineNumber: 240
        columnNumber: 7
        functionName: matchCmd
        isToplevel: true
      stack: |
        matchCmd (test/bin/windows-shims.js:240:7)
        Test.<anonymous> (test/bin/windows-shims.js:263:7)
        Test.<anonymous> (test/bin/windows-shims.js:253:7)
        Object.<anonymous> (test/bin/windows-shims.js:77:3)
      source: "    const result = spawnPath(cmd, [...args, isNpm ? 'help' :
        '--version'], opts)\r

        \r

        \    t.match(result, {\r

        ------^

        \      status: 0,\r

        \      signal: null,\n"
      ...

    not ok 2 - npx.cmd npx
      ---
      diff: |
        --- expected
        +++ actual
        @@ -1,4 +1,4 @@
         Object {
        -  "status": 0,
        -  "stdout": "11.3.0",
        +  "status": 1,
        +  "stdout": "",
         }
      at:
        fileName: test\bin\windows-shims.js
        lineNumber: 240
        columnNumber: 7
        functionName: matchCmd
        isToplevel: true
      stack: |
        matchCmd (test/bin/windows-shims.js:240:7)
        Test.<anonymous> (test/bin/windows-shims.js:264:7)
        Test.<anonymous> (test/bin/windows-shims.js:253:7)
        Object.<anonymous> (test/bin/windows-shims.js:77:3)
      source: "    const result = spawnPath(cmd, [...args, isNpm ? 'help' :
        '--version'], opts)\r

        \r

        \    t.match(result, {\r

        ------^

        \      status: 0,\r

        \      signal: null,\n"
      ...

not ok 1 - cmd # time=1434.598ms
  ---
  at:
    fileName: test\bin\windows-shims.js
    lineNumber: 253
    columnNumber: 7
    typeName: Test
  source: "\r

    \  for (const { cmd, skip, name, match } of shells) {\r

    \    t.test(name, t => {\r

    ------^

    \      if (skip.reason) {\r

    \        if (skip.fail) {\n"
  ...

# Subtest: pwsh
    ok 1 - pwsh - not installed # SKIP
    1..1
ok 2 - pwsh # time=0.61ms

# Subtest: git bash
    ok 1 - git bash - not installed # SKIP
    1..1
ok 3 - git bash # time=0.507ms

# Subtest: user git bash
    ok 1 - user git bash - not installed # SKIP
    1..1
ok 4 - user git bash # time=1.478ms

# Subtest: wsl bash
    ok 1 - wsl bash - not installed # SKIP
    1..1
ok 5 - wsl bash # time=1.499ms

# Subtest: cygwin bash
    ok 1 - cygwin bash - not installed # SKIP
    1..1
ok 6 - cygwin bash # time=1.304ms

not ok 3 - run shims # time=1907.767ms

at:
fileName: test\bin\windows-shims.js
lineNumber: 77
columnNumber: 3
typeName: Object
source: "})\r

\r

t.test('run shims', t => {\r

--^

\  const path = t.testdir({\r

\    ...SHIMS,\n"

...

1..3

{ total: 17, pass: 10, fail: 2, skip: 5 }

time=1953.236ms

PS C:\Users\FakeUser\Downloads\cli>

mbtools · 2025-05-03T12:58:28Z

This is not the same js as in this PR:

Your Log: const result = spawnPath(cmd, [...args, isNpm ? 'help' : '--version'], opts)\r
PR: const result = spawnPath(cmd, [...args, ...params], opts)

alexsch01 · 2025-05-03T12:59:49Z

This is not the same js as in this PR:

Your Log: const result = spawnPath(cmd, [...args, isNpm ? 'help' : '--version'], opts)\r PR: const result = spawnPath(cmd, [...args, ...params], opts)

Right...I meant I'm getting 2 failing tests before adding my PR

mbtools · 2025-05-03T13:55:43Z

That's with Windows PowerShell? I'm not sure if this ever was an option (or tested). Here it's about cmd, pwsh, and bash. If we can make it work for the older Windows PowerShell as well, why not?

noseratio · 2025-05-03T14:05:18Z

My 2c. If it's possible to make it work for both legacy (v5.0 and below) and modern (v6+) PowerShell with little efforts, that's great. Otherwise, I'd say specifying #Requires -Version 6.0 should be enough. For Windows people, it's been long overdue to move over to the modern portable PowerShell.

alexsch01 · 2025-05-03T20:12:26Z

I manually tested that echo 'test'; npm help a=1,b=2,c=3 now works with this PR (locally not the test) in both Windows PowerShell and pwsh

alexsch01 · 2025-05-03T22:17:05Z

the 2 failing tests I get on my machine do not appear in GitHub Actions CI so it must be my setup issue

alexsch01 · 2025-05-06T00:04:57Z

Thanks for the feedback! I'll look into the not so good part tomorrow

I also just made the ps1 files simpler by going back to the old logic if the grave key is found in the command....too many edge cases with that one

…e-Expression

test/bin/windows-shims.js

alexsch01 · 2025-05-06T17:53:46Z

Ok this PR looks complete to me, but if there's anything else to change please let me know

wraithgar · 2025-05-07T03:25:52Z

Letting CI run. Will let @mbtools weigh in here before landing. Node 24 is now live with npm 11 so it will be good to get this landed.

mbtools · 2025-05-07T09:37:28Z

👍

Let's add single quoted parameters to the test:

npm run test -- hello -p1 world -p2 "hello world" -p3 'hello world' --q1=hello world --q2="hello world" --q3='hello world'

hello
-p1
world
-p2
hello world
-p3
hello world
--q1=hello
world
--q2=hello world
--q3=hello world

I'm not sure how you came up with the $NPM_OG_COMMAND syntax but it works like a charm. Thank you!

Just a bit of styling to keep "exit" at the end:

if { 
  #...

  # Support pipeline input
  if ($MyInvocation.ExpectingInput) {
    $input | Invoke-Expression "& `"$NODE_EXE`" `"$NPM_CLI_JS`" $NPM_ARGS"
  } else {
    Invoke-Expression "& `"$NODE_EXE`" `"$NPM_CLI_JS`" $NPM_ARGS"
  }
} else {
  # Support pipeline input
  if ($MyInvocation.ExpectingInput) {
    $input | & $NODE_EXE $NPM_CLI_JS $args
  } else {
    & $NODE_EXE $NPM_CLI_JS $args
  }
}

exit $LASTEXITCODE

mbtools · 2025-05-07T10:32:28Z

with single quotes, it works on the command line but fails the spawn test with cmd and pwsh:

alexsch01 · 2025-05-07T10:32:32Z

@mbtools

CMD treats single-quotes as literals - output on my machine

hello
-p1
world
-p2
hello world
-p3
'hello
world'
--q1=hello
world
--q2=hello world
--q3='hello
world'

Don't know why pwsh7 is not passing the test

alexsch01 · 2025-05-07T10:41:38Z

It almost feels like pwsh7 and cmd output always match with spawnSync ??

const { spawnSync } = require('child_process')

const cmd = 'pwsh'
const result = spawnSync(`"${cmd}"`, [`.\\abc.ps1 "hello world" 'hi world' a=1,b=2,c=3`], {
	shell: true,
        cwd: __dirname,
})

if(result.stderr.length > 0) throw new Error(result.stderr.toString().trim())
console.log(result.stdout.toString().trim())

>node script.js
hello world
'hi
world'
a=1,b=2,c=3

const { spawnSync } = require('child_process')

const cmd = 'powershell'
const result = spawnSync(`"${cmd}"`, [`.\\abc.ps1 "hello world" 'hi world' a=1,b=2,c=3`], {
	shell: true,
        cwd: __dirname,
})

if(result.stderr.length > 0) throw new Error(result.stderr.toString().trim())
console.log(result.stdout.toString().trim())

>node script.js
hello
world
hi world
a=1
b=2
c=3

mbtools · 2025-05-07T10:48:42Z

Let's remove the single quote tests for now. It doesn't look related to the ps1 scripts. We will figure it out after (new issue).

This PR is hard to parse now. Could you create a new PR with a single commit and summary of the change (co-pilot 😉), please?

alexsch01 · 2025-05-07T11:15:43Z

Continuing in #8278

mbtools · 2025-05-07T11:33:53Z

PS: Set $env:NODE_DEBUG="*" and run node script.js. Then you see how spawn will map it back to cmd.exe and sets args (due to shell: true). So in the end it's always cmd.exe.

alexsch01 · 2025-05-07T14:53:05Z

ahhhh cmd /d /s /c "pwsh -Command .\abc.ps1 "hello world" 'hi world' a=1,b=2,c=3" gives the expected output

hello
world
hi world
a=1
b=2
c=3

EDIT: I updated the new PR

alexsch01 · 2025-05-07T15:38:29Z

@mbtools with the single-quote tests which I'm not merging into my branch.....we now get this https://github.com/alexsch01/cli/actions/runs/14887289051/job/41810161408

Only CMD is now failing!!! which is expected for CMD

ehoogeveen-medweb · 2025-05-07T15:48:44Z

Yeah, cmd /d /s /c "pwsh -Command .\abc.ps1 "hello world" 'hi world' a=1,b=2,c=3" should evaluate pwsh -Command .\abc.ps1 "hello world" 'hi world' a=1,b=2,c=3 which would pass "hello world" as a single argument but split 'hi world' into 'hi and world' since CMD has no special handling for single quotes, right? You said it's expected, so just confirming since you seemed to report success in your previous comment.

alexsch01 · 2025-05-07T15:51:47Z

Yeah, cmd /d /s /c "pwsh -Command .\abc.ps1 "hello world" 'hi world' a=1,b=2,c=3" should evaluate pwsh -Command .\abc.ps1 "hello world" 'hi world' a=1,b=2,c=3 which would pass "hello world" as a single argument but split 'hi world' into 'hi and world' since CMD has no special handling for single quotes, right? You said it's expected, so just confirming since you seemed to report success in your previous comment.

yes the test is good that's what I meant
We aren't going to use the single-quote test

@mbtools

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 ```

fix(powershell): use Invoke-Expression to pass args

755c75a

Co-authored-by: noseratio <[email protected]>

alexsch01 requested a review from a team as a code owner May 1, 2025 17:10

This was referenced May 1, 2025

fix(powershell): use Invoke-Expression to pass args #7458

Closed

Cypress PowerShell: broken comma-separated-values CLI parsing since NodeJS 22.0.0 cypress-io/cypress-documentation#6166

Closed

$MyInvocation.Statement is undefined in Windows PowerShell

5981a1b

windows-shims.js: cover passing regular and named parameters to scr…

d37f8f9

…ipts Co-authored-by: @mbtools

alexsch01 added 4 commits May 2, 2025 09:31

add tests for comma-separated values

7a931ed

remove test that doesn't work in command prompt

6ac5541

add npx tests

230fcd8

correct test

d18d75e

alexsch01 added 7 commits May 3, 2025 14:38

small change

f3ff347

Update windows-shims.js

ece490b

Update windows-shims.js

f446d38

make less strict

a4077f5

oops

9f8bb56

npm.ps1 fix multiple commands

fee22f1

npx.ps1 fix multiple commands

3a4bde2

Update windows-shims.js

16ee512

formatting change

d8bd5a2

make simpler

60161d2

alexsch01 added 3 commits May 5, 2025 20:08

oops

622f7fe

properly quote $NODE_EXE and $NPM_CLI_JS/$NPX_CLI_JS with Invok…

09f002d

…e-Expression

use reflection to get the full powershell command

cd4eab7

wraithgar reviewed May 6, 2025

View reviewed changes

test/bin/windows-shims.js Show resolved Hide resolved

use $MyInvocation.Statement if exists already

a128038

wraithgar mentioned this pull request May 6, 2025

[BUG] cygwin bash does not support CWD parameter npm/statusboard#980

Open

1 task

alexsch01 added 2 commits May 6, 2025 13:29

combine tests

d74a744

fix test

8235520

make simpler

59e56c0

alexsch01 added 2 commits May 7, 2025 05:56

single exit for ps1 bin files, single-quote test

81f2426

Merge branch 'npm:latest' into latest

7386bde

alexsch01 mentioned this pull request May 7, 2025

fix(powershell): use Invoke-Expression to pass args #8278

Merged

alexsch01 closed this May 7, 2025

alexsch01 deleted the latest branch May 7, 2025 11:23

fix(powershell): use Invoke-Expression to pass args #8267

fix(powershell): use Invoke-Expression to pass args #8267

Uh oh!

Conversation

alexsch01 commented May 1, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

mbtools commented May 1, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

alexsch01 commented May 2, 2025

Uh oh!

alexsch01 commented May 2, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

alexsch01 commented May 2, 2025

Uh oh!

alexsch01 commented May 3, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Output

Subtest: shim contents

Subtest: node-gyp

Subtest: run shims

not ok 3 - run shims # time=1907.767ms

{ total: 17, pass: 10, fail: 2, skip: 5 }

time=1953.236ms

Uh oh!

mbtools commented May 3, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

alexsch01 commented May 3, 2025

Uh oh!

mbtools commented May 3, 2025

Uh oh!

noseratio commented May 3, 2025

Uh oh!

alexsch01 commented May 3, 2025

Uh oh!

alexsch01 commented May 3, 2025

Uh oh!

alexsch01 commented May 6, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

alexsch01 commented May 6, 2025

Uh oh!

wraithgar commented May 7, 2025

Uh oh!

mbtools commented May 7, 2025

Uh oh!

mbtools commented May 7, 2025

Uh oh!

alexsch01 commented May 7, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

alexsch01 commented May 7, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

mbtools commented May 7, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

alexsch01 commented May 7, 2025

Uh oh!

mbtools commented May 7, 2025

Uh oh!

alexsch01 commented May 7, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

alexsch01 commented May 7, 2025

Uh oh!

ehoogeveen-medweb commented May 7, 2025

Uh oh!

alexsch01 commented May 7, 2025

Uh oh!

Uh oh!

alexsch01 commented May 1, 2025 •

edited

Loading

mbtools commented May 1, 2025 •

edited

Loading

alexsch01 commented May 2, 2025 •

edited

Loading

alexsch01 commented May 3, 2025 •

edited

Loading

mbtools commented May 3, 2025 •

edited

Loading

alexsch01 commented May 6, 2025 •

edited

Loading

alexsch01 commented May 7, 2025 •

edited

Loading

alexsch01 commented May 7, 2025 •

edited

Loading

mbtools commented May 7, 2025 •

edited

Loading

alexsch01 commented May 7, 2025 •

edited

Loading