Get rid of `cross-spawn` dependency
It's pretty unmaintained.
We do a lot to the shell, so would be simpler to do that too in execa ourselves.
Maybe some of the things it fixes are already fixed in Node.js too.
https://github.com/moxystudio/node-cross-spawn/blob/master/lib/parse.js
I agree that it not being maintained is a problem. Some of the bug reports seem rather important and are not currently looked into.
Breaking down cross-spawn feature by feature (which are all fixing problems with Windows):
- Executing files that have a shabang (since that's a Unix concept). Workaround for users would be to specify the interpreter explicitly instead. This would not work if the script location relies on the
PATHenvironment variable, as opposed to being a file path. It does so by usingwhichon the command, then reading the first bytes of the file to detect the shabang. - Allows the command to a file path using the Unix path syntax. It does so by using
path.normalize(), after having runwhichon the command. - Some issues with escaping (commands with spaces, etc.) although I am not completely sure whether this is fixed or not in the latest version of Node.
cross-spawnseems to fix this by wrapping the command in an additionalcmd.execall and manually escaping the command and arguments, usingcmd.exe-specific syntax. - It supports
PATHEXTeven when theshelloption isfalse.
I am feeling conflicted.
- On one hand, I am concerned that some of those fixes might be important to some users and any change might break their usage of Execa.
- On the other hand, the current logic seems rather intricate and apparently buggy (according to the current GitHub issues on that repository), and therefore might be a high burden to maintain ourselves.
What are your thoughts?
Relevant: https://github.com/bcoe/awesome-cross-platform-nodejs/issues/26
Maybe we could convince Bun to fix some of the things mention here to force Node.js' hand 🤣
What are your thoughts?
I think it's worth doing it. cross-spawn is already buggy and unmaintained. We would release it as a major version to reduce the potential impact of any regressions.
Yes, that sounds good.
We would also need to add automated tests for the features covered by cross-spawn, although our current tests already cover some of it.
That's some major undertaking, so maybe we should start by just copy/pasting the code as is in an initial PR, and then start refactoring it. For example, some of it can be:
- Trimmed, e.g. we do not want the
ENOENTlogic - Simplified
- Fixed, i.e. by looking into the GitHub issues of
cross-spawn
That's some major undertaking, so maybe we should start by just copy/pasting the code as is in an initial PR, and then start refactoring it.
👍