-
-
Notifications
You must be signed in to change notification settings - Fork 217
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Get rid of cross-spawn
dependency
#578
Comments
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
I am feeling conflicted.
What are your thoughts? |
Relevant: bcoe/awesome-cross-platform-nodejs#26 |
Maybe we could convince Bun to fix some of the things mention here to force Node.js' hand 🤣 |
I think it's worth doing it. |
Yes, that sounds good. We would also need to add automated tests for the features covered by 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:
|
👍 |
I have spent some time figuring out what ShebangsShebangs are a Unix concept. Windows does not support them.
Arguments splittingIn Unix, a subprocess file and arguments are specified as a file string and an arguments array of strings to the underlying C syscall. In Windows, those are passed as a single string, even when no shell is involved. The caller must escape the file and arguments with Windows-specific quoting rules so they are correctly split. Node.js fixes this with the
It does not escape all shell-specific characters, so it is only meant for arguments splitting, not full-on shell escaping. In particular, it does not prevent shell injection.
Node modules binariesOn Windows, OS binaries are often This means that, to run Node modules binaries on Windows, one must use the This has a few problems:
|
Great summary 👍 |
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
The text was updated successfully, but these errors were encountered: