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

Types are messed up after HMR applied to out of rendered tree components, dynamically loaded using react-loadable #1050

Closed
asyncmax opened this issue Aug 16, 2018 · 4 comments
Assignees

Comments

@asyncmax
Copy link

Description

For the components that are loaded dynamically using react-loadable, a HMR update applied to a component that was once rendered, but currently out of rendered tree appears to mess up RHL's internal component type cache.

I created a small app to reproduce this case.

https://github.com/asyncmax/hmr-react-loadable

Clone the repo and run the following:

yarn
yarn hot

Open http://localhost:8080 in the browser.

Expected behavior

When you click radio buttons "Panel A" and "Panel B", you will see the bottom area changes to show a currently selected panel. These panels are loaded dynamically using react-loadable.

Actual behavior

While "Panel B" is selected, change the src/PanelA.jsx file. It will trigger a HMR and changes will be applied. Now when you click "Panel A" and then "Panel B", the bottom area doesn't change and keep showing the content of "Panel A".

In browser's console, there will be an error message showing two panels' type became the same.

  ...
  const panelA = <LoadableA/>;
  const panelB = <LoadableB/>;

  if (panelA.type === panelB.type)
    console.error("panelA.type and panelB.type are pointing to the same object!");
  ...

Environment

React Hot Loader version: 4.3.4

  1. node -v: 8.9.4
  2. npm -v: 5.6.0
  3. yarn -v: 1.9.4
    4: react-loadable: 5.5.0

Then, specify:

  1. Operating system: Windows, Mac & Linux (Yes, I ran the app on all three OSes to verify)
  2. Browser and version: latest Chrome

Reproducible Demo

https://github.com/asyncmax/hmr-react-loadable

@theKashey
Copy link
Collaborator

Oh, that doesn't sounds good.
RHL could merge components, but they have to have the same names, and almost identical content.

@theKashey theKashey self-assigned this Aug 16, 2018
@theKashey
Copy link
Collaborator

This is super cool stuff - you found big 💩!

React-Hot-Loader has a special thing, called hot replacement render, which tries to update component when it did not update them before.
In case of code splitting - unused code path was not updated, and by changing the route you are kicking RHL into this "hot-render" mode.
During that render it will find a new component, which is just slightly different from the old one - contains A, not B. But the name will be the same.
And then it will do it's job - hot update stuff.

Problems here:

  • attempt to hot-update something out of HRM phase.
  • attempt to swap "registered" components by the mechanism created for "not-registered"(internal)

Thank you for this bug.

@asyncmax
Copy link
Author

Thanks for taking time to investigate and fix this problem. I will try your fix on the original, bigger repo and let you know what I have found.

@theKashey
Copy link
Collaborator

Released in 4.3.5

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

2 participants