-
Notifications
You must be signed in to change notification settings - Fork 11
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
Concurrent mode? #15
Comments
Don't forget - many of these issues, including the original state tearing one - are theoretical. |
@MrWolfZ Where can I find a source code comment @markerikson noted? |
Thanks! Quoting it again.
|
I don't think I fully understand the comment. Maybe, we need to ask @markerikson, who's obviously busy... We can't conclude anything until we know what the final concurrent mode is like, but if the store state is updated within a single batchedUpdates process, it will break for sure. If that's the case, we would need to lock the store while running the batched Updates and put dispatches in a pending queue. |
We don't need to lock the store, but just keep the state for updates. Oh no, it doesn't work, because we don't know when exactly render functions are called. So, I think I now understand @markerikson 's comment. Unless batchedUpdates run render functions synchronously, reading from store.getState() is not safe. But, then, it can't be helped anyway until we can pass the entire state as a context. Or, if React provices a callback for batchedUpdates, that would help. |
Something like this: 1191f77 |
From the comment by @MrWolfZ : reduxjs/react-redux#1179 (comment)
That's what I thought. Not technically pure, but reproducible. So, flushing rendering results in concurrent mode is fine.
Note: WeakMaps are there for performance reasons. It works without them.
That's something I thought doubtful too. Let me learn about it.
The text was updated successfully, but these errors were encountered: