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-05T11:07:42Z

It depends on how you pass the parameters to spawn. Args is an array but the test passes just one string (not split by parameters). The reason was testing the part after --. Now there are other tests that might require a split first.

The way we call spawn here actually must match how npm run script does it. Otherwise, the test results will be misleading. It's a separate package. I can look tomorrow.

it's bizarre spawnSync with Windows PowerShell gives what I think the correct behavior, where as spawnSync with pwsh7 gives what I think the wrong behavior

when I mean the correct behavior, I mean that it gives the same output for .\abc.ps1 first a=1,b=2,c=3 and here fix(powershell): use Invoke-Expression to pass args #8267 (comment)

For both Windows PowerShell and pwsh7, doing .\abc.ps1 first a=1,b=2,c=3 gives

first
a=1
b=2
c=3

alexsch01 · 2025-05-05T13:38:47Z

Converted to a draft.....

this will fail the scenario in Windows PowerShell

{
  bin: 'npm',
  params: ['test -- aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa'],
  expected: `aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa`
},

alexsch01 · 2025-05-05T15:45:27Z

Modifed the code to try to replicate $MyInvocation.Statement on Windows PowerShell

mbtools · 2025-05-05T16:16:48Z

I was afraid of this slight mess...

Here's how npm run script works. It uses @npmcli/promise-spawn which has this big issue. I have seen this before when porting pacote tests to Windows and left this comment.

And here we are again... Calling spawn on Windows is a very tricky thing. There are solution like cross-spawn but that's not an option here.

I know you're interested in getting Windows PowerShell to work. Can we get back to the primary goal to solve issues with passing named parameters to PowerShell7 ($args doesn't work)?

…specific pwsh.exe test

alexsch01 · 2025-05-05T19:44:40Z

OK @mbtools, I'm all done. Let me know what you think of it

mbtools · 2025-05-05T23:53:02Z

This is good

We are closer than ever 👍

Maybe we can combine some test cases. I used this one:

{ bin: 'npm', params: ['test -- -a 1 --b 2 --c=3 "4" --d="5"'], expected: -a\n1\n--b\n2\n--c=3\n4\n--d=5 },

Not so good

The solution fails if there are spaces in the path to node or npm-cli.js.

The full expression looks like this:

& C:\Program Files\nodejs/node.exe C:\Program Files\nodejs/node_modules/npm/bin/npm-cli.js run test

It does need the quoting 😉

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.

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

Open

$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

alexsch01 marked this pull request as draft May 5, 2025 13:32

alexsch01 added 4 commits May 5, 2025 11:31

complete change

1bfbd78

also for npx.ps1

360b090

oop

668d40d

formatting

72c9fdd

alexsch01 marked this pull request as ready for review May 5, 2025 15:44

alexsch01 added 4 commits May 5, 2025 14:10

remove normalize part

fb49f5b

check value of $MyInvocation.OffsetInLine

2d03217

finally get it to pass

204a228

add powershell.exe test, have default case for PS1 scripts, revert …

4befbeb

…specific pwsh.exe test

Merge branch 'npm:latest' into latest

8502ce0

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

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

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

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

alexsch01 commented May 1, 2025 •

edited

Loading

mbtools commented May 1, 2025 •

edited

Loading

alexsch01 commented May 2, 2025

alexsch01 commented May 2, 2025 •

edited

Loading

alexsch01 commented May 2, 2025

alexsch01 commented May 3, 2025 •

edited

Loading

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

mbtools commented May 3, 2025 •

edited

Loading

alexsch01 commented May 3, 2025

mbtools commented May 3, 2025

noseratio commented May 3, 2025

alexsch01 commented May 3, 2025

alexsch01 commented May 3, 2025

alexsch01 commented May 5, 2025 •

edited

Loading

alexsch01 commented May 5, 2025 •

edited

Loading

alexsch01 commented May 5, 2025

mbtools commented May 5, 2025

alexsch01 commented May 5, 2025

mbtools commented May 5, 2025 •

edited

Loading

alexsch01 commented May 6, 2025 •

edited

Loading

alexsch01 commented May 6, 2025

wraithgar commented May 7, 2025

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

Are you sure you want to change the base?

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

Conversation

alexsch01 commented May 1, 2025 • edited Loading

mbtools commented May 1, 2025 • edited Loading

alexsch01 commented May 2, 2025

alexsch01 commented May 2, 2025 • edited Loading

alexsch01 commented May 2, 2025

alexsch01 commented May 3, 2025 • edited Loading

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

mbtools commented May 3, 2025 • edited Loading

alexsch01 commented May 3, 2025

mbtools commented May 3, 2025

noseratio commented May 3, 2025

alexsch01 commented May 3, 2025

alexsch01 commented May 3, 2025

alexsch01 commented May 5, 2025 • edited Loading

alexsch01 commented May 5, 2025 • edited Loading

alexsch01 commented May 5, 2025

mbtools commented May 5, 2025

alexsch01 commented May 5, 2025

mbtools commented May 5, 2025 • edited Loading

This is good

Not so good

alexsch01 commented May 6, 2025 • edited Loading

alexsch01 commented May 6, 2025

wraithgar commented May 7, 2025

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 5, 2025 •

edited

Loading

alexsch01 commented May 5, 2025 •

edited

Loading

mbtools commented May 5, 2025 •

edited

Loading

alexsch01 commented May 6, 2025 •

edited

Loading