Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Connected child component receives state updates earlier than the props computed from state by a parent connected component #1415

Closed
chandlerprall opened this issue Feb 17, 2016 · 2 comments

Comments

@chandlerprall
Copy link

Issue: asynchronous actions trigger re-render in non-deterministic order which causes some components to be in a bad state.

Example:
https://jsfiddle.net/chandlerprall/do2mnLr3/8/ (watch console)

  1. page loads and renders with “Lorem ipsum…”
  2. click “Change the State” button
  3. The proptypes on InnerComponent fails, InnerComponent re-renders with undefined
  4. InnerComponent re-renders again with the correct text

Breakdown:
When using the thunk middleware to asynchronously dispatch an action the mounted components are not rendered top-down but instead the listeners are processed in the order the components mounted (in the example above InnerComponent is mounted before OuterComponent).

If a property is pulled from the state and passed into another connected component that listens for a separate state value, the component re-renders with the old property but new state which can be out of sync. This is mitigated in synchronous dispatches because of how React enqueues pending changes, but could break in the same way if setState is changed to a true asynchronous model.

Walkthrough:
(the example demonstrates a split-store approach where a Page Level store would track the IDs of content to show and the content is stored separately, mapped by ID)

  1. page renders top-down with the initial state, everything is fine
  2. clicking “Change the State” waits 200ms before calling dispatch() and the reducer sets a brand new state
  3. InnerComponent’s Connect listener fires first; its textKey prop hasn’t changed but the store did, so shouldComponentUpdate returns true and InnerComponent uses an old key (‘lorem’) which is no longer valid in the store.
  4. OuterComponent renders, pulling the new textKey out of the store and passing it down through props to InnerComponent.
  5. InnerComponent’s shouldComponentUpdate returns true as the incoming property changed, it now uses the up-to-date textKey to pull from the content store and renders correctly.

Again, this issue is avoided when the dispatch happens synchronously because setState isn’t asynchronous, but from the ReactJS docs: There is no guarantee of synchronous operation of calls to setState and calls may be batched for performance gains. This may be a bigger issue in the future, affecting all operations.

This may belong in the react-redux project instead, if so I’ll gladly move it over there.

@gaearon
Copy link
Contributor

gaearon commented Feb 17, 2016

This definitely isn’t related to asynchronous dispatches, middleware, or Redux Thunk.

By default, React batches updates that happen during an event handler. This is why dispatching inside a click handler does not cause the problem. So in fact setState() is asynchronous inside event handlers, and that’s why you don’t have the problem: it being asynchronous means that components receive new props from Redux and from parent component at the same time during a batch later.

However if you dispatch from a network request callback or an interval, React has no knowledge of when “related updates” might happen so each setState() is synchronous. In this case updates happen as soon as they happen. When you dispatch, the subscribers are notified, and indeed, in this case, children are subscribed before their parents. So a child subscribed to Redux receives new props from connect() earlier than the parent component has a chance to supply it the new props. Hence the problem.

I’m not sure if this is something we can solve in React Redux easily. If you rely heavily on this style of connecting, there is a workaround: you can force React to batch the updates.

setTimeout(() => {
  ReactDOM.unstable_batchedUpdates(() => {
    dispatch({
      type: 'CHANGE-STATE'
    });
  });
}, 2000);

As you can see this in an unstable API but it solves the problem. You can also use something like redux-batched-subscribe store enhancer to do this automatically for every action:

import { createStore } from 'redux';
import { batchedSubscribe } from 'redux-batched-subscribe';
import { unstable_batchedUpdates as batchedUpdates } from 'react-dom';

const store = createStore(
  reducer,
  batchedSubscribe(batchedUpdates)
);

There may be better ways of addressing this but I am not aware of them.
If you still consider this to be an issue worth investigating, please file it in React Redux repo.

@gaearon gaearon closed this as completed Feb 17, 2016
@gaearon gaearon changed the title asynchronous dispatch creates bad state Connected child component receives state updates earlier than the props computed from state by a parent connected component Feb 17, 2016
@gaearon
Copy link
Contributor

gaearon commented Feb 17, 2016

Let’s discuss this in reduxjs/react-redux#292.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants