Make cmd optional

[timdp]

child_process.spawn recently got a shell option that spawns cmd.exe for you on Windows, pretty much the way cross-spawn does. However, as discussed in nodejs/node#6671 and nodejs/node#6784, it signifcantly impacts performance.

To avoid unnecessarily slowing down the spawn call, I suggested looking at the file extension to determine if a shell is actually required. The PR got rejected, which I understand (see link above), but I think it makes sense to include the same logic in cross-spawn.

Before I actually create a PR, I'd like to test the water first. Do you think it's a good idea?

[satazor]

So basically, using cmd.exe makes the spawn a lot slower if I understood correctly. The detection to whether use cmd or not must be clever. There's tooling such as del that are only available when running with the windows shell.

If it doesn't cause any harm, I'm willing to say it's a good idea.

[timdp]

del might be quite easy to cover: we'd spawn cmd.exe by default and only bypass it if the file extension is in the whitelist (basically .exe and .com, I think). However, that'll mean that we also spawn a shell for commands that don't specify the extension, which will be the vast majority of cross-platform code.

Alternatively, perhaps we should maintain a list of commands included in the output of cmd's help command?

Another question is, should cross-spawn support del by default (i.e., without a proprietary option)? It's not spawning del, it's spawning cmd and running its del command, so the behavior is somewhat magical.

[satazor]

del might be quite easy to cover: we'd spawn cmd.exe by default and only bypass it if the file extension is in the whitelist (basically .exe and .com, I think). However, that'll mean that we also spawn a shell for commands that don't specify the extension, which will be the vast majority of cross-platform code.

Alternatively, perhaps we should maintain a list of commands included in the output of cmd's help command?

cross-spawn resolves the command to a file using which. This means that even if you type git, it will resolve to {path}/git.exe on windows. I think that analysing the extension of the resolved file is the best option.

Another question is, should cross-spawn support del by default (i.e., without a proprietary option)? It's not spawning del, it's spawning cmd and running its del command, so the behavior is somewhat magical.

Removing the support for it would be a breaking change. cross-spawn supports it since the initial release and it's stated in the README.

[timdp]

That's what semver is for. ;-)

[satazor]

That's what semver is for. ;-)

I know but keeping it won't interfere with the solution we are agreeing with. When attempting to resolve del to a file, it will return null. Since we ONLY avoid cmd.exe for known extensions, del would still run through cmd.exe.

[timdp]

I agree. Just wanted to point out that breaking changes wouldn't necessarily be an issue, and that conceptually, del isn't being spawned.

At any rate, I'll see if I can come up with a PR that adds the extension check. I'll probably also add an option to bypass it, so people have an override for executables that do need to run in a shell for whatever reason.

[satazor]

sgtm