Store change not immediately visible to component

[htzhang2]

This code used to work in react-redux v5.0.7:

1. componentDidUpdate dispatch action, then call reducer to update store
2. reducer updates store
3. componentDidUpdate called again with new store value from selector

Step 3 no longer works in react-redux ^6.0.1:
selector not called

in 7.1.0, selector is called but the value not picked up by component props

[timdorr]

Can you provide an example of this on Codesandbox?

[markerikson]

I would definitely expect that there is a difference in behavior between v5 and v6. One of the specific changes we made for v6 was trying to pass the store state through context, thus ensuring that the entire tree always seems the same value during a render pass.

I would generally expect that v5 and v7 behave similarly to each other, though.

[Benjamin-Dobell]

I'm seeing this as well. Specifically I've got an event listener in a Component that calls setState(someState) and dispatch(someAction).

Logging shows that action is dispatched, and the reducer called before the component re-renders, however despite the store being updated by the reducer, mapStateToProps is not called before the component is re-rendered. As such when the render happens my internal component state is out of sync with the components props (coming from mapStateToProps).

However, I must admit, I can't remember whether this was always the case. I'm on [email protected] and was doing some refactoring when I ran into this race condition.

[YoshiYo]

I think I have the same problem when I updated the package to latest. ^7.1.0
I downgrade to 7.0.3.

---

My component had a value={aValue}. And I don't know why, on the onChange, this one dispatched the action, mutated the store and then render my component with new value, but it's was "bugged" liked the component had it's internal state changed before then apply the value from store.

It just happened on one component on my side, and I can't explain why. I didn't see this on my others components. And it was just a basic component. No massive store or something like this.

---

---

GIF on :

"react-native": "https://github.com/expo/react-native/archive/sdk-33.0.0.tar.gz",
"react": "16.8.3",
"react-redux": "^7.1.0",
"expo": "^33.0.6",

[timdorr]

Can you share that code via Codesandbox or just the source somehow? It would really help to have code to look at here.

Also, you probably don't want to store every bit of state in your store in general anyways. Things like form values are usually a smell that you're over-reliant on the store. (Sorry for the arm chair critique!)

[slavikdenis]

@timdorr here is a expo snack example. You can see the issue @YoshiYo is describing. After changing react-redux version to 7.0.3 this issue dissapears.

https://snack.expo.io/@denis.slavik/react-redux-7.1.0-rn-error

[slavikdenis]

Related sources:
facebook/react-native#24409
facebook/react-native#23578
https://stackoverflow.com/questions/43298709/textinput-flickering-choppy-behaviour-on-render

[Benjamin-Dobell]

@slavikdenis Not going to lie, the complexity of that example hurt my head so much it motivated me to make a minimal reproduction 😉

Actually, my results are different than yours, 7.0.3 is also broken for me. Need to go back to 6.0.1 to resolve the race condition in my REPL. However, @htzhang2 said that 6.0.1 doesn't work for them. I suspect there are a several different race conditions people are running into, perhaps related, perhaps not.

https://repl.it/@BenjaminDobell/ReactReduxRaceCondition

@timdorr The above repl.it is stock react (DOM) & react-redux. It will throw an error when the redux store (props) comes out of sync with StatefulComponent's state - which happens (but shouldn't) when you click the checkbox.

If you fork the project on repl.it. downgrade to 6.0.1 and re-run you'll see it goes off without a hitch.

However, more than that, I noted that the problem only rears its head when the parent component re-renders as well as a child. To verify this, if you go to App.js, I've marked two lines that, either of which, when commented out resolve the race condition.

Additionally, I've added 4 console logs through-out the code base; #1, #2, #3, #4 that should be logged in order. Thus if you open your browser console output, you'll be able to see that #3 (mapStateToProps) is not being hit before the re-render when using react-redux 7.x.x.

[Benjamin-Dobell]

@timdorr Sorry, just realized I better address this...

Also, you probably don't want to store every bit of state in your store in general anyways. Things like form values are usually a smell that you're over-reliant on the store. (Sorry for the arm chair critique!)

Yep, totally agree, although my example above is doing just that; my real-world app where I first encountered this issue definitely isn't doing that!

My real world app is storing information about async operations in progress (i.e. data fetching in progress) in redux. Various components through-out the app use this information to display spinners, disable buttons etc.

However, some internally stateful components (e.g. forms) which generate these async operations are kicking off an async operation (according to their state) but when they re-render immediately after kicking off the async operation, the component's props (from redux) indicate no such operation is in progress (until the next re-render). So there's this 1 render where things explode as states become out of sync, unless I introduce more stateful logic (which is my temporary work-around).

[timdorr]

Here's a "fix" for that example: https://repl.it/@timdorr/CalmImaginaryBacktick This is because we get the latest store state in the case that the incoming props have updated

This is ultimately caused by our top-down subscription mechanic. We ensure ancestor components are updated before descendants, so that if one of those descendants leaves the tree, they won't try to render that store update (the "zombie child" problem).

The problem is the update to the internal state of the descendant connected component is being batched at the same time as the ancestor connected component. It looks like this:

1. setState enqueues an update and dispatch updates the store.
2. The <Provider> fires its subscription and notifies the top-most connected component(s).
3. The top-most connected components render, including their sub-tree. The existing mapped props (stored in state) for the descendants are still the same, but include the setState update. (This is your mismatch)
4. The top-most connected notify the next level of descendants and they render with the new store state and mapped props. Everything is back in sync now.

It's basically two different state management systems fighting for control. React's built-in system will get its updates immediately and doesn't generally have to worry about ordering (at least not until Concurrent Mode...). But this library has to "bolt on" via whatever APIs are provided and therefore cannot influence that ordering.

I'm not sure there is a good fix for that. Like I mentioned at the top, you can "fix" it by providing updated props to your child components. Those would have to be drilled down if you're deeply nested, so it's not necessarily ideal. Another option would be to call store.getState() directly inside your render function to make sure you're dealing with the latest state.

I think while the example cases are useful to understand things, I don't see the practicality. That is, why are you storing the same bit of information (a box is checked) in two different places? I would argue that's architected incorrectly and the stateful elements around that operation should be unified into one place (Redux or React state).

[Benjamin-Dobell]

I think while the example cases are useful to understand things, I don't see the practicality. That is, why are you storing the same bit of information (a box is checked) in two different places? I would argue that's architected incorrectly and the stateful elements around that operation should be unified into one place (Redux or React state).

Guessing our messages crossed, just in case you missed it, I'm not doing that. Well, I am in the example obviously, but my real webapp is not using the redux store to store any user interface related state. It's exclusively being used to manage app-wide state, in particular track async operations that, once completed, themselves will update the redux store (i.e. the app-wide state).

Various components all through-out the app therefore need to know when a async operation is in progress. Sometimes that's as simple as displaying a spinner rather than stale content, other times it's to disable buttons, other times it's to modify those components to track the async operation, and sometimes it's to ensure that other components don't attempt to kick-off duplicate or conflicting async operations - granted the app logic does actually prevent that, but the UI should always accurately reflect what operations are possible.

[Benjamin-Dobell]

Okay, perhaps I better explain exactly what's happening my real-world app so you don't think I'm crazy. Granted, this is all highly use-case specific, quite boring, and honestly not particularly related to react-redux (beyond the seeming regression in functionality from 6.0.1 -> 7). But here goes...

I'm not storing the same data in two different places. Well, actually I wasn't, but am doing so now as part of my work-around for this issue - but it's certainly not my intention. In the past (react-redux <7) I was able to store my async operations only in the Redux store.

To be specific, the real-world issue I'm hitting is that my component has a "toggle" (of sorts) which among other things controls which child component(s) ought to be displayed. This toggle is indeed UI state, so it is stored as part of the component's state, not in the Redux store (which is pure model). However, when the toggle is "switched" we need to fetch a different set of data, for the newly visible child component(s). So a web request is kicked off, and a redux action is dispatched indicating the web request is in progress - such that in theory the entire app knows new data is on its way.

Because the toggle has changed, React correctly goes to re-render my component. The component correctly sees the new toggle state (internal component state), however props (from the Redux store) indicate there is no web request in progress (stale store data). There's supposedly no web request in progress, thus it shouldn't render a loading interface, but should instead render the child component corresponding with the "toggle state". However, the child component of course fails to render (throws errors) because the data it expects (doesn't matter where it attempts to get it from; passed props or redux store) simply doesn't exist - the child component should never have been displayed whilst the app is in this state.

There are of course workarounds, and I've implemented one already. There's also undoubtedly ways to fundamentally differently architect the app logic to decrease the likelihood of running into this sort issue. I've just chimed in here to report what seems to be a regression.

However, I also understand that we're limited with respect to what APIs React provides, particularly with all the internal rejigging going on post-Fiber. The work you're doing keeping up with all this is by no means under-appreciated, so thank you very much!

[Relax594]

Got this behavior as well. Going back to 7.0.3 'fixes' this. No change in code needed, just downgrade to 7.0.3 and everything is fine again. Seems weird to me.

[htzhang2]

We have 2 components: A and B. A and B are parallel (not parent/child).

A and B both dispatch actions to update store.
First A dispatch action in didMount, store updated with changeA.
Then B dispatch action in didUpdate, store updated with changeB.

In v5, after B dispatch action and called redcuer, B's didUpdate is called with changeB.

In v6 and v7, after B dispatch action, B's didUpdate is called with changeA. (changeB not visible)
Then B's didUpdate is called again with changeB.

v6 and v7 called B's didUpdate with changeA twice.

[timdorr]

@Relax594 There were only tiny changes to how connect works in 7.1.0, so I'm not sure why that version change would affect things. It sounds like something unrelated to this issue is going on for you.

[timdorr]

@htzhang2 See #1354 for that.

[Benjamin-Dobell]

Woh, closed?

This is a major issue. I'm not expecting you to resolve it overnight or anything like that, however I certainly would rather it not be swept under the rug. I gave a sample reproduction and detailed real-world explanation (no I'm not storing any UI state in my Redux store).

Sure, there are work-arounds (e.g. Bypassing react-redux and querying the store directly), however I'm not convinced that this can't be made to work (as it used to) using existing React APIs.

EDIT: To clarify, I'm not even necessarily expecting you to resolve it. If no-one else beats me to it, I fully intend to dig into this and submit a PR.

[timdorr]

Feel free to submit a PR. But again, it's at odds with our top-down mechanic and how React manages its own state. If it were possible, we would have resolve this several versions ago.

[Benjamin-Dobell]

Even if something is presently impossible, that doesn't mean it's not an issue. If it's not possible, this ought to be reported against React, and this issue reference it. If other projects do the same it'll get some weight behind it. Closing issues that are genuine issues doesn't help anyone.

[Relax594]

@timdorr all our apps are working fine in 7.0.3 but have these issues with 7.1. No other package was updated during this process. We worked a full day on trying to solve it ourself but couldn’t find a problem on our side.

[gguidotti]

Same here, I have no idea how I got 7.1.0 since my package.json was showing 7.0.2 but I can confirm I had the exact same issue described by @YoshiYo . Edited my package-lock.json to use the 7.0.2 instead of 7.1.0 did solve that strange flickering.

[markerikson]

I would find it genuinely surprising if there's an actual difference in behavior for connect between 7.0.3 and 7.1.0. The only change was a tweak to how we extract fields from the wrapper props:

-       const { context, forwardedRef, ...wrapperProps } = props
-       return [context, forwardedRef, wrapperProps]
+       const { forwardedRef, ...wrapperProps } = props
+       return [props.context, forwardedRef, wrapperProps]

[htzhang2]

For me, both 7.0.3 and 7.1.0 rendered twice

[Relax594]

For me, both 7.0.3 and 7.1.0 rendered twice

Did you update your package-lock after that? For me 7.0.3 works fine but 7.1.0 doesn't.

[WoodyWoodsta]

Can confirm that 7.1.0 doesn't work, but 7.0.3 does.

(Almost ripped out redux-form as a result, which needs ripping out at some point but industry projects have time and resource constraints 😄).

[markerikson]

I still truly don't understand how there's any meaningful differences between 7.0.3 and 7.1.0 related to this.

I don't have time to look into this myself at the moment. If someone can dig in, I'd appreciate a breakdown of the exact internal sequences that are supposedly causing this behavior.

[alimek]

hmm do we have any update on this issue ? or its just closed or any other solution to the problem?

[markerikson]

It looks like this is basically the same issue as #1298 .

I think the key analysis comments are #1313 (comment) and #1298 (comment) .

At the moment, I don't immediately see any solutions to this, but I also haven't tried to spend meaningful time trying to come up with one.

I also still don't understand how there can be any difference in behavior between v7.0.3 and v7.1.0 in this regard.

[markerikson]

Based on the discussion in #1437, I think this is a real bug, but not actually related to #1298.

The key issue is that this is specific to React Native. The problem is that we altered the definition of useIsomorphicLayoutEffect to better feature-detect DOM environments, and that appears to break in RN . So, it seems like we're running useEffect() accidentally, not useLayoutEffect().

[markerikson]

This should now be resolved in https://github.com/reduxjs/react-redux/releases/tag/v7.1.2 (but go install https://github.com/reduxjs/react-redux/releases/tag/v7.1.3 +, because I forgot to remove a console statement in 7.1.2 😟 )

I'd like to offer an apology to the folks in this thread for A) letting this issue slip past us, and B) not understanding what was actually going on here. I don't use RN myself, and so I wasn't familiar with the nuances of the RN environment regarding window and such. Because of that, I was focused on the one tiny change inside connect in 7.1.0, and missed that we'd made the change to the useIsomorphicLayoutEffect feature detection. I also didn't see that the common thread here was the reports were in RN projects.

I've added some unit tests to try to ensure that our RN support stays consistent with ReactDOM.

Please let us know if there's any further issues like this.

[Benjamin-Dobell]

Just in case anyone is confused. It seems there were two issues in this thread. @markerikson has resolved a React Native specific issue. However the issue I provided a REPL for (https://repl.it/@BenjaminDobell/ReactReduxRaceCondition) still persists with 7.1.3; as it's a result of react-redux 7's fundamental design, rather than a bug per se.

[maxalbrecht]

I am getting what seems to be the same issue. I am saving a new state through this.setState(newState), however if I immediately console.log(this.state) the new values in newState are not reflected until the next time I run my code. Unfortunately one of my dependencies (react-beautiful-dnd version 12.2.0) requires react-redux ^7.1.1, so at first glance it does not look like downgrading react-redux is an option. Thankfully the functionality of my app itself should not be affected by this, but it does make debugging more complicated.

[markerikson]

@maxalbrecht100 : setState() is just React, and has nothing to do with React-Redux. It sounds like what you're seeing is the fact that setState() is normally asynchronous - you cannot see the "new" state values until after React has actually re-rendered and updated your component.

[ilibaz]

The issue was not resolved in 7.1.3 in my case, going back to 7.0.3.
Exactly the same error is still being thrown on 7.1.3:

ReactWrapper::setState() can only be called on class components

[markerikson]

@ilibaz : I'm not familiar with that error message, and it doesn't look like it has anything to do with this thread.

[edinburghrules]

@timdorr all our apps are working fine in 7.0.3 but have these issues with 7.1. No other package was updated during this process. We worked a full day on trying to solve it ourself but couldn’t find a problem on our side.

I was getting this behaviour with react-redux v7.2.2 too, had to do a funky work around using componentDidUpdate. But thanks to your heads up going back to 7.0.3 completely fixed the issue. Thanks! 👍

[markerikson]

@edinburghrules Dropping back to 7.0.3 should not be a valid resolution here.

If you're seeing a problem with 7.1.3+, please file a new issue with a reproduction so we can look at it.