React-hot-loader: Error in a render method causes re-rendering to infinity.

[niba]

It's a bug.

Description

When one of your components throw error in a render method AppContainer causes re-rendering to infinity because of the way of handling errors using componentDidCatch

Expected behavior

Show error page, don't re-render components tree.

Actual behavior

AppContainer tries to load entire stack again

Environment

React Hot Loader version: 3.1.2

[thibautRe]

I've had that issue for a while too, it seems like it's because of the render method of AppContainer : if you don't pass an errorReporter prop, then the render method will just log the error but _still re-render the children_.

Now I'm not sure of what would be an appropriate behavior (maybe just logging the error and returning null but this might have some side-effects).
If we can agree on a proper behavior, I can submit a PR.

In the meantime, you can follow these steps, they should fix the issue since the render method returns the custom error reporter box.

[niba]

Current behaviour can be confusing for a people who recently upgraded to the latest version. I've spent a few hours trying to figure out what's hapenning. We should at least add some information in console or somewhere to warn users that this can happen if they don't provide error reporter.

[theKashey]

As I understand - the proper solution is just to break the cycle.
Ie https://github.com/gaearon/react-hot-loader/blob/master/src/AppContainer.dev.js#L56

 componentDidCatch(error) {
    if(!this.state.error) { // set error only once per update
       this.setState({
         error,
       })
   }
  }

And next, when user will update the code, that state will be reseted.
Currently, setState cause re-render, and next Application gets a new error, but with the same information. So, buy freezing update we will not break anything, except the infinite loop.
Sounds like a one-line fix.

[thibautRe]

Yes that sounds actually like a better behavior. Although I would argue that this logic should live in shouldComponentUpdate instead of componentDidCatch, since you're not 100% sure that this.state is up-to-date (this.setState being asynchronous)