State updates can lead to multiple re-renders

[frontendphil]

What is the current behavior?

In our application, we provide part of our state as context to multiple components. With the latest update 7.0.1 we see an error popping up that is the result from the context and the state a component use being out of sync. Both components are connected to the same store.

The scenario is as follows:

• Parent component (provides part of the state as context)
• Child component uses both context and also connects to store

If an update is dispatched to the store then a re-render is caused by the parent component putting a new value on the context before the child component is notified about the change in the data. This leads to a situation where the child component operates on stale data while simultaneously referring to new data coming in through the context. This then causes our application to crash.

While creating this issue I realize that we're using react-broadcast@0.7.1 for the context. I'm going to rewrite that to the official context API and see whether this influences the behavior.

[UPDATE]: Using the official context API does not solve this issue.

What is the expected behavior?

Both updates are flushed in one go and the application only re-renders once with all components operating on the same version of the data.

Which versions of React, ReactDOM/React Native, Redux, and React Redux are you using? Which browser and OS are affected by this issue? Did this work in previous versions of React Redux?

• react@16.8.6, react-dom@16.8.6
• react-redux@7.0.1
• redux@4.0.1

[timdorr]

We ensure the correct top-down ordering of state updates, provided you're using the state directly from your connect() props. Using context to pass that state doesn't let us control the ordering at all; React handles it itself. As such, there's nothing we can really do about this. It's just a React behavior, not a bug.

If ordering is important, you should be just selecting that state directly from connect() and not use a different source.

[frontendphil]

Thanks for being my rubber duck. After I gave that whole topic some more thought I came to the same conclusion. You're absolutely right and this was a well hidden but super weird design decision we made. Changing it makes the code easier to follow, highlights dependencies and also makes tests easier to write and maintain. It also might explain some weird bugs we've seen in the past. Definitively a case of a shitty abstraction where someone was too lazy to type and used context where they shouldn't.

[frontendphil]

@timdorr after spending another day refactoring I essentially still experience the same bug but could pinpoint the root cause to a different source. Could you give me an advise whether we're using the library in the wrong way or whether this might actually be something of a bug?

Just some context. We're building an application where you can build workflows. When you're editing exactly one workflow we'll put this one into the state (under the key workflow). Additionally, workflows have activities.

Now we're in a situation where we're composing a bunch of higher order components. In our situation, two HOCs connect to the store. The one higher up in the tree creates two props activity (one particular activity) and workflow the whole workflow. Some levels deeper a second HOC also connects and also creates workflow.

When we now dispatch an action that modifies the state the "inner" component re-renders before the outer component re-renders. This means that workflow and activity are "out of sync" (If I would look for activity in the new workflow prop it wouldn't be the activity which was created by the "outer" connect). This, in turn, creates a bug.

My question is: Is this an anti-pattern we shouldn't be using?

I would have expected that the application will only re-render once and that data from the outermost connect "flows down" so that all connects always represent the same data from the state.

In this particular situation, I can avoid the bug by not using the whole state but only pulling out the parts that I'm really interested in. However, this seems just that tiny bit too circumstantial.

I'd appreciate any advise!

[timdorr]

Are you connecting using React Redux connect() or your own HoC? One of the things we guarantee is top-down subscription handling, so the situation you're describing is something we specifically do and won't be an issue if you're only ever getting store state directly from connect(). It's designed for performance, so it's fine to nest them and use many of them in your app.

[frontendphil]

I'm connecting only using connect from react-redux. The HoC in question also doesn't use anything else. However, I can consistently reproduce that the inner component receives a new state before the out component does.

[markerikson]

@frontendphil : it would really help if you can provide a repro project, preferably as a CodeSandbox.

[markerikson]

While it may not be exactly related, a user on Reddit reported some noticeable differences in React commit sequences between v6 and v7. v7 is actually faster, but they took the time to create a repro anyway. Linking here for point of comparison:

https://www.reddit.com/r/reactjs/comments/bb5b3m/reactredux_7_released/ekh8i53/

https://github.com/trappar/redux-7-multi-commit-issue

I loaded it up in CodeSandbox and did a quick React profile, and this is behaving exactly as I would have expected. There's 4 levels of nested connected components, and so each level of nesting is getting updated in turn due to the cascading subscription behavior.

[frontendphil]

I know :) I'm trying to re-create this in a simple fashion but so far with no "luck". In all examples, it behaves exactly as I would expect. I'm investigating further whether this has anything to do with other libraries/structure/etc and it only looks like it's coming from react-redux...

[frontendphil]

Turns out 7.0.2 doesn't cause the problem described above. I'll stop my investigation for now. Anyway thanks for the discussion!