Infinite render loop in 4.12.1 and up.

hakanderyal commented 5 years ago

Description

For versions 4.12.1 and up (last tested at 4.12.5), my app goes into a infinite render loop, resulting in "Invariant: Maximum update depth exceeded" error displaying in the rhl redbox anytime it hot reloads, or on pages that makes change to redux state. It works fine in 4.12.0.

The error is the result of one of the root components of my app, exported with react-redux connect:

export default connect(mapStateToProps, undefined, undefined, {pure: false})(MyComponent)

If I change the pure option to true, the problem disappears.

export default connect(mapStateToProps, undefined, undefined, {pure: true})(MyComponent)

Don't have time right now to create a reproducible demo, I'll look into it if its needed.

Expected behavior

It's shouldn't break.

Actual behavior

It goes into an infinite render loop.

Environment

React Hot Loader version: 4.12.1 and up

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

node -v: v11.9.0
npm -v: 6.5.0

Then, specify:

Operating system: MacOS Mojave
Browser and version: Chrome/Firefox latest

theKashey commented 5 years ago

does it work right with 4.12.0?
does it not work right with 4.12.3 or 4?
what is react-redux version?

I've checked code changes between 4.12.0 and 4.12.1 and there is only one place related not related to hooks, and it's quite small.

hakanderyal commented 5 years ago

Yes, it works fine in 4.12.0
It breaks in all of the versions between 4.12.1 - 4.12.5, tested each of them separately.
react-redux@7.1.0, redux@4.0.1, @hot-loader/react-dom@16.8.6, @babel/core@7.5.0 (also happens with @babel/core@7.4.5)

I did some further testing, and this only happens when there are 2 nested components at the root with {pure: false}. It works correctly with only one.

theKashey commented 5 years ago

Ok, sounds like infinity loops in hooks. cc @natew

So:

2 nested components

<Component1>
<Component2/>
</Component1>

both just mapStateToProps? Are they reading something from props?

Just help me reproduce it.

hakanderyal commented 5 years ago

I was able to reproduce it with 1 component, here are the details:

using babel plugin "react-hot-loader/babel"
using webpack alias: 'react-dom': '@hot-loader/react-dom',
react-redux@7.1.0, redux@4.0.1, @hot-loader/react-dom@16.8.6, @babel/core@7.5.0, webpack@4.35.2, webpack-dev-server@3.7.2
webpack command webpack-dev-server --hot --config webpack/webpack.config.dev.js

app.js

import { hot } from 'react-hot-loader/root'
import { Provider } from 'react-redux'

import store from './store'

import { MyComponent1 } from './test'

const App = () => (
  <Provider store={store}>
    <MyComponent1>
        test
    </MyComponent1>
  </Provider>
)

export default hot(App)

test.js

import React from 'react'
import { connect } from 'react-redux'

export class MyComponent1 extends React.Component {
  render() {
    return (
      <div>
        {this.props.children}
      </div>
    )
  }
}

MyComponent1 = connect(state => state, undefined, undefined, {pure: false})(MyComponent1)

When I update the test component:

(MyComponent1, in Connect(MyComponent1) (created by App)) Invariant Violation: Maximum update depth exceeded. This can happen when a component repeatedly calls setState inside componentWillUpdate or componentDidUpdate. React limits the number of nested updates to prevent infinite loops.

// edit: If any of the child components of MyComponent1 updates the redux store, it also triggers the infinite loop.

theKashey commented 5 years ago

👍I'll update tests and double check. Still not sure how changes between 12.0 and 12.1 might affect it.

ghost commented 5 years ago

Just want to +1 this. I was having an issue with useSelector in an unrelated component (notifications) causing an infinite render only on a particular unrelated page (neither are using the others redux state or modifying it, especially on render) as reported by this module and when I downgraded to 4.12.0 it works fine and anything above 4.12.1 breaks. Very odd as I said, these components are completely unrelated. When I remove the useSelector it doesn't break :/

import React, { useEffect } from 'react'
import { useSelector, useDispatch } from 'react-redux'

import { dismissServerNotifications as _dismissServerNotifications } from './actions'
import { getNotifications } from './reducer'

function Notifications() {
    const notifications = useSelector(getNotifications)

    const dispatch = useDispatch()

    useEffect(() => {
        const dismissServerNotifications = notifications => dispatch(_dismissServerNotifications(notifications))

        if (notifications.length)
            dismissServerNotifications(notifications)
    }, [])
...

dmarkow commented 5 years ago

Similar issue here -- but instead of getting an update depth error, Chrome just freezes the page and pegs my CPU at 100%. If I throw a couple console logging lines into my component hierarchy, I can see the whole tree rendering 20-30x/second.

Rolling back to my dependencies from last week (react-hot-loader@4.12.0, babel plugins at 7.4.5, etc.) everything works fine. Upgrading react-hot-loader to anything newer (4.12.1-4.12.5) causes the issue. I'm not using react-redux, though it looks like one of my dependencies (react-beautiful-dnd) is.

Also, disabling the 'react-dom': '@hot-loader/react-dom' webpack alias seems to at least stop the infinite loop (though probably breaks hooks support).

theKashey commented 5 years ago

:oof:

nmbrone commented 5 years ago

Just wanna confirm the same issue. Didn't spend time to find what exactly causes it in my case, just got rolled back to 4.12.0 as it works perfectly for me now.

bafxyz commented 5 years ago

same issue

theKashey commented 5 years ago

Problem has been triaged - related to hooks reloading Fixing....

RHL is adding a dependency the any hook with dependency which represent "hot-reloading sequence" There are 3 ways to track that sequence:

on component register(babel plugin). That's a wrong way, but it's in use right now for almost all cases. Does not play well with code splitting - it shall not activate any hot replacement logic.
on component secondary register, that's already much closer to the realty. This thing is used to track "sequence" for hooks.
somewhere in hot, ie track HMR events. But some applications (react-static) uses HMR without hot helper.

The problem is with .2 - component "autodiscovery" breaks logic, moving generation forward during hot replacement process, thus causing infinite loop.

Fix - dont change it if hot replacement is already enabled.

theKashey commented 5 years ago

v4.12.6 released. In my tests everything is good.

hakanderyal commented 5 years ago

Confirming, the problem no longer occurs in my code.

dmarkow commented 5 years ago

Confirming 4.12.6 stops the infinite render loop.

However, I'm now seeing that my useState calls get re-initialized when a component hot reloads. I have a component with an input tied to useState. On 4.12.0 if I type something into the input field, then make a change to the component, the component hot reloads but keeps what I typed. On 4.12.6, it clears out what I typed back to the default.

I added a useEffect hook that should only print when the component gets unmounted:

useEffect(() => {
  return () => {
    console.log("This should only print when the component gets unmounted");
  };
}, []);

On 4.12.0, this line never gets printed and my useState values don't get reset. On 4.12.6, it gets printed after hot reloading is complete, at which point my useState values reset.

theKashey commented 5 years ago

@dmarkow - that's another issue, and there are 2 things, which may cause it:

RHL failed to do the job, and React first destroyed the old tree, then created a new one. That does not mean to be a "full page reload", but could happen at any place.
Hooks reloading (⚛️🔥🎣 Hook order change detected: component) did a false positive, so you got a component remount to overtake the rule of hooks.

Please create a new issue with an example to reproduce - only you have a broken example (at least yet)

dmarkow commented 5 years ago

@theKashey #1299

GuillaumeCisco commented 4 years ago

Hello I got the same issue recently. 4.12.6 to 4.12.13 are working correctly, but from 4.12.14, it starts doing the infinite loop again. I've tested up to 4.12.21.

Did something huge change between 4.12.13 and 4.12.14?

Notice that 4.12.14 freezes the tab with infinite loop, with 4.12.15, tab is not freezed but infinite loop can be seen.

Can we reopen this issue?

theKashey commented 4 years ago

@GuillaumeCisco - please provide an example to reproduce. It's not known how to cause an infinite loop.

Did something huge changed between 4.12.13 and 4.12.14?

Lets double-check the logs...

GuillaumeCisco commented 4 years ago

I will try to provide a code to reproduce it, but I think this is the same as before ;)

Cool-Star commented 3 years ago

我在一个项目中也复现了此问题，经排查，无论高低版本的hot-loader都会出现此问题。近日得以解决，原因是react-loadable 的写法问题，当loadable每次编译不是准确的编译到同一流程时会触发死循环无限渲染。

eranimo commented 3 years ago

This is still happening in 4.13.0

gaearon / react-hot-loader