Fix child process spawning on Windows - #189
Open
franbarbalopez wants to merge 1 commit into
Open
Conversation
Author
|
Can't test this on Mac fyi @WendellAdriel |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Use
cross-spawnfor synchronous and asynchronous child processes invoked by the orchestrator scripts.This fixes command resolution on Windows for executables provided through
.batand.cmdwrappers, including Composer, npm, and npx.Problem
The orchestrator used Node.js
child_process.spawn()andspawnSync()directly:On Windows, tools such as Composer, npm, and npx are commonly exposed through wrapper scripts:
composer.batnpm.cmdnpx.cmdThese scripts cannot be executed directly by modern Node.js versions without a command shell. As a result, running:
failed while attempting to execute the generated kit setup:
Composer was installed and available on
PATH, but Node.js could not execute its Windows wrapper using the defaultspawn()behavior.Using
shell: truemade the command work, but it also produced the following deprecation warning:Therefore, enabling the shell was not an appropriate permanent solution.
Solution
Add
cross-spawnas a development dependency and use it for both asynchronous and synchronous process execution:cross-spawnhandles platform-specific executable resolution, including Windows.batand.cmdwrappers, while preserving the existingspawn()API and avoidingshell: true.This fixes command execution for:
Verification
The change was verified on Windows with Node.js 26 by executing Composer through both APIs:
spawn('composer', ['--version'])spawnSync('composer', ['--version'])Both commands completed successfully without
ENOENT,EINVAL, or theDEP0190warning.Alternatives considered
Another possible approach would be to detect Windows and resolve each command to its platform-specific wrapper:
However, this approach has several drawbacks:
.bat, while npm and npx commonly use.cmd..batand.cmdexecution withEINVALunless the command is launched throughcmd.exe.shell: trueto execute these wrappers triggers theDEP0190warning when arguments are passed separately.cmd.exewould require carefully implementing Windows command-line quoting and escaping.For example, a complete Windows-specific implementation would need to resemble:
This still requires special handling for argument escaping and shell metacharacters.
Using
cross-spawncentralizes this platform-specific behavior, preserves the existingspawn(command, args, options)API, and avoids maintaining a custom command resolution and escaping implementation.