-
Notifications
You must be signed in to change notification settings - Fork 194
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
Try integrating into Create React App #7
Comments
I would love to get this into Create React App. If there's anything needed on our end (other than installing and using the plugin) please let me know. |
Would be great to see this being implemented without ejecting from CRA too, using Craco or React-App-Rewired. |
Yes, that’s what I’m referring to by “creating a pull request for CRA” 🙂 that was the goal all along. |
To my knowledge, most of the work have to be done here:
|
Could Babel detection be failing because the injected module doesn’t have a path that matches files going though Babel? Like usually people limit it to their source directory. |
Yes exactly. People often ignore |
Based on the suggestion by @drather19 I have published a customize-cra plugin to make it easier. See esetnik/customize-cra-react-refresh. |
I tested this with mostly a CRA 3.3.0 build and got a few issues:
|
I'm using craco with configurations from this codesandbox example and it seems to be working fine, but I'm also fighting with duplicated error overlays - #28 |
Edit: Previously I thought I had everything working. I was wrong, swapping back to the stock client actually prevented the refreshing form working at all. I got this working in my CRA by
I do get the duplicate overlay thing, but haven't experienced the other issues described by @bugzpodder. |
After playing with it a little bit, I did a quick fork of the https://gist.github.com/charrondev/c06cd5a9616a1d1a0db45de6f10b592b#file-webpackhotdevclient-js If this is used instead of the
Update I created a PR adding this to cra-kitchen-sink example. #41 PR over on CRA as well. facebook/create-react-app#8582 If that merges sooner than #41 then I can replace #41 with a simple README update to state it's built in to some future version of CRA. |
facebook/create-react-app#8582 has been merged, but some API naming work still has to be done so I'll leave this open. |
I also use craco, but I am get the another error。The console says
|
Did you include the Babel plugin in your config? I won't be able to help unless there is a reproducible example. |
Thank you, I will try to provide an example for you. |
It ’s weird, my project will report an error, but when I wrote a minimal demo using craco, it worked very well. |
do you have an example where i can test as well? i recently had tried to apply to a project but couldn't get it to work. |
This is my demo for test the hot reload, it works very well. But I can't bring it to my project. |
@thoamsy craco didn't work for me (got |
@thoamsy @bugzpodder I won't be able to help without more insights into the specific setups for your apps, or with more debugging information. |
@bugzpodder Do you mean that use |
Thank for your advice, but I think our problem is that the dev mode can't run 😂。Maybe I should try to use another tool instead of |
@thoamsy yes, but react-app-rewired just modifies your webpack config like craco so you can get it to work with some modification I believe. @pmmmwh that's because you are relying on codesandbox. I am doing it locally on a mac. Just goto your codesandbox, download it as ZIP, install it locally, add your plugin (0.3.0-beta.5) and yarn start it. I always only used |
@thoamsy @bugzpodder The problem is that this plugin by default don't process files within To solve this, other than requesting a proper fix on const { whenDev } = require('@craco/craco');
const ReactRefreshWebpackPlugin = require('@pmmmwh/react-refresh-webpack-plugin');
module.exports = {
webpack: {
configure: (webpackConfig) => {
whenDev(() => {
const babelLoaderRule = webpackConfig.module.rules
.find((x) => Array.isArray(x.oneOf))
.oneOf.find(
(x) =>
x.loader &&
x.loader.includes('babel-loader') &&
x.options &&
x.options.presets &&
x.options.presets.some((p) => p.includes('babel-preset-react-app'))
);
babelLoaderRule.options.plugins.push(require.resolve('react-refresh/babel'));
});
return webpackConfig;
},
plugins: [
...whenDev(
() => [
new ReactRefreshWebpackPlugin({
// Proper setup for this would require the new unpublished version of CRA.
// We'll disable the integration for now.
overlay: false,
}),
],
[]
),
],
},
}; I have updated the example for |
@pmmmwh It works! Thank you for your effort to solve this problem, it's amazing! |
Is this released already with CRA? I don’t see the env variable mentioned in the PR in the documentation |
@kelly-tock Yes, it is. Use |
Note there are some pending fixes that haven't made into CRA version yet. |
It's on the |
@pmmmwh that's exactly what I was asking. thanks! |
I am using If i want to use it with CRA now. |
It depends on your setup, e.g. if you're using any rewire tools. I would suggest you take a look at the |
Fixed - released in CRA 4.0.0: https://github.com/facebook/create-react-app/releases |
I've tried integrating this into an ejected CRA project but got stuck (#6).
This gets me thinking: perhaps, the next milestone is to get it to a point where we can create a pull request for CRA? What's missing for that to work? cc @iansu
The text was updated successfully, but these errors were encountered: