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

Try integrating into Create React App #7

Closed
gaearon opened this issue Dec 6, 2019 · 33 comments
Closed

Try integrating into Create React App #7

gaearon opened this issue Dec 6, 2019 · 33 comments

Comments

@gaearon
Copy link

gaearon commented Dec 6, 2019

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

@iansu
Copy link

iansu commented Dec 6, 2019

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.

@Ericnr
Copy link

Ericnr commented Dec 6, 2019

Would be great to see this being implemented without ejecting from CRA too, using Craco or React-App-Rewired.

@gaearon
Copy link
Author

gaearon commented Dec 7, 2019

Yes, that’s what I’m referring to by “creating a pull request for CRA” 🙂 that was the goal all along.

@pmmmwh
Copy link
Owner

pmmmwh commented Dec 7, 2019

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

To my knowledge, most of the work have to be done here:

  1. Allow custom error overlay and alter react error overlay/the API needed here
  2. Fix assumptions when sniffing Babel/Webpack, since CRA does not use webpack's mode argument
  3. Need to try come up with an integration with the Webpack socket situation (related Does the error overlay websocket need a ping heartbeat? #9 ), and also make this work with servers under proxy/custom paths)

@gaearon
Copy link
Author

gaearon commented Dec 19, 2019

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.

@pmmmwh
Copy link
Owner

pmmmwh commented Dec 19, 2019

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 node_modules so the injected module does not get processed, thus yielding high percentage of false positives.

@esetnik
Copy link

esetnik commented Jan 8, 2020

Based on the suggestion by @drather19 I have published a customize-cra plugin to make it easier. See esetnik/customize-cra-react-refresh.

@bugzpodder
Copy link

bugzpodder commented Jan 14, 2020

I tested this with mostly a CRA 3.3.0 build and got a few issues:

  1. Got the same error in Use safeThis function in cleanup phase instead of "window" to better support web workers #29 and that fix worked for me.
  2. The first change always refreshes the page for some reason. subsequent changes doesn't.
  3. After a bit I get "The development server has disconnected.
    Refresh the page if necessary. " then if I make a change nothing happens (page doesn't refresh/update). This seem to happen without this plugin so likely unrelated. react-scripts 3.3.0 - "The development server has disconnected. Refresh the page if necessary" facebook/create-react-app#8203
  4. disableRefreshCheck had to be enabled otherwise I get a bunch of warnings BabelDetectComponent logic doesn't work for consumers #15
  5. the new error overlay seem to fight over the CRA error overlay. After I make a syntax error I can't get the new error overlay to go away. Suggestion: Don't display error iframe #28

@jimmyn
Copy link

jimmyn commented Jan 14, 2020

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

@charrondev
Copy link

charrondev commented Feb 29, 2020

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

  • Ejecting.
  • Applying the babel and webpack plugin w/ disableRefreshCheck.

I do get the duplicate overlay thing, but haven't experienced the other issues described by @bugzpodder.

@charrondev
Copy link

charrondev commented Feb 29, 2020

After playing with it a little bit, I did a quick fork of the react-dev-utils/webpackHotDevClient. This removes all the error & warning handling.

https://gist.github.com/charrondev/c06cd5a9616a1d1a0db45de6f10b592b#file-webpackhotdevclient-js

If this is used instead of the react-dev-utils one I find

  • There are no duplicate errors.
  • Errors properly dismiss after being fixed.

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.

@pmmmwh
Copy link
Owner

pmmmwh commented Apr 7, 2020

facebook/create-react-app#8582 has been merged, but some API naming work still has to be done so I'll leave this open.

@thoamsy
Copy link

thoamsy commented Apr 21, 2020

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

I also use craco, but I am get the another error。The console says

Uncaught ReferenceError: $RefreshReg$ is not defined
at Object../node_modules/events/events.js (events.js:447)
at webpack_require (bootstrap:806)
at fn (bootstrap:167)
at Object../node_modules/webpack/hot/emitter.js (emitter.js:1)
at webpack_require (bootstrap:806)

@pmmmwh
Copy link
Owner

pmmmwh commented Apr 23, 2020

I also use craco, but I am get the another error

Did you include the Babel plugin in your config? I won't be able to help unless there is a reproducible example.

@thoamsy
Copy link

thoamsy commented Apr 24, 2020

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.

@thoamsy
Copy link

thoamsy commented Apr 24, 2020

It ’s weird, my project will report an error, but when I wrote a minimal demo using craco, it worked very well.

@bugzpodder
Copy link

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.

@thoamsy
Copy link

thoamsy commented Apr 26, 2020

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.

@bugzpodder
Copy link

@thoamsy craco didn't work for me (got $RefreshReg$ error), however I tried https://dutzi.party/react-fast-refresh/ and it worked better

@pmmmwh
Copy link
Owner

pmmmwh commented Apr 26, 2020

@thoamsy @bugzpodder
I'm not sure what broke for you guys, but I am able to get the plugin working with craco. Here is a CodeSandbox demonstrating this (Notice the use of whenDev, that is important because the plugin do not inject required runtime code in non-dev modes, and will cause the Babel plugin to break builds because of missing globals).

I won't be able to help without more insights into the specific setups for your apps, or with more debugging information.

@thoamsy
Copy link

thoamsy commented Apr 26, 2020

@bugzpodder Do you mean that use react-app-rewired instead of craco can works well in your project?

@thoamsy
Copy link

thoamsy commented Apr 26, 2020

@thoamsy @bugzpodder
I'm not sure what broke for you guys, but I am able to get the plugin working with craco. Here is a CodeSandbox demonstrating this (Notice the use of whenDev, that is important because the plugin do not inject required runtime code in non-dev modes, and will cause the Babel plugin to break builds because of missing globals).

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 craco to let my project works 🤔

@bugzpodder
Copy link

@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 yarn start so whenDev didn't really matter.

Screen Shot 2020-04-26 at 9 01 30 AM

@pmmmwh
Copy link
Owner

pmmmwh commented Apr 27, 2020

@thoamsy @bugzpodder
So I did some debugging, it seems that the issue here is related to dilanx/craco#36 and dilanx/craco#49 .

The problem is that this plugin by default don't process files within node_modules, but craco will apply react-refresh/babel unconditionally to both instances of babel-loader, thus breaking all JS files within node_modules.

To solve this, other than requesting a proper fix on craco's side, you can manually inject the Babel plugin as follows:

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 craco here.

@thoamsy
Copy link

thoamsy commented Apr 28, 2020

@pmmmwh It works! Thank you for your effort to solve this problem, it's amazing!

@kelly-tock
Copy link

Is this released already with CRA? I don’t see the env variable mentioned in the PR in the documentation

@deftomat
Copy link

deftomat commented May 13, 2020

@kelly-tock Yes, it is. Use FAST_REFRESH env variable.

https://github.com/facebook/create-react-app/blob/c5b96c2853671baa3f1f297ec3b36d7358898304/packages/react-scripts/config/env.js#L100

@gaearon
Copy link
Author

gaearon commented May 13, 2020

Note there are some pending fixes that haven't made into CRA version yet.

@pmmmwh
Copy link
Owner

pmmmwh commented May 13, 2020

Is this released already with CRA?

It's on the master branch, but they haven't released a new version with the experimental support.

@kelly-tock
Copy link

@pmmmwh that's exactly what I was asking. thanks!

@xyy94813
Copy link

I am using react-hot-loader with CRA.
But i got some problems when i use code splitting.
So, i am looking for a new solution.

If i want to use it with CRA now.
Is it the best option to use esetnik/customize-cra-react-refresh and then remove it after the CRA published?

@pmmmwh
Copy link
Owner

pmmmwh commented Jul 23, 2020

Is it the best option to use esetnik/customize-cra-react-refresh and then remove it after the CRA published?

It depends on your setup, e.g. if you're using any rewire tools. I would suggest you take a look at the examples/create-react-app folder, then if you really don't want to bother you can move on with the rewire plugin.

@Saibamen
Copy link

Saibamen commented Oct 24, 2020

Fixed - released in CRA 4.0.0: https://github.com/facebook/create-react-app/releases

@pmmmwh pmmmwh closed this as completed Oct 25, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests