npm / cli

the package manager for JavaScript
https://docs.npmjs.com/cli/
Other
8.47k stars 3.16k forks source link

[BUG] Passing CLI arguments via a Node scripts no longer works #7375

Open JPStrydom opened 6 months ago

JPStrydom commented 6 months ago

Is there an existing issue for this?

This issue exists in the latest npm version

Current Behavior

With the following index.js NodeJS file in the root of a new clean NodeJS project:

console.log(process.argv.slice(2));

I get the following output when I run node ./index.js -test-arg test-arg-value:

[ '-test-arg', 'test-arg-value' ]

When I add an NPM script with:

  ...
  "scripts": {
    "arg-test": "node ./index.js",
    ...

and then run it with either of the following:

This leaves me unable to run CLI tools, such as Yargs, via NPM scripts like we used to be able to.

Expected Behavior

When using NPM to compose reusable scripts, CLI arguments should still be supported - which enables the use of CLI tools such as Yargs

Steps To Reproduce

  1. Using the latest NodeJS (20.12.0) and the latest NPM (10.5.2)
  2. With an index.js file in the root of a new NodeJS project containing the following:
    • index.js:
      console.log(process.argv.slice(2));
  3. Run the file with node ./index.js -test-arg test-arg-value
  4. See the expected output: [ '-test-arg', 'test-arg-value' ]
  5. Add an NPM script to the package.json file to run the file:
    • package.json:
      "type": "module",
      "scripts": {
      "arg-test": "node ./index.js"
      }
  6. Run the file with either of the following:
    • npm run arg-test -- -test-arg test-arg-value,
    • npm run arg-test -test-arg test-arg-value,
  7. See the incorrect output [ 'test-arg-value' ]

Notes:

Environment

//registry.npmjs.org/:_authToken = (protected)

; node bin location = C:\Program Files\nodejs\node.exe ; node version = v20.12.0 ; npm local prefix = C:\Data\Development\XXX\XXX ; npm version = 10.5.2 ; cwd = C:\Data\Development\XXX\XXX ; HOME = C:\Users\XXX ; Run npm config ls -l to show all defaults.

JPStrydom commented 6 months ago

It seems like adding another argument delimiter (--) fixes the issue.

i.e. running with npm run arg-test -- -- -test-arg test-arg-value causes the correct behavior. Not sure when Microsoft changes this, because only using on delimiter definitely used to work.

hp8wvvvgnj6asjm7 commented 6 months ago

same here, its broken! https://github.com/wclr/ts-node-dev/issues/345

noseratio commented 6 months ago

I'm also affected by this issue. It appears to be a shenanigan of both PowerShell and NPM working together. Node.js is not involved. A proof:

package.json:

{
  "name": "cli-test",
  "scripts": {
    "showcli": "echo",
    "showbatcli": "test.cmd"
  }
}

test.cmd:

@echo %*
PS C:\temp\cli-test> npm run showcli -- command --arg=value

> cli-test@1.0.0 showcli
> echo command

command

showbatcli test.cmd command

command


- Via CMD.exe, it works:

```text
PS C:\temp\cli-test> cmd /c npm run showcli -- command --arg=value

> cli-test@1.0.0 showcli
> echo command --arg=value

command --arg=value
PS C:\temp\cli-test2> ./test.cmd -- command --arg=value
-- command --arg=value
noseratio commented 6 months ago

Digging more into this, the problem appears to be with "C:\Program Files\nodejs\npm.ps1", which gets invoked when we type npm from a PowerShell prompt on Windows.

OTOH, running the npm.cmd ("C:\Program Files\nodejs\npm.cmd") explicitly works ok, a workaround I'm settling on for now:

PS C:\temp\cli-test2> npm.cmd run showbatcli -- command --arg=value

> showbatcli
> test.cmd command --arg=value

command --arg=value
lukekarrys commented 6 months ago

@noseratio Can you test with the latest branch which now includes new Powershell scripts as of 52306473da03123ef5623e9e152e10285c8097f3?

Confirmed this does not fix the issue.

lukekarrys commented 6 months ago

npm recently shipped this .ps1 script in addition to the existing .cmd script.

Looking at Powershell docs it appears that there are different semantics to get it to stop argument parsing. Can you try npm run showbatcli --% command --arg=value and see if it works as expected? AFAIK there isn't a good way to get Powershell to do this in our npm.ps1 script and it comes down to using a different shell and the specifics of escaping, etc within that shell.

At this point it might be a better idea for npm to revert the addition of the npm.ps1 and npx.ps1 scripts, but this would also need to land as a change to the Node.js installer.

noseratio commented 6 months ago

@lukekarrys, thanks for looking into this. On Windows, --% kind of works, but not the way -- has worked before. On Linux, --% doesn't work at all, which is a problem with portable build scripts.

E.g.:

package.json:

{
  "name": "cli-test",
  "version": "1.0.0",
  "main": "index.js",
  "scripts": {
    "start": "node index.js",
    "showcli": "echo",
    "showbatcli": "test.cmd"
  }
}

index.js:

console.dir(process.argv);

test.cmd:

@echo %*

With --% on Windows, "command --arg=value" now comes down quoted, which means it also comes as a single parameter in Node's process.argv[2], as opposed to the previos behavior, where it was command in process.argv[2] and --arg=value in process.argv[3], a breaking change:

PS C:\temp\cli-test> npm run showcli --% command --arg=value

> cli-test@1.0.0 showcli
> echo command --arg=value

"command --arg=value"
PS C:\temp\cli-test> npm run showbatcli --% command --arg=value

> cli-test@1.0.0 showbatcli
> test.cmd command --arg=value

"command --arg=value"
PS C:\temp\cli-test> npm start --% command --arg=value

> cli-test@1.0.0 start
> node index.js command --arg=value

[
  'C:\\Program Files\\nodejs\\node.exe',
  'C:\\temp\\cli-test\\index.js',
  'command --arg=value'
]

Note that npm.cmd fails with --%, --arg=value is lost:

PS C:\temp\cli-test> npm.cmd start --% command --arg=value

> cli-test@1.0.0 start
> node index.js command

[
  'C:\\Program Files\\nodejs\\node.exe',
  'C:\\temp\\cli-test\\index.js',
  'command'
]

Here is the correct wanted behavior (via explicit npm.cmd):

PS C:\temp\cli-test> npm.cmd start -- command --arg=value

> cli-test@1.0.0 start
> node index.js command --arg=value

[
  'C:\\Program Files\\nodejs\\node.exe',
  'C:\\temp\\cli-test\\index.js',
  'command',
  '--arg=value'
]

On Linux, --% doesn't work (--arg=value is lost), while -- still works as expected:

noseratio@i3msi:/mnt/c/temp/cli-test$ npm run showcli --% command --arg=value

> cli-test@1.0.0 showcli
> echo command

command
noseratio@i3msi:/mnt/c/temp/cli-test$ npm run showcli -- command --arg=value

> cli-test@1.0.0 showcli
> echo command --arg=value

command --arg=value
noseratio@i3msi:/mnt/c/temp/cli-test$ npm start --% command --arg=value

> cli-test@1.0.0 start
> node index.js command

[
  '/home/noseratio/.nvm/versions/node/v22.0.0/bin/node',
  '/mnt/c/temp/cli-test/index.js',
  'command'
]
noseratio@i3msi:/mnt/c/temp/cli-test$ npm start -- command --arg=value

> cli-test@1.0.0 start
> node index.js command --arg=value

[
  '/home/noseratio/.nvm/versions/node/v22.0.0/bin/node',
  '/mnt/c/temp/cli-test/index.js',
  'command',
  '--arg=value'
]
lukekarrys commented 6 months ago

@noseratio Thanks for the thorough reproductions and examples. I see how --% is not ideal. It "works" to preserve all parameters but having them only accessible as a single parameter is a deal breaker.

Tbh, I'm not sure how to get the same behavior of -- across cmd, powershell, and bash. I will continue to look into this. These .ps1 scripts were added as a feature request, but if it's not possible to maintain the same basic usage between powershell and the other shims then the answer might be to remove them entirely. I know this doesn't unblock you or anyone affected currently, but just a note that if we can't fix it, we can at least go back to having the cmd script take precedence again which should work as expected.

noseratio commented 6 months ago

@lukekarrys, thank you and maybe the following could help. Besides traditional $args (which only gives the remaining unbound args), there is a way to access the entire original command line in PowerShell, via $MyInvocation.Statement, and parse it your way. Also note $MyInvocation.BoundParameters and $MyInvocation.UnboundParameters, they can be useful:

E.g.:

ps-test.ps1:

param (
    [string]$firstArg
)

Write-Host "First Parameter: $firstArg"
Write-Host "Remaining Arguments: $args"
Write-Host "PSCommandPath: $PSCommandPath"
Write-Host "MyInvocation: $($MyInvocation | ConvertTo-Json)"
Write-Host "Process command line: $([System.Environment]::CommandLine)"

Running it:

PS C:\temp\cli-test> ./ps-test start -- command --arg=value
First Parameter: start
Remaining Arguments: command --arg=value
PSCommandPath: C:\temp\cli-test\ps-test.ps1
WARNING: Resulting JSON is truncated as serialization has exceeded the set depth of 2.
MyInvocation: {
  "MyCommand": {
    "Path": "C:\\temp\\cli-test\\ps-test.ps1",
    "Definition": "C:\\temp\\cli-test\\ps-test.ps1",
    "Source": "C:\\temp\\cli-test\\ps-test.ps1",
    "Visibility": 0,
    "ScriptBlock": {
      "Attributes": "",
      "File": "C:\\temp\\cli-test\\ps-test.ps1",
      "IsFilter": false,
      "IsConfiguration": false,
      "Module": null,
      "StartPosition": "System.Management.Automation.PSToken",
      "DebuggerHidden": false,
      "Id": "892f6269-b42d-45fa-9732-83c831400669",
      "Ast": "param (\r\n    [string]$firstArg\r\n)\r\n\r\nWrite-Host \"First Parameter: $firstArg\"\r\nWrite-Host \"Remaining Arguments: $args\"\r\nWrite-Host \"PSCommandPath: $PSCommandPath\"\r\nWrite-Host \"MyInvocation: $($MyInvocation | ConvertTo-Json)\"\r\nWrite-Host \"Original command line: $([System.Environment]::CommandLine)\"\r\n"
    },
    "OutputType": [],
    "ScriptContents": "param (\r\n    [string]$firstArg\r\n)\r\n\r\nWrite-Host \"First Parameter: $firstArg\"\r\nWrite-Host \"Remaining Arguments: $args\"\r\nWrite-Host \"PSCommandPath: $PSCommandPath\"\r\nWrite-Host \"MyInvocation: $($MyInvocation | ConvertTo-Json)\"\r\nWrite-Host \"Original command line: $([System.Environment]::CommandLine)\"\r\n",
    "OriginalEncoding": {
      "Preamble": null,
      "BodyName": "utf-8",
      "EncodingName": "Unicode (UTF-8)",
      "HeaderName": "utf-8",
      "WebName": "utf-8",
      "WindowsCodePage": 1200,
      "IsBrowserDisplay": true,
      "IsBrowserSave": true,
      "IsMailNewsDisplay": true,
      "IsMailNewsSave": true,
      "IsSingleByte": false,
      "EncoderFallback": "System.Text.EncoderReplacementFallback",
      "DecoderFallback": "System.Text.DecoderReplacementFallback",
      "IsReadOnly": true,
      "CodePage": 65001
    },
    "Name": "ps-test.ps1",
    "CommandType": 16,
    "Version": null,
    "ModuleName": "",
    "Module": null,
    "RemotingCapability": 1,
    "Parameters": {
      "firstArg": "System.Management.Automation.ParameterMetadata"
    },
    "ParameterSets": [
      "[[-firstArg] <string>]"
    ]
  },
  "BoundParameters": {
    "firstArg": "start"
  },
  "UnboundArguments": [
    "command",
    "--arg=value"
  ],
  "ScriptLineNumber": 1,
  "OffsetInLine": 1,
  "HistoryId": 1,
  "ScriptName": "",
  "Line": "./ps-test start -- command --arg=value",
  "Statement": "./ps-test start -- command --arg=value",
  "PositionMessage": "At line:1 char:1\r\n+ ./ps-test start -- command --arg=value\r\n+ ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~",
  "PSScriptRoot": "",
  "PSCommandPath": null,
  "InvocationName": "./ps-test",
  "PipelineLength": 1,
  "PipelinePosition": 1,
  "ExpectingInput": false,
  "CommandOrigin": 0,
  "DisplayScriptPosition": null
}
Process command line: "C:\Program Files\PowerShell\7\pwsh.dll" –noprofile
noseratio commented 6 months ago

@lukekarrys, I've now invested more time into this and come up with a working fix, based on what I said above. Here is a complete version of "C:\Program Files\nodejs\npm.ps1" file that works as expected with --:

#!/usr/bin/env pwsh
$basedir=Split-Path $MyInvocation.MyCommand.Definition -Parent

$exe=""
if ($PSVersionTable.PSVersion -lt "6.0" -or $IsWindows) {
  # Fix case when both the Windows and Linux builds of Node
  # are installed in the same directory
  $exe=".exe"
}
$ret=0

$nodeexe = "node$exe"
$nodebin = $(Get-Command $nodeexe -ErrorAction SilentlyContinue -ErrorVariable F).Source
if ($nodebin -eq $null) {
  Write-Host "$nodeexe not found."
  exit 1
}

$nodedir = Split-Path $nodebin

$npmprefixjs="$nodedir/node_modules/npm/bin/npm-prefix.js"
$npmprefix=(& $nodeexe $npmprefixjs)
if ($LASTEXITCODE -ne 0) {
  Write-Host "Could not determine Node.js install directory"
  exit 1
}
$npmprefixclijs="$npmprefix/node_modules/npm/bin/npm-cli.js"

$npmparams = $MyInvocation.Statement.Substring($MyInvocation.InvocationName.Length).Trim()
$invokenpm = "$nodeexe $npmprefixclijs $npmparams"

# Support pipeline input
if ($MyInvocation.ExpectingInput) {
  $input | Invoke-Expression $invokenpm
} else {
  Invoke-Expression $invokenpm
}
$ret=$LASTEXITCODE
exit $ret

The fix itself, in a nutshell:

$npmparams = $MyInvocation.Statement.Substring($MyInvocation.InvocationName.Length).Trim()
$invokenpm = "$nodeexe $npmprefixclijs $npmparams"
# ...
Invoke-Expression $invokenpm

I've also replaced:

$nodedir = $(New-Object -ComObject Scripting.FileSystemObject).GetFile("$nodebin").ParentFolder.Path

with:

$nodedir = Split-Path $nodebin

which produces the same result. I can't think of any benefits of using legacy, Windows-only COM objects nowadays, unless there is something really subtle? In which case, a cross-platform [System.IO.Path]::GetDirectoryName() might still be a better option.

Please feel free to create a PR for this fix, as I don't have access rights to this repo. I understand it will have to make its way into a future Node.js release, to become a proper fix, and can't be fixed with just an NPM update. Still better than nothing, I recon 🙂

Thanks much!

anonrig commented 6 months ago

@lemire and I fixed the problem on node --run at https://github.com/nodejs/node/pull/52810

noseratio commented 6 months ago

My current take at patching "C:\Program Files\nodejs\npm.ps1": https://github.com/npm/cli/pull/7458#issuecomment-2093920138

HannaTarasevich commented 5 months ago

Hi @lukekarrys, Is there any update? We're still limping with Powershell, the arguments cannot be passed

Mentioned workaround does not help: npm run test-local -- command --TEST_ENV=ENV --tags=@TC turns into wdio wdio.conf.js command

npm run test-local --TEST_ENV=ENV --tags=@TC turns into wdio wdio.conf.js

Previous and expected behavior for Powershell: turns into wdio wdio.conf.js --TEST_ENV=ENV --tags=@TC

hp8wvvvgnj6asjm7 commented 5 months ago

this came after i updated node & npm, not powershell.

reduckted commented 4 months ago

This sounds very much like the same problem as #3136. I posted a workaround that may help some people: https://github.com/npm/cli/issues/3136#issuecomment-1968089218

AnderssonPeter commented 2 months ago

We just upgraded from node 20 to node 22, and now we need -- -- instead of -- to pass additional arguments. Will this be fixed mid 22 and break our pipelines again or will it be this way until node 23?

dartess commented 2 weeks ago

I'm not sure if this is related to this issue, but I also can't seem to pass any additional arguments when running tests, like node --test "**/*.test.js"

noseratio commented 1 week ago

Nowadays, every time there is a new Node.js release on Windows which I just have installed (v23.1.0 at the time of writing), the next thing I do is to execute sudo cmd /k "C:\Program Files\nodejs\_patch-npm-ps1.bat", to patch npm.ps1 and make -- work again.

Where C:\Program Files\nodejs\_patch-npm-ps1.bat is:

@echo off
cd /d "%~dp0"
fc /a npm.ps1 npm.ps1.orig >nul
if errorlevel 1 goto err
copy npm.ps1 npm.ps1.orig 
copy npm.ps1.patch npm.ps1
exit /b 0
:err
echo Something has changed in npm.ps1, exiting without patching.
exit /b 1

And npm.ps1.patch is:

#!/usr/bin/env pwsh

$NODE_EXE="$PSScriptRoot/node.exe"
if (-not (Test-Path $NODE_EXE)) {
  $NODE_EXE="$PSScriptRoot/node"
}
if (-not (Test-Path $NODE_EXE)) {
  $NODE_EXE="node"
}

$NPM_PREFIX_JS="$PSScriptRoot/node_modules/npm/bin/npm-prefix.js"
$NPM_CLI_JS="$PSScriptRoot/node_modules/npm/bin/npm-cli.js"
$NPM_PREFIX=(& $NODE_EXE $NPM_PREFIX_JS)

if ($LASTEXITCODE -ne 0) {
  Write-Host "Could not determine Node.js install directory"
  exit 1
}

$NPM_PREFIX_NPM_CLI_JS="$NPM_PREFIX/node_modules/npm/bin/npm-cli.js"
if (Test-Path $NPM_PREFIX_NPM_CLI_JS) {
  $NPM_CLI_JS=$NPM_PREFIX_NPM_CLI_JS
}

function Normalize {
    [CmdletBinding()]
    param(
        [Parameter(Mandatory=$true, ValueFromPipeline=$true, Position=0)]
        [string]$Path
    )

    $Path = [System.IO.Path]::GetFullPath($Path)
    # remove trailing " or ' quotes (if any) and put back " quotes around the path
    $Path = $Path -replace '^\s*"\s*(.*?)\s*"\s*$', '$1'
    $Path = $Path -replace "^\s*'\s*(.*?)\s*'\s*$", "$1"
    return """$Path"""
}

$NPM_ARGS = $MyInvocation.Statement.Substring($MyInvocation.InvocationName.Length).Trim()
$INVOKE_NPM = "& $(Normalize $NODE_EXE) $(Normalize $NPM_CLI_JS) $NPM_ARGS"

# Support pipeline input
if ($MyInvocation.ExpectingInput) {
  $input | Invoke-Expression $INVOKE_NPM
} else {
  Invoke-Expression $INVOKE_NPM
}

exit $LASTEXITCODE

Solves the problem for local Windows development, but of course it is a nightmare for CI builds.

I hope my proposed patch will be merged in one day 🙏