Skip to content
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

refactor!: replace execa with tinyexec #151

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

ziebam
Copy link

@ziebam ziebam commented Sep 18, 2024

Resolves #150.

This PR is a suggestion to replace heavier execa with a lighter tinyexec to reduce the dependency count and the bundle size.

@ziebam
Copy link
Author

ziebam commented Sep 19, 2024

Actually, since there are only two usages, we could just use node:child_process directly. What do you think?

@pi0 pi0 changed the title cleanup: replace execa with tinyexec refactor!: replace execa with tinyexec Sep 25, 2024
@pi0
Copy link
Member

pi0 commented Sep 25, 2024

Thanks for PR. I think so we might directly use node:child_process.

Nypm was doing this before. We moved to execa to support cross-platform search path of corepack mainly (#54). However with Node.js not going to ship corepack by default.. i think we might change strategy altogether.

Since this change might introduce regression, i am thinking more about options we have.

@ziebam
Copy link
Author

ziebam commented Sep 25, 2024

Thanks for PR. I think so we might directly use node:child_process.

Nypm was doing this before. We moved to execa to support cross-platform search path of corepack mainly (#54). However with Node.js not going to ship corepack by default.. i think we might change strategy altogether.

Since this change might introduce regression, i am thinking more about options we have.

No rush! No matter which solution you decide on, I'm happy to help driving this replacement forward.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Replace execa with tinyexec
2 participants