React-redux: Child may render with new state before parent

[OliverJAsh]

What is the current behavior?

Full reduced test case: https://github.com/OliverJAsh/react-redux-and-context-renders-test

Same reduced test case in the form of a StackBlitz: https://stackblitz.com/edit/react-redux-and-context-renders-test?file=index.tsx

The example might look contrived but it's based on real world code that is in production on Unsplash. Apologies in advance that I couldn't provide a simpler example—this is the smallest reduced test case I was able to find.

The simplified component tree looks something like this:

  Grid
    Item

The child component Item is rendered with new Redux state (windowWidth) before its parent, Grid. We can see this from the logs I added to component render functions:

What I expect: the child component Item _should not_ be rendered with the new Redux state before its parent, Grid.

The issue seems to occur when React re-renders due to simultaneous changes to context _and_ Redux state.

From my understanding, this is what's happening:

1. Item is subscribed to both the context value and Redux state.
2. After the first render, we simultaneously update the context value and Redux state. React will batch these renders because they are triggered by lifecycle methods (componentDidMount).
3. React Redux will re-render GridWrapper with the new state. At the same time, React will re-render context consumers with the new context value. Item is inside of a context consumer so it will re-render, but incidentally this will also mean it will use the new Redux state, even though React Redux hasn't told it to.
   
4. Finally, React Redux will re-render Grid with the new state.
   

As I understand it, the only way to avoid this problem would be to fix https://github.com/reduxjs/react-redux/issues/1510 so that nested updates at different levels are batched.

What is the expected behavior?

See above.

Which versions of React, ReactDOM/React Native, Redux, and React Redux are you using?

The latest versions of everything, at the time of writing.

  "dependencies": {
    "react": "16.13.1",
    "react-dom": "16.13.1",
    "react-redux": "7.2.0",
    "redux": "4.0.5",
    "typescript": "3.9.3"
  },

Which browser and OS are affected by this issue?

All.

Did this work in previous versions of React Redux?

I'm not sure.

[OliverJAsh]

For now we were able to workaround this by moving the React Redux connect to live outside of the context consumer.

Before:

Context.Consumer
  Connect
    Item

After:

  Context.Consumer
    Item

This way, when the context consumer is re-rendered due to context changes, the previous connected props will be re-used.

https://stackblitz.com/edit/react-redux-and-context-renders-test-hvhqwv?file=index.tsx

[markerikson]

Yeah, isn't this basically the same thing as #1510 ?

[OliverJAsh]

The same root cause, yeah! I just didn't realise it could result in discrepancies like this when used in specific scenarios such as when using React context, so I filed it as a separate issue. (Maybe it will also make the issue easier for someone to find if they are also running into this.)

I think this puts more emphasis on fixing https://github.com/reduxjs/react-redux/issues/1510. What do you think?

[markerikson]

I don't think the issue is "fixable" in v7 given our current architecture. I'd be happy to be proven wrong, and if anyone has any concrete suggestions on how we can do that, I'm absolutely interested. I just don't see how it's avoidable given the way we trigger nested subscriptions.

Looking forward, it's likely that React-Redux v8 will be based around use of useMutableSource for managing subscriptions. I don't yet know if that will be _only_ in useSelector, or if it's something we can leverage in connect as well. There's also the question of whether "context selectors" will actually get implemented - Seb said he thought they ought to be, but it doesn't sound like there's any plans to work on that any time soon.

I don't yet know enough about how that implementation might look to predict whether or not RRv8 would still have these issues.

I _think_ that useSelector doesn't suffer from this exact issue because they should generally get subscribed to the root, but it trades that off by having to deal with stale props (which is exactly what connect was trying to avoid in the first place). Although as I say that... even if there's no uses of connect in the codebase, <Provider> has a Subscription instance inside, and so maybe the timing on that would still cause this exact problem to still occur. Can you throw together another repro that does useSelector instead just for point of reference?

[OliverJAsh]

Thanks for your insightful answer. It's going to take awhile for those bits to register in my head!

Can you throw together another repro that does useSelector instead just for point of reference?

https://stackblitz.com/edit/react-redux-and-context-renders-test-ng6tpb?file=index.tsx

(I also had to add React.memo since we no longer have connect doing that for us.)

The issue doesn't occur with this example. So when using the hook, nested updates are batched (unlike when using connect)?

[markerikson]

Both connect and useSelector will subscribe to the instance of Subscription that is in the ReactReduxContext.

The difference is that connect, as a HOC, will always then render a new <ReactReduxContext.Provider>, and override the Subscription value from the ancestor with its own Subscription instance. So, as we nest components, there's a tree of subscriptions.

This is good for ensuring the "components never recalculate or render with stale props" invariant, but obviously has the edge cases we've seen here and in #1510.

useSelector, as a hook, cannot render context providers (or anything else). Therefore, in an app that only uses the hooks API, effectively all instances of useSelector will be subscribed to the single Subscription instance in <Provider>. That means that they will all get triggered in a single batched update.

This is good for ensuring that there's fewer individual commits and avoiding the edge case here, but runs into the stale props issue because nested use of hooks will triggering updates before the component has received updated props from its parent.

[OliverJAsh]

Very interesting, thank you so much for explaining that.

[sourabhv]

For now we were able to workaround this by moving the React Redux connect to live outside of the context consumer.

Before:

Context.Consumer
  Connect
    Item

After:

  Context.Consumer
    Item

This way, when the context consumer is re-rendered due to context changes, the previous connected props will be re-used.

https://stackblitz.com/edit/react-redux-and-context-renders-test-hvhqwv?file=index.tsx

This works for class component. How would one fix it when using functional components?