-
-
Notifications
You must be signed in to change notification settings - Fork 32.4k
Open
Labels
docIssues and PRs related to the documentations.Issues and PRs related to the documentations.
Description
Affected URL(s)
https://nodejs.org/docs/latest-v24.x/api/child_process.html#spawning-bat-and-cmd-files-on-windows
Description of the problem
The suggestions here include using spawn
with shell: true
, however doing so is deprecated with DEP0190. This conflicting messaging is confusing and makes this difficult-to-use-safely API even more confusing to use for non-experts.
I don't know what the right call is here. My guess is that DEP0190 was created to nudge people away from using spawn
with shell: true
by either using it safely or explicitly using the insecure exec
command. Since the only alternative in the documentation is exec
I'd suggest removing the suggestion to use spawn
with shell: true
entirely.
Metadata
Metadata
Assignees
Labels
docIssues and PRs related to the documentations.Issues and PRs related to the documentations.
Type
Projects
Milestone
Relationships
Development
Select code repository
Activity
shell: true
for scripts ericcornelissen/shescape#2021juanarbol commentedon Jun 16, 2025
Nice catch.
Can you send a PR fixing this? :)
ericcornelissen commentedon Jun 17, 2025
Can do 👍
doc: update "Spawning `.bat` and `.cmd` files on Windows" section
.bat
and.cmd
files on Windows" section #58739Knagis commentedon Jun 19, 2025
But
exec
doesn't havestdio
option, so it isn't a proper replacement.ericcornelissen commentedon Jun 19, 2025
Interesting point,
execSync
does have that option, tho I guess using the sync version might not always be desirable... Could it be added to theexec
API?Knagis commentedon Jun 25, 2025
See #58763 (comment) - turns out that
spawn
accepts the command line arguments directly in the command parameter, and args array can be omitted. So that likely is the best option for the docs.