concurrently
concurrently copied to clipboard
Add shell option / support `spawn` in configured api.
The default spawn behavior on Windows is spawn with cmd.exe, which is reasonable for compatibility, but makes it hard to kill due to cmd.exe prompting with "Terminate batch job (Y/N)?" for each command; among other issues. In most cases using pwsh.exe would be preferred.
Similarly, the shell on other platforms is hard-coded to /bin/sh which is quite a bit more limited than may be expected.
Worse, this behavior is essentially already implemented in Node's native spawn() function since v5.7.0 as the { shell: true } option - which additionally accepts a string for the shell (which uses essentially execFile(options.shell, ["-c", command])).
In either case, users may prefer to use a shell emulator like @yarnpkg/shell to get portable behavior.
There is an undocumented spawn option which you can use to override this implementation, but only for the unconfigured createConcurrently option, the default configured export seemingly accidentally doesn't forward it to the underlying createConcurrently() - requiring the caller to re-implement the default behavior. Adding a single spawn: options.spawn line works fine. Perhaps the configured concurrently should use something like return createConcurrently(commands, { outputStream: process.stdout, ...options, controllers: [<existing>, ...options.controllers] }) to forward all options by default.
Alternatively, the API could expose a new shell or spawnOptions which would override the default shell behavior, or even a shellEmulator: boolean option (to match e.g. the pnpm equivalent option) which uses @yarnpkg/shell.
The last issue about this, #533, provides some historical context.
I understand the frustration - I'm happy to start with allowing customising the shell through a few means - be it API, env variable, or both. Would like to eventually just run on the current shell, but i'm not sure about making it safely yet.
this behavior is essentially already implemented in Node's native spawn() function since v5.7.0 as the { shell: true } option
making this change to the default spawn behaviour would be good too 🙂
I'm not sure what you're referring to in that issue? That seems to be talking about changing the default behavior when used as a CLI, and the difficulty in implementing that behavior helpfully. I actually think it makes sense to use a "default" shell not the current one - the command being run is likely written with a particular implementation in mind, and regardless al the most common use even for the CLI is as npm scripts, which themselves are run in a configued shell for the same reason.
This issue, in contrast, is solely about better exposing the existing API functionality to allow callers to do whichever behavior makes sense for them, which can be as straightforward as adding the line to forward the spawn option and documenting it.
I've used the above change as a pnpm patch and it seems to work fine, in fact it seems a bit odd you have had this partially implemented spawn option (among others) for so long without documenting it. It's even referenced by the current configured implementation for one of the hooks, just not forwarded to the core to affect the user's commands so it seems like a bug.
The other mentioned approaches are mostly because this option isn't documented, so you may as well consider what behavior you want to actually document and support when you do so. For example, documenting and supporting just a shell: string | boolean you can forward to spawn might be something you feel happier about rather than documenting spawn itself.