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

Default preferLocal to true when using $ #522

Closed
ehmicky opened this issue Mar 1, 2023 · 4 comments · Fixed by #529
Closed

Default preferLocal to true when using $ #522

ehmicky opened this issue Mar 1, 2023 · 4 comments · Fixed by #529

Comments

@ehmicky
Copy link
Collaborator

ehmicky commented Mar 1, 2023

The default value for preferLocal used to be true. We changed it to false in #314 because it conflicted with some uncommon setups (#196, #153). IMHO preferLocal is still a very helpful option for most users, and it's too bad we cannot enable it by default due to the above.

Now, I am wondering whether the default value should be true with the new $ methods we just introduced? The reasons for this is:

  • It is easier for users with the uncommon setups mentioned above to change the default value thanks to the options binding method: const $$ = $({ preferLocal: false })
  • $() is more likely than execa() to be used in a script with many commands to execute. This increases the chance of at least one of them to be a local binary.

What do you think @sindresorhus and @aaronccasanova?

@sindresorhus
Copy link
Owner

I agree. true is a better default for scripting. We need to clearly document the behavior though.

@ehmicky
Copy link
Collaborator Author

ehmicky commented Mar 2, 2023

Great, let's do this then!
@aaronccasanova Woud you like to take this up, as this relates to the $ API? Otherwise, I'm also happy to do it.

@aaronccasanova
Copy link
Contributor

I like it 👍 Feel free to pick this one up @ehmicky!

If I'm not mistaken it should be a quick update to export const $ = create$({preferLocal: true}); here:

execa/index.js

Line 245 in eae327c

export const $ = create$();

@ehmicky
Copy link
Collaborator Author

ehmicky commented Mar 5, 2023

Done at #529. 👍

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 a pull request may close this issue.

3 participants