-
Notifications
You must be signed in to change notification settings - Fork 949
chore: replace execa with tinyexec and Node's child_process.spawnSync
#4134
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
chore: replace execa with tinyexec and Node's child_process.spawnSync
#4134
Conversation
|
This pull request is automatically built and testable in CodeSandbox. To see build info of the built libraries, click here or the icon next to each commit SHA. |
|
Thanks @ziebam ! I had a look at the issues/PRs related to vitest. Looks like one test is failing: ❯ @commitlint/cli/src/cli.test.ts (57 tests | 1 failed) 20862ms
× should throw when called without [input]
→ Test timed out in 5000ms.
If this is a long-running test, pass a timeout value as the last argument or configure it globally with "testTimeout".
⎯⎯⎯⎯⎯⎯⎯ Failed Tests 1 ⎯⎯⎯⎯⎯⎯⎯
FAIL @commitlint/cli/src/cli.test.ts > should throw when called without [input]
Error: Test timed out in 5000ms.
If this is a long-running test, pass a timeout value as the last argument or configure it globally with "testTimeout".
⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯[1/1]⎯
Test Files 1 failed | 83 passed (84)
Tests 1 failed | 1055 passed (1056)
Start at 08:47:41
Duration 22.33s (transform 3.14s, setup 0ms, collect 7.98s, tests 29.32s, environment 21ms, prepare 7.67s)Did all tests pass for you locally? |
|
@escapedcat, thanks for taking a look! Should be fixed now. It seems that the Sorry for that! I initially assumed the test failed because my machine isn't powerful enough (since it timed out), but it turns out I was wrong. 😄 |
|
Thanks for fixing this! |
I think there are still downstream dependencies that use it? Namely: |
|
Got it, yes, makes sense. Thanks! |
This PR is a suggestion to migrate from
execato a lighter alternative:tinyexec.Description
tinyexecwhich provides a wrapper aroundchild_process'sspawn.execaSynchas been rewritten to use Node'schild_process.spawnSyncdirectly astinyexecdoesn't provide a wrapper around that. I haven't dug into it very deeply, but I've noticed a ~50% speedup in runningtrailer-exists.test.ts, so that's another gain related to this change.Motivation and Context
The migration improves the bundle size and installation time for the project, therefore improving the UX. Most of the advanced
execafeatures are unused, and those that are, are simple to rewrite into a blend oftinyexecand native functionality.Usage examples
N/A
How Has This Been Tested?
yarn testTypes of changes
Not sure what to pick here, please advise. 😄
Checklist: