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

fix: add react-refresh as a dependency for templates #9671

Closed
wants to merge 2 commits into from
Closed

fix: add react-refresh as a dependency for templates #9671

wants to merge 2 commits into from

Conversation

pmmmwh
Copy link
Contributor

@pmmmwh pmmmwh commented Sep 20, 2020

This fixes #9446 - basically I've added react-refresh to the starting templates so that the peerDependency from react-refresh-webpack-plugin would properly resolve in stricter module systems (e.g. Yarn 2 and PnP). It is already a dependency of react-scripts but that does not suffice because imports to react-refresh/runtime will be injected to the users' bundle.

@mrmckeb
Copy link
Contributor

mrmckeb commented Sep 22, 2020

Thanks for this @pmmmwh!

We also have community templates, it might be better to add it here?
https://github.com/facebook/create-react-app/blob/master/packages/create-react-app/createReactApp.js#L434

Maybe @iansu and/or @ianschmitz have some thoughts too.

@pmmmwh
Copy link
Contributor Author

pmmmwh commented Oct 18, 2020

I've update the PR to install react-refresh within the CRA command instead - I've made the changes where I think is necessary, but please feel free to correct me cause I'm not very familiar with the CLI's structure!

@merceyz
Copy link
Contributor

merceyz commented Oct 24, 2020

This should be fixed in the plugin instead by having it resolve it before injecting the import - pmmmwh/react-refresh-webpack-plugin#230

@pmmmwh
Copy link
Contributor Author

pmmmwh commented Nov 7, 2020

This should be fixed in the plugin instead by having it resolve it before injecting the import - pmmmwh/react-refresh-webpack-plugin#230

Do you think that this is still needed after the fix in the plugin?

@merceyz
Copy link
Contributor

merceyz commented Nov 8, 2020

Do you think that this is still needed after the fix in the plugin?

Nope, it's not needed anymore. The plugin is updated in #9872 together with other dependency fixes

@pmmmwh
Copy link
Contributor Author

pmmmwh commented Nov 11, 2020

Superseded by #9872

@pmmmwh pmmmwh closed this Nov 11, 2020
@pmmmwh pmmmwh deleted the fix/react-refresh-peer-dep branch November 11, 2020 20:10
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
6 participants