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

Component modifications are not applied #334

Closed
yekver opened this issue Mar 23, 2021 · 26 comments
Closed

Component modifications are not applied #334

yekver opened this issue Mar 23, 2021 · 26 comments

Comments

@yekver
Copy link

yekver commented Mar 23, 2021

It is quite strange but I got stuck in the following situation:

  • any scss change is successfully applied in-place
  • any json changes is successfully applied in-place
  • all typescript component changes are ignored.

In the network tab I can see that all changes were successfully received and console notifies me that changes are applied and App is up to date!, unfortunately there is no visual or logic changes and UI keeps the stale state. Only full reload can fix the situation.

I reviewed project configuration multiple times without any luck. I've modified entry to match the following format: entry: { main: ['./src/index.tsx'] } and added target: 'web'. I went across the provided Troubleshooting guide without any luck.
I'm confused because of no warnings/errors.

Used versions:

react-refresh-webpack-plugin: 0.5.0-beta.1 (last stable version also affected)
babel-loader: 8.2.2
@babel-core: 7.13.10
react-refresh: 0.9.0
react: 16.13.1
webpack: 5.27.2
webpack-dev-server: 3.11.2 (last beta makes no difference)

Seems like this bug is closely related to this one

How can I debug this and provide you with some more helpful information?

@pmmmwh
Copy link
Owner

pmmmwh commented Mar 23, 2021

Is it possible to create a reproducible example?

Can you share some of your configurations (Babel, Webpack)?

@yekver
Copy link
Author

yekver commented Mar 23, 2021

@pmmmwh thank you for the quick reply. Unfortunately there is no possibility to provide a valuable reproducible example. Project is proprietary and huge. We didn't use babel though configuration was copied from your example.

Maybe you can guide me with possible cases? Because right now I got no ideas how to debug this.

@yekver
Copy link
Author

yekver commented Mar 26, 2021

Here some app architecture level details. It is based on micro-frontends solution. Host app provides exposed to global scope instances of react / react-dom and react-refresh. All of these packages are in the same vendors bundle.

Plugin app bundles as a umd library and use externals property to get all the react stuff. react-refresh-webpack-plugin and react-refresh/babel is defined only in Plugin app webpack config.
Plugin library option also used new ReactRefreshPlugin({ library: 'YourLibrary' }) Host and Plugin apps are served on different ports.

Do I have to run react-refresh/runtime manually in the Host app to make sure that it was executed before any react code? Should I also define react-refresh-webpack-plugin and react-refresh/babel in Host app webpack config?

@smtrd3
Copy link

smtrd3 commented Apr 13, 2021

@pmmmwh I am facing similar issue, details in #352

@pmmmwh
Copy link
Owner

pmmmwh commented May 4, 2021

Is it possible to give the latest beta a try and report back? I know that the plugin does not work quite as reliable when micro-frontends are used due to the different runtime chunks involved in Webpack ...

Can you expose a bit more of the Webpack configuration so I can take a look? Maybe try runtimeChunk: 'single'?

@yekver
Copy link
Author

yekver commented May 13, 2021

@pmmmwh still no luck with the latest beta :(

Here is how Plugin app webpack config looks like:

const libName = 'my_lib'

return {
  entry: './src/main.tsx',
  output: {
    library: libName,
    type: 'umd'
  },
  filename: `${libName}.js`,
  publicPath: 'https://localhost:3000'
  clean: true,
  externals: {
    'react': 'React',
    'react-dom': 'ReactDom',
    'react-refresh': 'ReactRefresh'
  }
  module: {
   rules: [
    {
       test: /.\tsx?$/,
       use: [{
         loader: 'babel-loader',
         options: { plugins: ['react-refresh/babel'] }
       }, {
         loader: 'ts-loader',
         options: { transpileOnly: true }
       }]
    }
   ]
  },
  plugins: [
    () => new ReactRefreshPlugin({ library: libName })
  ],
  devServer: {
     host: 'localhost',
     port: 3000,
     client: {
       host: 'localhost',
       port: 3000,
     },
     liveReload: false,
     hot: 'only'
  }
}

@pmmmwh
Copy link
Owner

pmmmwh commented May 13, 2021

Are you using module federation or how are you managing micro front-ends?

@yekver
Copy link
Author

yekver commented May 13, 2021

Micro frontends are managed this way: umd bundle is imported by host application. Plugin app provides a piece of JSX that will be injected and rendered by host application.

@pmmmwh
Copy link
Owner

pmmmwh commented May 13, 2021

Would you mind creating a example on how the app would be structured and how it would work? I'm quite unfamiliar with micro-frontends so I can't really provide any reasonable help.

@s-KaiNet
Copy link

I have the same problem - hot-refresh works when I change *.scss files, but not when I change *.tsx components.
I also have quite a complicated setup where webpack builds a library, then the library is used on a page by a third-party vendor.

I tried to find out the root cause, I copied an example from this repo (typescript-without-babel), updated deps to v0.5.0-beta.8, [email protected], [email protected], applied the webpack config I use in my solution and it worked without any issue.

Then I returned back to my not working solution and accidentally found out that hot-refresh works for my solution in one Chrome profile. Literally, I open two profiles - change the .tsx component - it refreshes in one profile and not in the other. The difference between profiles is that the profile, which works, has react-dev-tools Chrome add-in installed. As soon as I installed this add-in to the second profile, it also starts working.

I'm not sure how it helps here, but maybe it will give some clues on what might be the cause? It seems react-dev-tools enables something, which somehow unblocks hot-refresh. M - magic :)

Just out of curiosity - @yekver do you have react-dev-tools add-in installed? Could you check whether it works with this add-in enabled for you?

@s-KaiNet
Copy link

Maybe the reason is, that in my solution react is externalized (it's an external dependency and not bundled in the resulting lib). So it works for typescript-without-babel because react is an internal dependency, and it works for my solution with chrome add-in because react-dev-tools patches react instance found on the page.

@s-KaiNet
Copy link

s-KaiNet commented May 16, 2021

So I found simple reproduction:

  1. Grab typescript-without-babel

  2. Update deps to "@pmmmwh/react-refresh-webpack-plugin": "0.5.0-beta.8", "react-refresh-typescript": "2.0.0"

  3. Update webpack.config.js - add

    externals: {
        'react': 'React', 
        'react-dom': 'ReactDOM' 
      },
  4. Update index.html - add

<script crossorigin src="https://unpkg.com/react@17/umd/react.development.js"></script>
<script crossorigin src="https://unpkg.com/react-dom@17/umd/react-dom.development.js"></script>
  1. npm run start - hot-refresh doesn't work for all components.

If you have react-dev-tools add-in enabled, then disable it, otherwise, it patches react and it starts working. Also, it doesn't work event with react-dev-tools if external production react build is used.

A good question whether it's a bug here or in react-refresh-typescript repo.

@pmmmwh
Copy link
Owner

pmmmwh commented May 16, 2021

If you externalize React and React DOM, you need to also externalise React Refresh.
React Refresh needs to run before ANY React code runs, and externalising React breaks this guarantee within the bundle.

It is expected behaviour that this does not work with the production build of React, it simply does not contain the development tools needed.

@s-KaiNet
Copy link

If you externalize React and React DOM, you need to also externalise React Refresh.
React Refresh needs to run before ANY React code runs, and externalising React breaks this guarantee within the bundle.

Yea, but react-dev-tools is managed to somehow patch external react instance so that hot-refresh works when react-dev-tools is enabled on a page. Is it possible to do the same automatically in react-refresh-webpack-plugin or do you know what exactly react-dev-tool does to make it work? Maybe I can just apply the same in my dev code base.

@pmmmwh
Copy link
Owner

pmmmwh commented May 16, 2021

DevTools (and React Refresh) initialises global development hooks where the development build of React would attach renderer instances to.

This is what needed to happen before React renders.
I do not recommend applying this logic yourself.

This plugin ensures this is the case by using this entry point, which is injected in front of all your entry points. However, in the case of externalisation, this approach can be easily defeated.

In theory you can patch your app in a way where the entrypoint above would always run first - there are checks to ensure single injection so it should be quite safe to have it in multiple places.
I would strongly advise going the route of properly externalising things and ensuring execution order.

Edit:
Actually, now that I think about it -
If React is externalised and provided as a singleton in the host app, wiring up plugin apps for React Refresh (specifically the setup part) seems to be useless because:

  1. It doesn't really have it's own React instance, so in technical sense it actually re-uses the host app's React instance, which means it is actually the host app that needs to be wired up.
  2. It is potentially and often imported AFTER the host app executes, which in turn would have already executed React renderer code, which means it would have been too late for any setup to run.

You will still need the plugin code which handles the runtime globals as well as the injected HMR runtime code though. We do not currently have the option for you to exclude entry injection, but since entries have checks for single entry you could be quite fine by doing overlay: false and ignore the library option (which means the whole app would reuse the same singleton of React Refresh logic).

Mind trying this approach?

  • Wire up the "library" apps with React Refresh, skipping the library option.
  • Wire up the "host" app with React Refresh WITHOUT externalizing react-refresh, only react and react-dom. In this instance of RR you can configure it to your liking.

@pmmmwh
Copy link
Owner

pmmmwh commented May 16, 2021

I actually tested externalisation and I couldn't reproduce.
Can you tweak this sandbox to show the issue?

https://codesandbox.io/s/react-refresh-externals-14fpn

@s-KaiNet
Copy link

Actually your sandbox doesn't work for me:

2021-05-17_1-32-41.mp4

Just make sure you don't have react-dev-tools plugin running in browser (or maybe other react-related things?).

@pmmmwh
Copy link
Owner

pmmmwh commented May 16, 2021

Aha let me just create a new profile to work with this ...

@pmmmwh
Copy link
Owner

pmmmwh commented May 17, 2021

I have fixed the sandbox (and all the HTTPS errors etc.) and made it work with externalisation.
https://codesandbox.io/s/react-refresh-externals-14fpn

It should be generic enough - whenever you're externalising React this should be the solution you can use in the places where React is pulled from scripts (so in micro-frontends, this would be the host app; in mono-repos, this would be the app entry).
I would admit that it is a bit clunky, but in this case I'm really unsure if there's anything more we can do. Using React Dev Tools is a quick and dirty solution to ensure the React DOM renderer always inject if you don't want to deal with the mess.

We could allow skipping entry injection, but given that in most cases it is unsafe to do so (it is up to the user to properly organize and order execution which is quite easy to get wrong) I'm not sure if I want to allow this additional API surface (and the issues that would come in with this "mode") ...

@s-KaiNet
Copy link

I don't have control over the react injection (as said it's a vendor's platform where I create kind of extensions using react). However, I found that if I explicitly include react in my dev builds it works without issues on the platform and I can leverage hot-refresh. I will use this trick for the time being.

@pmmmwh thank you for your help here, it's extremely useful for those, who externalize react and try to use hot-refresh plugin at the same time.

@pmmmwh
Copy link
Owner

pmmmwh commented May 17, 2021

Yep, I think it is still a bit of an undefined behaviour to use React Refresh in these circumstances (it actually cannot be properly externalised by default as there is no UMD build for the runtime).

Basically the only guarantee we depend on is to have react-refresh/runtime setup BEFORE the renderer, which in most cases is react-dom.

I'll try to update the troubleshooting documentation soon.

@enuchi
Copy link

enuchi commented Jun 2, 2021

Very glad I found this -- thanks for the info @pmmmwh. Had a similar issue implementing for this project here: https://github.com/enuchi/React-Google-Apps-Script/

I'm externalizing React, and also loading the app in an iframe. The console showed refresh was working, but the changes weren't being reflected. I loaded the app in a regular browser window instead of in an iframe and then it worked fine! -- but based on above realized it was due to React Dev Tools being loaded (I'm assuming Dev Tools isn't getting loaded inside the iframe window). I just tried in a separate profile without React Dev Tools and it also didn't work.

So my solution was to just not externalize React when in development and it works great. Is that the recommended approach (if it's something we can control)? Couldn't figure out what the solution actually was in the codesandbox example you gave.

@pmmmwh
Copy link
Owner

pmmmwh commented Jun 7, 2021

@enuchi
The solution is to ensure our ReactRefreshEntry runs BEFORE the externalized UMD React builds are loaded. In the sandbox I achieved this by using it as a separate entrypoint and manipulating the generated HTML so the script will be parsed first, as well as playing with chunking to ensure there is only 1 runtime chunk + 1 instance of React Refresh.

If you don't have control over execution order then yes, skipping externalization in development is your best bet.

I'm assuming Dev Tools isn't getting loaded inside the iframe window

Yes, DevTools intentionally does not copy things over through a frame boundary.

facebook/react#18945

@enuchi
Copy link

enuchi commented Jul 15, 2021

Thanks again for your help on this @pmmmwh!

@erikt9
Copy link

erikt9 commented Jul 29, 2021

In my case I have an external react that is built as a separate webpack bundle and shared throughout the site (for use by other webpack and non-webpack apps). What helped me was to externalize the react-refresh / runtime as well (see #212 (comment)).

So in summary, in my bundle that builds react and exposes it on window, I make sure to import ReactRefreshEntry and react-refresh/runtime before React. This bundle now exposes the react-refresh/runtime on window and is referenced as an external in my main webpack bundle.

@pmmmwh
Copy link
Owner

pmmmwh commented Sep 4, 2021

A guide on externalising React is added now.

@pmmmwh pmmmwh closed this as completed Sep 4, 2021
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

6 participants