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

4.12.13 breaks hot reloading in some cases #1342

Closed
egorovli opened this issue Sep 15, 2019 · 13 comments · May be fixed by novalina26/anchor#1
Closed

4.12.13 breaks hot reloading in some cases #1342

egorovli opened this issue Sep 15, 2019 · 13 comments · May be fixed by novalina26/anchor#1
Assignees

Comments

@egorovli
Copy link

egorovli commented Sep 15, 2019

Description

In some cases hot updates are not applied, although the console says it has updated the changed modules and that the app is now up to date.

See the repo below for reproduction or this video: https://www.dropbox.com/s/4v0gxbkn9ytsvt7/react-hot-loader-4.12.13.mov?dl=0.

Previous version (4.12.12) works fine though.

Expected behavior

Hot updates are applied.

Actual behavior

Hot updates are not applied in some cases.

Environment

React Hot Loader version: 4.12.13

Run these commands in the project folder and fill in their results:

  1. node -v: v12.10.0
  2. npm -v: 6.10.3

Then, specify:

  1. Operating system: macOS 10.14.6 (18G95)
  2. Browser and version: Brave Version 0.68.132, Chromium: 76.0.3809.132 (Official Build) (64-bit)

Reproducible Demo

https://github.com/egorovli/react-hot-loader-4.12.13-demo

First, open ./pages/index.jsx. Note that there is an unused axios import on top. Try changing the text inside the component to something different (e.g. "Hello, world"). Nothing should change. Then, change the text into something different again (e.g. "Foo bar"). Notice how it changes to the previous value now ("Hello, world").

Now, remember the unused axios import. Remove it. Now the hot reloading should work as expected.

@hadimostafapour
Copy link

For me it was related to react-loadable (dynamic import).

If i try to force update component, It works:

shouldComponentUpdate(nextProps: Readonly<P>, nextState: Readonly<S>, nextContext: any): boolean {

        setTimeout(() => {
            this.forceUpdate();
        }, 0);

        return true;
    }

@theKashey
Copy link
Collaborator

So it's a old good "not updating the tree" problem:

  • every this is "updated"
  • visual update comes with the very first change

And one of the changes in v13 was about deepForceUpdate....

@theKashey theKashey self-assigned this Sep 15, 2019
@braceta
Copy link

braceta commented Sep 17, 2019

I have exactly the same problem with lazy/suspend as with the axios import in the example, but on importing a css file instead.

P.S: Just tested my code with 4.12.10 and 4.12.11 and works fine. Seems a bug in 4.12.13.

@alexseitsinger
Copy link

I believe I have a similar issue. I just tried downgrading to .12 with no luck. Any suggestions on how to debug this would be appreciated.

@theKashey
Copy link
Collaborator

@alexseitsinger - as long as downgrade hasn't helped you - your issue is not the same.

It's known that there was some issue prior v13, especially with hooks, but I am still not sure why it's not working sometimes (even if I have an example where it does not work) - all tests, and all the simple examples are working perfectly.

@alexseitsinger
Copy link

alexseitsinger commented Sep 19, 2019

I see. Okay.

In my case, it seems that updating something like <Component css={{minWidth: 200}}> (emotion) to anything else "updates" in the console, but no changes are displayed. However, when I change it to <Component style={{minWidth: 220}}> the changes are reflected correctly. Also, sometimes when the edit is changed back to <Component css={{minWidth: 200}}>, the hot update reverts to a prior version, also changing other elements back to a pervious version, rather than a new current version.

It seems likely this may be related to something outside of react-hot-loader, but I don't have a good idea of how to debug the issue. I have tried changing my webpack settings, among other things with no luck.

Anyway, sorry to redirect the focus here. Perhaps I should open another ticket?

@theKashey
Copy link
Collaborator

Yeah, I never seen such time traveling before. It should be theoretically impossible.

@alexseitsinger
Copy link

I suspect it is an issue with Emotion, rather than RHL. Regardless, thanks for your feedback.

@theKashey
Copy link
Collaborator

Long story short - the problem is not within RHL. I don't found the root cause, but found a way to solve the problem.

Let me first recall why Hot Loader was created - after Hot Module Update you are starting to using a new generation of components, which are not ref equal to the old one, and React kills the "old" tree.
So RHL was wrapping all components with proxies, which also was always pointing to the latest known version of components.
Years later we have removed proxies if you have hot-loader/react-dom installed somehow. And so components versions were controlled only by webpack module system for a while, and by some reason it was not working right.

I did a quick check - in React.createElement I read the last version of a component, and compare it to the one given - and sometimes they differ. More of it - with hungreds of components - just a few errors was displayed, indicating the problem with the deep update.

So another fix - is to override "old" types in the underlying fiber during the hot update, and look like it solved the problem...

theKashey added a commit that referenced this issue Sep 23, 2019
fix: resolve all components to their last versions, #1342
@theKashey
Copy link
Collaborator

released as 4.12.14. Should keep all components up to date, and also check all lazy-loaded stuff to be updated after the load.

In short - everything always should be up to date.

@theKashey theKashey reopened this Sep 23, 2019
@hadimostafapour
Copy link

@theKashey thanks, I update to new version and my issues got resolved, everything now goes fine.

@braceta
Copy link

braceta commented Sep 24, 2019

@theKashey thanks a lot for this. Tried 4.12.14 and it works fine to me now. :)

@theKashey
Copy link
Collaborator

What a relief :) Gonna close this issue then.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
5 participants