-
-
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
fix(react-scripts): add missing peer dependency react and update react-refresh-webpack-plugin #9872
Conversation
Hi @merceyz! Thank you for your pull request and welcome to our community. We require contributors to sign our Contributor License Agreement, and we don't seem to have you on file. In order for us to review and merge your code, 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. 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! |
Ping @iansu? It causes both our CRA and Storybook tests to fail, which decreases our coverage |
48dd151
to
7c998dc
Compare
@arcanis @merceyz we discussed this issue in the maintainers meeting today. In general we're fine with this change. Adding the The We are open to ideas on how to make this work. |
Awesome, thanks 🙏
Understandable. I can see two alternatives, what do you think?: Peer deps
Require.resolve
Note that we can also do a bit of both: move |
This reverts commit 5c6106b.
@iansu Alright, I've reverted the eslint changes so that can be tackled in a seperate PR to allow the other changes to be merged |
* Fix noFallthroughCasesInSwitch/jsx object is not extensible (facebook#9921) Co-authored-by: Konstantin Simeonov <[email protected]> * Add logo license to README * Remove trailing space in reportWebVitals.ts (facebook#10040) * docs: add React Testing Library as a library requiring jsdom (facebook#10052) Co-authored-by: Ian Schmitz <[email protected]> * Increase Workbox's maximumFileSizeToCacheInBytes (facebook#10048) * Create FUNDING.yml * replace inquirer with prompts (facebook#10083) - remove `react-dev-utils/inquirer` public import * Prepare 4.0.1 release * Prepare 4.0.1 release * Publish - [email protected] - [email protected] - [email protected] - [email protected] - [email protected] * chore: bump web-vital dependency version (facebook#10143) * chore: bump typescript version (facebook#10141) Co-authored-by: Ian Schmitz <[email protected]> * Add TypeScript 4.x as peerDependency to react-scripts(facebook#9964) * remove chalk from formatWebpackMessages (facebook#10198) * Upgrade @svgr/webpack to fix build error (facebook#10213) Co-authored-by: Ian Schmitz <[email protected]> * Improve vendor chunk names in development (facebook#9569) * Update postcss packages (facebook#10003) Co-authored-by: Ian Schmitz <[email protected]> * Recovered some integration tests (facebook#10091) * Upgrade sass-loader (facebook#9988) * Move ESLint cache file into node_modules (facebook#9977) Co-authored-by: Ian Schmitz <[email protected]> * Revert "Update postcss packages" (facebook#10216) This reverts commit 580ed5d. * Remove references to Node 8 (facebook#10214) * fix(react-scripts): add missing peer dependency react and update react-refresh-webpack-plugin (facebook#9872) * Update using-the-public-folder.md (facebook#10314) Some library --> Some libraries * docs: add missing override options for Jest config (facebook#9473) * Fix CI tests (facebook#10217) * appTsConfig immutability handling by immer (facebook#10027) Co-authored-by: mad-jose <[email protected]> * Add support for new BUILD_PATH advanced configuration variable (facebook#8986) * Add opt-out for eslint-webpack-plugin (facebook#10170) * Prepare 4.0.2 release * Publish - [email protected] - [email protected] - [email protected] - [email protected] - [email protected] - [email protected] * tests: update test case to match the description (facebook#10384) * Bump webpack-dev-server 3.11.0 -> 3.11.1 (facebook#10312) Resolves facebook#10084 security vulnerability in websocket-driver library version 0.5.6, imported transitively by sockjs * Upgrade eslint-webpack-plugin to fix opt-out flag (facebook#10590) * update immer to 8.0.1 to address vulnerability (facebook#10412) Resolves facebook#10411 Bumps immer version to 8.0.1 to address the prototype pollution vulnerability with the current 7.0.9 version. * Prepare 4.0.3 release * Update CHANGELOG * Publish - [email protected] - [email protected] - [email protected] Co-authored-by: Ryota Murakami <[email protected]> Co-authored-by: Konstantin Simeonov <[email protected]> Co-authored-by: Ian Sutherland <[email protected]> Co-authored-by: sho90 <[email protected]> Co-authored-by: Anyul Rivas <[email protected]> Co-authored-by: Ian Schmitz <[email protected]> Co-authored-by: Jeffrey Posnick <[email protected]> Co-authored-by: Evan Bacon <[email protected]> Co-authored-by: Sahil Purav <[email protected]> Co-authored-by: Hakjoon Sim <[email protected]> Co-authored-by: Chris Shepherd <[email protected]> Co-authored-by: Jason Williams <[email protected]> Co-authored-by: Jabran Rafique⚡️ <[email protected]> Co-authored-by: John Ruble <[email protected]> Co-authored-by: Morten N.O. Nørgaard Henriksen <[email protected]> Co-authored-by: Sergey Makarov <[email protected]> Co-authored-by: EhsanKhaki <[email protected]> Co-authored-by: Kristoffer K <[email protected]> Co-authored-by: Aviv Hadar <[email protected]> Co-authored-by: Tobias Büschel <[email protected]> Co-authored-by: mad-jose <[email protected]> Co-authored-by: mad-jose <[email protected]> Co-authored-by: Andrew Hyndman <[email protected]> Co-authored-by: Brody McKee <[email protected]> Co-authored-by: James George <[email protected]> Co-authored-by: Dion Woolley <[email protected]> Co-authored-by: Walker Clem <[email protected]>
…t-refresh-webpack-plugin (facebook#9872)
@iansu @arcanis Here is the problem: So far the most sensible workaround was to install eslint-config-react-app and modifying .yarnrc.yml to explicitly tell react-scripts to use it. Like explained here: #10463 (comment) It works but the fact that people have to google for a solution is wrong: not everyone will find it, others will find wrong solutions, etc. I am a bit puzzled what happened with the discussion here about
|
…t-refresh-webpack-plugin (facebook#9872)
…t-refresh-webpack-plugin (facebook#9872)
What's the problem this PR addresses?
react-scripts
has a missing peer dependency onreact
react-scripts
depends on a version of@pmmmwh/react-refresh-webpack-plugin
which doesn't resolve the imports it injects causing projects to rely on hoisting to bringreact-refresh
to the rootnode_modules
folder which is not guaranteed.The templates lack a dependency oneslint-config-react-app
making it also rely on hoisting.Delayed for another time
Read https://yarnpkg.com/advanced/rulebook#packages-should-only-ever-require-what-they-formally-list-in-their-dependencies for more in depth information about hoisting.
Fixes https://github.com/yarnpkg/berry/runs/1299440256#step:5:132
Fixes #9446
Fixes pnpm/pnpm#2957
Closes #9671
How did you fix it?
react
Addeslint-config-react-app
as a dependency in the templatesDelayed for another time
@pmmmwh/react-refresh-webpack-plugin
to get fix: resolve react-refresh/runtime to an absolute path pmmmwh/react-refresh-webpack-plugin#230 (cc @pmmmwh)