ctrlplusb / react-async-component

Resolve components asynchronously, with support for code splitting and advanced server side rendering use cases.
MIT License
1.45k stars 62 forks source link

Infinite Loops are common and easy to occur #42

Closed bradennapier closed 6 years ago

bradennapier commented 7 years ago

If ANY problem happens then I get insanely fast errors going on. We need some sort of loop detection here to handle this. Not even sure why it happens - perhaps a way to solve it? Or am I just doing something wrong?

image

andylhansen commented 7 years ago

I'm also having this problem. Any chance of getting this fixed?

ctrlplusb commented 7 years ago

Hey @bradennapier and @andylhansen

Is there any chance you could put together a minimal repo example that reproduces this. Happy to help from there.

bradennapier commented 7 years ago

@ctrlplusb - I have had difficulty making it happen on demand. It seems to randomly occur so there is simply error handling somewhere not being handled and it seems to retry non-stop

Seems like it is likely related to hot reloading in some way. Perhaps it is saving old chunks and when it fails it just fails non stop rather than refreshing or something - really not sure. I will definitely try to find out how to re-create it....

One way that i have seen it happen a lot is if the async component actually fails to render all together the loop will occur.

image

mschipperheyn commented 7 years ago

Here's a repo where it occurs: https://github.com/mschipperheyn/react-universally/tree/styled-components-762

In this case, it;s related to styled components 2.0.0: https://github.com/styled-components/styled-components/issues/762

@ctrlplusb I noticed that if I referenced a process.env.variable within the async component, I get an infinite loop.

ctrlplusb commented 7 years ago

@mschipperheyn thank you for this valuable diagnosis. This will be very helpful!

I apologise yet again for my delayed responses. I have a lot going on at the moment, but will try. PR's gladly welcome if you reckon you may know the root cause. It'll likely have to be done against react-tree-walker.

frankleng commented 7 years ago

also ran into this. but hard refresh usually fixes it in my case. tho still not a viable solution to end users.

mschipperheyn commented 7 years ago

Just had another case. I am using react universally. My website responds to project.com and www.project.com but I had not been entirely consistent in using one or the other. In any case, when I was on project.com everything was fine, but on www.project.com after an initial load, it went into that loop. Perhaps it had something to do with using the canonical meta tag in Helmet. I'm not sure.

mschipperheyn commented 7 years ago

What are the thoughts on simply using the new import('./SomeComponent).then() syntax?

newsiberian commented 6 years ago

Hi, in my case I see the following loop. Can't reproduce it in dev environment. cryptit_168

Also, sometimes I see this: cryptit_169

bradennapier commented 6 years ago

I mean essentially the issue is that asyncComponent is the one catching the errors. Any error in the resolution of the component will cause an endless loop. I actually have been thinking this is more of an issue with webpack and it's retry logic when resolution fails but can't be sure really.

xzyfer commented 6 years ago

@ctrlplusb I've tracked this down the HRM workaround in asyncComponent.

  render() {
      if (
        sharedState.module == null &&
        !this.resolving &&
        typeof window !== 'undefined'
      ) {
        this.resolveModule()
      }

      if (error) {
        return ErrorComponent ? (
          <ErrorComponent {...this.props} error={error} />
        ) : null
      }

      [...]
  }

Whenever an error occurs resolving a component this.state.error = true, however handling the error happens too late. As a result in an environment where typeof window !== 'undefined' i.e. the browser, this results in an infinite render loop.

I guess either the error handling needs to be moved above the this.resolveModule() call, or the guard around it should take into account the error state.

ctrlplusb commented 6 years ago

Awesome work @xzyfer! Are you up for creating a PR? Appreciate your time spent looking into this. 👍

xzyfer commented 6 years ago

Happy to open a PR. I don't grok the semantics of the HMR patch so just let me know which if the proposed solutions is appropriate.

xzyfer commented 6 years ago

Opened #60