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

render is called in the wrong order for connected nested components #509

Closed
ynonp opened this issue Oct 3, 2016 · 9 comments
Closed

render is called in the wrong order for connected nested components #509

ynonp opened this issue Oct 3, 2016 · 9 comments

Comments

@ynonp
Copy link
Contributor

ynonp commented Oct 3, 2016

Consider the following where both components are connected to a store:

<Parent>
  {hasChild && <Child />}
</Parent>

hasChild is a parent prop that is received from store.

If a single action changes both hasChild and a separate property that affects the child, first Child's render is called and only then Parent's render is called removing Child from the tree (unlike normal react flow)

Currently I used shouldComponentUpdate in Child to prevent redundant renders, but that involves implementing shouldComponentUpdate in every case.

Any ideas for a generic solution?

@jimbolla
Copy link
Contributor

jimbolla commented Oct 3, 2016

This is fixed in v5, which is currently in prerelease. You can try it out by npm install react-redux@next

@timdorr timdorr closed this as completed Oct 3, 2016
@esamattis
Copy link
Contributor

You can also workaround this by enabling batched updates http://stackoverflow.com/questions/38099512/redux-child-receiving-props-before-parent/38102408#38102408

@ynonp
Copy link
Contributor Author

ynonp commented Oct 3, 2016

Thanks @epeli this worked very well! BTW is this the way you solved it in v5?

@jimbolla
Copy link
Contributor

jimbolla commented Oct 3, 2016

Nope. Doesn't rely on batched updates. Basically, control the order of subscriptions to always fire top-down. Details are in #416 and #407 if you're curious.

@Mythli
Copy link

Mythli commented Apr 26, 2018

Is there a way to turn that on or is it on by default? Cuz I'm struggling with the same issue > v5.

I skimmed through your post @jimbolla but it seems that I'm to unfamiliar with Redux to follow through. Would love it if you could give me a hint.

I have also posted a more detailed question with example to SO:

https://stackoverflow.com/questions/50041988/connect-deep-connected-components-dependent-on-top-connected-components

@markerikson
Copy link
Contributor

@Mythli : can you post an example repo or CodeSandbox to illustrate what you're doing? Per Jim's earlier comments, v5 does indeed enforce top-down subscriptions. So, if you have a connected list rendering connected list items, the list will have a chance to update first, and that should handle deletion scenarios correctly.

@Mythli
Copy link

Mythli commented Apr 30, 2018

Hello markerikson, I'm trying to reproduce the issue now and provide an example. What I found out is that if I cloneDeep() my state everything works fine. This hints at state corruption on my side and the problem not having anything to-do with connect().

I'l let you know as soon as possible what I found out.

@Mythli
Copy link

Mythli commented Apr 30, 2018

cloneDeep though does not resolve the issue - it was something else that worked: If I move the component to another nesting level everything starts working fine... Which I can totally not explain myself.

State corruptiuon is therefore ruled out and I'm out of my wits :(

This is the test case (which is working fine).
https://codesandbox.io/s/p2wm6r8nj

Would you be willing to do a review together? My company is willing to pay for that of course.

Best regards

@markerikson
Copy link
Contributor

@Mythli : I might have some time later this week. I'd suggest dropping by the Reactiflux chat channels on Discord (https://reactiflux.com) and asking - there's plenty of others around there who can help answer as well.

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

6 participants