ReactReduxContext - infinite loop with react-hot-loader

[GuillaumeCisco]

Hello there,

I'm creating this issue as a following of #1126
I've discovered a new issue regarding multiple concurrent calls on the server side, and its fix leads me to a new issue: an infinite loop when using hot reloading with react-hot-loader. I need your enlightenment.
I described more precisely the new issue here: faceyspacey/react-universal-component#172

First of all, we need to clearly identify what can possibly going wrong, we will separate the client side from the server side at the beginning to conjugate them at the ending.

First issue: @rgrove described it very well with <Provider> misses state changes that occur between when its constructor runs and when it mounts in #1126.

We found a solution that is working great by using an HOC https://codesandbox.io/s/ryxk561qp4:

import React, { Component } from "react";
import { ReactReduxContext } from "react-redux";

export default function withRedux(WrappedComponent) {
  class WithRedux extends Component {
    constructor(...args) {
      super(...args);
      this.firstRender = true;
    }

    render() {
      if (this.firstRender) {
        this.firstRender = false;
        return (
          <ReactReduxContext.Consumer>
            {reduxContext => (
              <ReactReduxContext.Provider
                value={{
                  ...reduxContext,
                  storeState: reduxContext.store.getState()
                }}
              >
                <WrappedComponent {...this.props} />
              </ReactReduxContext.Provider>
            )}
          </ReactReduxContext.Consumer>
        );
      }
      console.log("not first render");
      return <WrappedComponent {...this.props} />;
    }
  }

  return WithRedux;
}

It works great on the client AND works great on the server.
But in a real application, you can have concurrent server calls. The best way to illustrate it is when using a service worker for caching routes.
Scenario with SSR + Service worker cache client:
server render app -> client loads app and takes priority -> service worker file is fetched and initialized -> routes to cache are fetched concurrently

Second issue: when injecting a reducer (dynamic loading) from a lazy loaded component, we need the store reference. In a single page app, the store is a singleton. On the server, with concurrent calls, the store is overrided. Which leads to errors when trying to render components that need injected reducer.
Example: two concurrent calls -> server render app with a global store -> inject reducer in the global store -> global store is reinitialized from the second concurrent call -> render of the component fail.
Same issue as the first one.
For fixing this we need to inject the reducer in a context store. This way we secure one store by concurrent call for the rendering of our routes.

From [email protected], using static contextTypes for the store won't work. And so these lines will not pass the context store: https://github.com/faceyspacey/react-universal-component/blob/master/src/index.js#L110

static contextTypes = {
      store: PropTypes.object,
      report: PropTypes.func
    }

What I'm using for loading component lazily is react-universal-component: https://github.com/faceyspacey/react-universal-component.
As you can see in the documentation, the onLoad method allow me to do things on the store like injecting reducers or dispatching actions:

onLoad is a callback function that receives the entire module. It allows you to export and put to use things other than your default component export, like reducers, sagas, etc. E.g:

onLoad: (module, info, props, context) => {
  context.store.replaceReducer({ ...otherReducers, foo: module.fooReducer })

  // if a route triggered component change, new reducers needs to reflect it
  context.store.dispatch({ type: 'INIT_ACTION_FOR_ROUTE', payload: { param: props.param } })
}
As you can see we have thought of everything you might need to really do code-splitting right (we have real apps that use this stuff). onLoad is fired directly before the component is rendered so you can setup any reducers/etc it depends on. Unlike the onAfter prop, this option to the universal HOC is only fired the first time the module is received. Also note: it will fire on the server, so do if (!isServer) if you have to. But also keep in mind you will need to do things like replace reducers on both the server + client for the imported component that uses new reducers to render identically in both places.

Now, the fourth parameter context gives {store: undefined, report: undefined}. Which does not allow us to use a context store and fixing our issue.

So I thought about using the ReactReduxContext for passing it myself. In my opinion, react-universal-component and other lazy loading libraries should not have to deal with redux/react-redux. The way react-universal-component could work with the redux store before 6.0.0 was a convenient enhancement.
You can read the issue on react-universal-component here: faceyspacey/react-universal-component#172

Updating my codesandbox, I ran into some funny issues.
This one does not work: https://codesandbox.io/s/5mk3564w0k
This one works: https://codesandbox.io/s/vmwm085633

The modification is in the route.js file.
I wrap my lazy loaded component in the ReactReduxContext.Consumer for getting the context store.
Interestingly using the props context will not work, and using the props reduxcontext will.
I wonder why, If you have any idea about that. I do not see why context should be a reserved name for a props.

By writing:

import React from "react";
import universal from "react-universal-component";
import { injectReducer } from "redux-reducers-injector";
import { ReactReduxContext } from "react-redux";

const UniversalComponent = universal(import("./component"), {
  loading: <span>loading</span>,
  onLoad: (module, info, { reduxcontext }) => {
    console.log("inject reducer");
    injectReducer("api", module.reducer, false, reduxcontext.store);
  }
});

export default () => (
  <ReactReduxContext.Consumer>
    {reduxContext => <UniversalComponent reduxcontext={reduxContext} />}
  </ReactReduxContext.Consumer>
);

we fixed our issue! 🎉 it is working great on the server with concurrent calls and on the client.

Unfortunately there is a very uncomfortable side effect.
With this new code, adding the ReactReduxContext.Consumer for the lazy loading part; if we modify something in our component and we have hot reload activated with react-hot-loader, it leads to an infinite render loop.

I'm really curious to understand what is going on.
Is this a bug with react-hot-loader or with react-redux? Having one Consumer, then a child with a Provider and inside this child a Consumer again, all from the same context leads to this behavior when hot reloading do things?
As we cannot put react-hot-loader in a codesandbox sample, this issue is worth having its own project.
I'll create it if anyone is interested for testing things.

Furthermore, I retested the codesandbox with the withRedux HOC that is working: https://codesandbox.io/s/ryxk561qp4
Try clicking on the fill results button, then modify something in the render method of the component MyComponent, you'll see a codesandbox hot reload, then try to click on fill results button again, it won't work.
I do not know what kind of hot reloading use codesandbox, it would be interesting to know.

I think this issue can deeply interest you @markerikson and @Ephem.
I hope this use case will help us building great applications and will deliver a robust documentation on how to use dynamic reducer injection with lazy loaded component and the new Context API.

Thank you for reading,

[markerikson]

Yipes, long complicated comment.

TBH, I just skimmed through this. I've never worked with SSR, so all that stuff is mostly over my head.

Per your questions about the context prop: in v6, connect can specifically accept an alternate React context instance to use instead of the default ReactReduxContext, and it specifically checks for props.context. So, just as store was a "reserved prop" up through v5, context is now a "reserved prop" in v6. I would assume that something about passing down a different value as props.context is causing a problem.

We've now got this behavior documented: Accessing the Store.

[GuillaumeCisco]

Ok folks, I have very good news 👍 . I succeeded to make it work with react-hot-loader.
The error I got is very similar to supasate/connected-react-router#204
Here connected-react-router is used. Looks like they fixed their issue in v6.1.0 release available here. I still do not know what they changed. Looks like the solution I implemented using firstRender.

First of all, we now know how to reproduce this issue. It appears when we use <ReactReduxContext.Consumer> before using universal from react-universal-component AND if there is a dispatch action in the componentDidMount of the dynamically loaded component.
When modifying the source code and waiting for hot reload, the componentDidMount method of the component is called a second time, this does not occurs without dynamic loading. Then a dispatch action is triggered which recall the componentDidMount method and so on.
<ReactReduxContext.Consumer> receives the update and componentDidMount triggers the update. The explanation of our infinite loop.
What we really wants here, in a development environment is to be able to inject reducer once and reload them on hot reload. We do not need to inject them at every hot reloading. Furthermore, when we want to reload them on hot reload, we do not need a specific context store as we are in the case of store singleton in a SPA.

The simplest solution I came for is to simply use a firstRender boolean like we did for the withRedux HOC.

The correct code looks like:

import React, {Component} from "react";
import universal from "react-universal-component";
import { injectReducer } from "redux-reducers-injector";
import { ReactReduxContext } from "react-redux";

class Universal extends Component {
    constructor(props) {
        super(props);
        this.firstRender = true;
    }

    render() {
        const UniversalComponent = universal(import("./component"), {
            loading: <span>loading</span>,
            onLoad: (module, info, { reduxcontext }) => {
                if (reduxcontext && reduxcontext.store) {                
                    injectReducer("api", module.reducer, false, reduxcontext.store);
                }
       }
     
       if (this.firstRender) {
            this.firstRender = false;
            return (<ReactReduxContext.Consumer>
                {(reduxContext) => <UniversalComponent reduxcontext={reduxContext}/>}
            </ReactReduxContext.Consumer>);
        }

        return <UniversalComponent/>;
});

export default Universal;

Hope it will help others.
ReactReduxContext is clearly not something easy to work with complex environment. And god know how frontend is complex these days...

Thank you,