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

Different behavior for multiple actions dispatched at the same time, 5.1.1 vs 6.0.0 #1105

Closed
jochakovsky opened this issue Dec 7, 2018 · 4 comments

Comments

@jochakovsky
Copy link

Do you want to request a feature or report a bug?

Bug

What is the current behavior?

In 6.0.0, if multiple actions are dispatched immediately after one another, connect will only evaluate the new state after the last action and will not evaluate any intermediate states. This may result in fewer renders than in 5.1.1. This is probably a good thing, but it is a breaking change and should probably be added to the docs.

Reproduction steps
To see 5.1.1 behavior, clone https://github.com/jochakovsky/react-redux-multiple-actions-one-render, yarn install, yarn start. Click the button in the app, look at these logs in the console:

mapStateToProps called with 'thing' as false
App component mounted
mapStateToProps called with 'thing' as true
mapStateToProps called with 'thing' as false
prop 'thing' was false, is now false

Note that the App component was not re-rendered after the first action even though mapStateToProps was called and returned a new state than it previously returned, but was re-rendered after the second action. To me, it would make sense for connect to re-render the component after each action with the new props, or to not re-render it at all since the state after the second action is the same as the state before the first action. 5.1.1's behavior of re-rendering only after the second action, with the same props as the last time it rendered, is odd to me.

You can see the behavior of 6.0.0 in this repo by just upgrading the dependency for react-redux in package.json to 6.0.0. With 6.0.0, the console reads:

mapStateToProps called with 'thing' as false
App component mounted
mapStateToProps called with 'thing' as false

With 6.0.0, the multiple consecutive actions are somehow only evaluated after the last action, and therefore react-redux detects no prop changes and does not re-render.

In some ways, the 6.0.0 behavior makes a lot more sense, but I feel like this difference in behavior should be mentioned somewhere. Was this a known issue with v5, or did v6 accidentally fix a v5 bug that we didn't even know about?

What is the expected behavior?

Not sure

@timdorr
Copy link
Member

timdorr commented Dec 7, 2018

This is mentioned in the release notes:

Also, there is a behavior change around dispatching actions in constructors / componentWillMount. Previously, dispatching in a parent component's constructor would cause its children to immediately use the updated state as they mounted, because each component read from the store individually. In version 6, all components read the same current store state value from context, which means the tree will be consistent and not have "tearing". This is an improvement overall, but there may be applications that relied on the existing behavior.

@timdorr timdorr closed this as completed Dec 7, 2018
@jochakovsky
Copy link
Author

@timdorr thanks for the quick reply! I did see that in the release notes but assumed it didn't apply since it specifically mentioned dispatching actions in constructors/componentWillMount. Should the release notes be updated to be more general?

Re. the 5.1.1 behavior, is that a bug in v5? It would make more sense to me if v5 had rendered the component twice.

@markerikson
Copy link
Contributor

markerikson commented Dec 7, 2018

It's presumably related to how we changed from individual components subscribing and calling setState() on themselves, to only having <Provider> subscribe and call setState() at the root.

You might want to read my post Idiomatic Redux: The History and Implementation of React-Redux to understand some of the internal changes that happened in v6.

Specific to the callstack logging you're doing:

With v5, the individual components subscribe, and all the mapState logic runs right away. The wrapper components only call setState() once we know that the child props have changed, and thus the wrapper component needs to re-render to pass those to the child. React ultimately batches the setState() calls in this component, since it's running in an event handler, but mapState did run twice and saw both states.

In v6, only <Provider> subscribes, so every store update triggers a setState() call in <Provider>. Since React is still batching setState() calls from this event handler, only the final state is passed down via context to the individual connected components, and thus mapState only runs once because there's only one React render pass.

In general, I don't think we've ever guaranteed that your components will actually see every individual store state change that occurs. This is particularly true if you're using a batching or debouncing store enhancer to cut down on the number of store update events, and may also be true depending on how React-Redux interacts with React. This is also likely to become a bigger point as we work on interacting with React's upcoming Suspense and time-slicing capabilities.

@jochakovsky
Copy link
Author

@markerikson that makes perfect sense, thank you very much for the detailed response!

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

3 participants