-
Notifications
You must be signed in to change notification settings - Fork 1
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
Further discussion of React-Redux and render timing #1
Comments
Thanks Mark! I am glad you enjoyed it. When I saw your avatar I was like AHH it's one of the guys in the issue and was worried I made a mistake in my analysis of the issue. Glad that there was nothing crazy broken. I have tested against the most recent version of React-Redux (5.x) and the issue does repro. The reason I am using 4.x is actually due to a separate issue in that React-Redux 5.x circumvents https://github.com/tappleby/redux-batched-subscribe which I am using to implement a stopgap solution to this problem in my actual application. You can see the solution I use in the last slide of my talk: The issue with redux-batched-subscribe derives from it allowing me to control when the store actually notifies listeners that there was a change. The issue is that in React-Redux 5.x the components read from the store directly each time they are rendering (https://github.com/reactjs/react-redux/blob/master/src/components/connectAdvanced.js#L160). This is a problem since if we trigger a synchronous update to the store in the middle of a render cycle that update will be immediately available in every child component in the tree. So you are back to the original issue that my talk was focused on. The reason this behavior doesn't exist in 4.x is that in 4.x Connected Components only read from the store when their store listener is called - otherwise they use a cached version of the store from the last time they read. As for next steps - I would love to brainstorm a bit more about where we could go with this particular behavior. Are you and possibly some other members of the React-Redux team in SF? If so it would be awesome to grab coffee sometime and think about this. Thanks again for checking out the talk! :) |
Heh. I'm in Dayton, OH, Tim Dorr is in... North Carolina, I think? And I think Jim Bolla is in Pittsburgh. So, none of us anywhere near SF that I know of :) Interesting - are you saying that Tell you what. Go ahead and file a new issue against React-Redux, recap your use case and what you've learned, reference 210 and this repo, and let's see what we can discuss from there. The 5.x code is entirely Jim Bolla's work. I've looked through it / talked with him enough to understand the overall approach, but I don't claim to know the fine nuances of the implementation. I'm actually hoping to do something maintainer-ish this weekend and dig into the "5.0.3 breaks React-Hot-Loader" issue, which would have me playing with the code for the first time. |
Aw bummer - worth a try at least. :) To be clear Sounds good re: the issue. Would be good to continue the overall discussion and see some ideas that can be generated. |
For reference, I'm in Atlanta. Come see our amazing crumbling infrastructure some time! |
Was the talk recorded? |
@naw Yup! We will be putting up the full recording soon. I will link it here and on the readme. |
@naw The talk is up! https://youtu.be/jj-9tcU3Xtw |
Hi! Sophia pinged me as you were starting your talk, and I was able to watch almost all of it via the livestream. That was an excellent talk!
I'm one of the maintainers of Redux, and also participated in the discussion in reduxjs/react-redux#210 . You bring up some very interesting points regarding child components rendering after a parent dispatches in a lifecycle method.
The one question I have off the top of my head is, does this issue still reproduce in React-Redux 5.x, or only in 4.x? Taking a look at this repo's
project.json
, I do see that it's using 4.x, so I'd be curious if that makes a difference.As you said, the lifecycle/dispatch/rendering sequence is not really a bug per se, but sort of a fallout of a couple specific known behaviors.
I don't have too much specific to discuss at the moment, but given that you've clearly spent time looking into this issue and thinking about it, I'd absolutely love to have any feedback you can offer regarding possible improvements to React-Redux, especially once React Fiber starts going asynchronous.
Pinging @timdorr, @jimbolla, and @naw , since they were involved in the discussion in 210.
The text was updated successfully, but these errors were encountered: