-
-
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 npm v7 with workspaces for local development and testing #11304
Conversation
Hi @lukekarrys! Thank you for your pull request and welcome to our community. Action RequiredIn order to merge any pull request (code, docs, etc.), we require contributors to sign our Contributor License Agreement, and we don't seem to have one on file for you. ProcessIn order for us to review and merge your suggested changes, please sign at https://code.facebook.com/cla. If you are contributing on behalf of someone else (eg your employer), the individual CLA may not be sufficient and your employer may need to sign the corporate CLA. Once the CLA is signed, our tooling will perform checks and validations. Afterwards, the pull request will be tagged with If you have received this in error or have any questions, please contact us at [email protected]. Thanks! |
Thank you for signing our Contributor License Agreement. We can now accept your code for this (and any) Facebook open source project. Thanks! |
081f7ad
to
810790e
Compare
ea23782
to
64ff23d
Compare
Could you elaborate on why this change is necessary? What is gained from this migration compared to what's already in place? |
0427380
to
1f40f05
Compare
1f40f05
to
a666acc
Compare
That doesn't answer my questions but alright |
Hey all! I believe there may be an issue with how For example in this e2e build in a recent babel PR we can see the following error (Copied below for ease of reading)
cc @lukekarrys edit: apologies just found this fix here babel/babel#13724. Please ignore |
@dan-kez Are you using |
Also if you're using node 14 in a CI environment, you'll need to manually install npm 7 like I did here: https://github.com/facebook/create-react-app/pull/11304/files#diff-5c3fa597431eda03ac3339ae6bf7f05e1a50d6fc7333679ec38e21b337cb6721R14-R15 |
It looks like this was the issue. nicolo-ribaudo fixed this in babel/babel#13724 Thanks for the prompt response and apologies for posting before digging into it further! |
This PR uses npm v7 (with its support of workspaces) for local development and testing. This should only affect local development, and not the generated output from
create-react-app
. So if I made changes that do affect the output, it is likely a mistake. 😄Lerna
lerna
is still being used for changelog generation and publishing. These two tasks could be replaced with individual tools/scripts, now thatnpm
is responsible for installing and linking dependencies.