-
-
Notifications
You must be signed in to change notification settings - Fork 26.9k
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
Use env var to detect yarn or npm as the package manager #11322
Conversation
I'm curious why? |
It would be better to use whichever package manager called it by checking |
Are you suggesting CRA should use yarn if the user runs |
I agree that's a much better solution, and I reworked this PR to implement something similar to that. I opted against using anything from |
@bryik Yes |
This can also be anything as it's user configurable with
I agree that @merceyz Do you have some examples of common use cases where |
If the user changes the
Since it's set to the main module filename pretty much always. $ docker run -it --rm node:16.7.0 yarn exec env | grep npm_execpath
npm_execpath=/opt/yarn-v1.22.5/bin/yarn.js
$ docker run -it --rm node:16.7.0 bash -c 'yarn set version latest && yarn exec env' | grep npm
_execpath
npm_execpath=/.yarn/releases/yarn-1.22.11.cjs
$ docker run -it --rm node:16.7.0 bash -c 'yarn set version nightly && yarn exec env' | grep np
m_execpath
npm_execpath=/.yarn/releases/yarn-nightly.cjs |
This also updates the package manager test to check for yarn based on user_config env var
@arcanis We've actually made some changes to this since the initial PR. Our goal is to make running The new behaviour in this PR looks at how |
Yep, I agree that some kind of detection based on how it gets invoked makes sense (related: nodejs/corepack#25). As a quick note en passant (since this PR is implemented on top of the other), #11304 happens to accidentally remove the PnP test: |
@arcanis This test has been failing on our main branch for a while too. We're just trying to get our tests into a workable state right now so we've disabled a number of failing tests. We're planning to go back and fix them as soon as possible. |
Looks like we've got one failing test with Yarn and Node 14. We're going to move ahead with this PR since we're still in the alpha stage and address this test failure in a follow up PR. cc @lukekarrys |
This removes the--use-npm
flag and adds the--use-yarn
flag. It also removes the previous behavior of checking for the existence of yarn as the signal to use it as the package manager. The new behavior will be to usenpm
by default and to use yarn with the--use-yarn
flag.The goal of this PR is to remove the check on whether
yarn
is present in the user's path as the signal to use yarn.This PR is implemented on top of #11304, since it changed some of the same files and was easier to get tests passing on top of that. I will rework this based on the outcome of that PR. For now, here's the differing commits in this PR: lukekarrys/create-react-app@lk/dev-npm-7...lk/use-npm