Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

tasks.json npm script does not work with arguments since 1.53 #115876

Closed
xmedeko opened this issue Feb 5, 2021 · 10 comments
Closed

tasks.json npm script does not work with arguments since 1.53 #115876

xmedeko opened this issue Feb 5, 2021 · 10 comments
Assignees
Labels
bug Issue identified by VS Code Team member as probable bug candidate Issue identified as probable candidate for fixing in the next release tasks Task system issues verified Verification succeeded

Comments

@xmedeko
Copy link
Contributor

xmedeko commented Feb 5, 2021

  • VSCode Version: 1.53.0
  • OS Version: Windows 10

Steps to Reproduce:

I have a npm task (tasks.json):

{
    "label": "test:web current file",
    "type": "npm",
    "script": "test:web -- suites=\"${relativeFileDirname}/${fileBasenameNoExtension}\"",
}

It has been working fine until I have upgraded to 1.53. Since then, I got error:

The terminal process "C:\WINDOWS\System32\WindowsPowerShell\v1.0\powershell.exe -Command npm run 'test:web -- suites="tests\dir\File"'" terminated with exit code: 1.

Seems to me v1.53 has added shell escaping. So, how I can add arguments to a task npm?

Also, I am miss information about additional npm tasks properties in

Does this issue occur when all extensions are disabled?: Yes

@alexr00
Copy link
Member

alexr00 commented Feb 5, 2021

I didn't realize users were adding args to npm tasks in tasks.json. Looks like the only way to support npm scripts with spaces and npm scripts with args is to add support for the --.

@alexr00 alexr00 added bug Issue identified by VS Code Team member as probable bug tasks Task system issues candidate Issue identified as probable candidate for fixing in the next release and removed new release labels Feb 5, 2021
@alexr00 alexr00 added this to the January 2021 Recovery milestone Feb 5, 2021
@xmedeko
Copy link
Contributor Author

xmedeko commented Feb 5, 2021

@alexr00 Or, support args property and join the result properly: npm run script -- arg1 arg2 ...

@xmedeko
Copy link
Contributor Author

xmedeko commented Feb 5, 2021

@alexr00 You have probably forgotten your older comment with a npm script params #82626 (comment) 😉

alexr00 added a commit that referenced this issue Feb 5, 2021
@alexr00
Copy link
Member

alexr00 commented Feb 5, 2021

Indeed, I had forgotten that, thank you. Since this used to work and now does not, I'd like to fix it.

@xmedeko
Copy link
Contributor Author

xmedeko commented Feb 5, 2021

@alexr00 Is your commit a4a6607 working for #113750 particularly for the "[test] unit --watch" script name?

@alexr00
Copy link
Member

alexr00 commented Feb 5, 2021

No. As a user you can either have the old behavior where we support arguments, or you can have the new behavior where script names have spaces.

@xmedeko
Copy link
Contributor Author

xmedeko commented Feb 5, 2021

I am happy with the "old behavior where we support arguments". You should ask @jonamat if he is happy with breaking #113750

That's why I have proposed to add args instead (working with other task types already). Or maybe some explicit flag to switch the behavior - not to relay on the presence of -- in the script param. These automagic behaviour changes may be very confusing when not documented. (I have pointed lack of documentation before). So, if you want it keep it this way, it would be helpful to improve the documentation.

@xmedeko
Copy link
Contributor Author

xmedeko commented Feb 6, 2021

I have converted type npm to shell:

{
    "label": "test:web current file",
    "type": "shell",
    "command": "npm",
    "argv": [ "run", "test:web", "--", "suites=\"${relativeFileDirname}/${fileBasenameNoExtension}\"" ]
}

And it does the job well. IMHO either type npm is redundant, or it should behave similarly to the type shell (i.e. user argv array).

@alexr00
Copy link
Member

alexr00 commented Feb 8, 2021

The fix for this bug allows the following:

The fix for this bug does not allow you have a task that uses an npm script with a space and the --. Certainly having an args property would be better. But this is a case of something worked by happy accident (allowing arguments in npm scripts) and now we would prefer not to break the users who depend on it.

@xmedeko
Copy link
Contributor Author

xmedeko commented Feb 8, 2021

OK, just such automagic behaviour may be very confusing when not documented. Please, document the type: npm and the script property:

https://code.visualstudio.com/docs/editor/tasks - For a custom task, this can either be shell or process. - No npm there, no doc for script, etc.
https://code.visualstudio.com/docs/editor/tasks-appendix - type: 'shell' | 'process'; - No npm there, no doc for script, etc.

It's difficult to find what's a proper behaviour and what is a bug without doc.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
bug Issue identified by VS Code Team member as probable bug candidate Issue identified as probable candidate for fixing in the next release tasks Task system issues verified Verification succeeded
Projects
None yet
Development

No branches or pull requests

4 participants