execa icon indicating copy to clipboard operation
execa copied to clipboard

Get rid of `cross-spawn` dependency

Open sindresorhus opened this issue 2 years ago • 14 comments

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

sindresorhus avatar Oct 27 '23 15:10 sindresorhus

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):

  1. 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 PATH environment variable, as opposed to being a file path. It does so by using which on the command, then reading the first bytes of the file to detect the shabang.
  2. Allows the command to a file path using the Unix path syntax. It does so by using path.normalize(), after having run which on the command.
  3. 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-spawn seems to fix this by wrapping the command in an additional cmd.exe call and manually escaping the command and arguments, using cmd.exe-specific syntax.
  4. It supports PATHEXT even when the shell option is false.

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?

ehmicky avatar Oct 28 '23 02:10 ehmicky

Relevant: https://github.com/bcoe/awesome-cross-platform-nodejs/issues/26

sindresorhus avatar Oct 29 '23 08:10 sindresorhus

Maybe we could convince Bun to fix some of the things mention here to force Node.js' hand 🤣

sindresorhus avatar Oct 29 '23 08:10 sindresorhus

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.

sindresorhus avatar Oct 29 '23 08:10 sindresorhus

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 ENOENT logic
  • Simplified
  • Fixed, i.e. by looking into the GitHub issues of cross-spawn

ehmicky avatar Oct 29 '23 20:10 ehmicky

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.

👍

sindresorhus avatar Oct 29 '23 20:10 sindresorhus