Closed bergkvist closed 4 years ago
Sorry, not gonna happen any time soon. If React ever implements "context selectors", we'll investigate that and see if the performance issues have been resolved, but the implementations are far too different to try to maintain both of them (and especially when the context-based approach resulted in lousy performance).
See #1177 , as well as #1298 .
dai-shi seems to have been quite active in terms of working on userland implementations of context selectors for React. Would it be a bad idea to test out/rely on his use-context-selector implementation?
The benchmarks seem to indicate that performance is the same as react-redux v7.
In the examples below, I try to have a table component that is expensive to render depend on state dispatched from a different (draggable) component (panel-layout):
My hypothesis was that react-redux would handle this better because it doesn't force everything to rerender at the same time - meaning you might have a more smooth dragging experience.
If I'm going to be honest I don't really notice a significant difference in this example.
We're probably going to have to do a React-Redux v8 once useMutableSource
is finalized. I don't know where the official context selectors implementation is on the React team's timeline, but it sounds like it'd be lucky to have that this year at best. Whenever that does come out, we'll certainly look at it.
I don't intend to do any other re-architecting beyond that.
I get that this is an issue for you, but you're basically asking us to gut the internals, again, for a very niche use case. If you or someone else would like to take the time and effort to put together a notional fork that actually implements the approach you're describing, I'd be willing to take a look at it. But, that's not something we have the time or interest in doing ourselves.
@bergkvist While I don't read the original issue, here's my two cents.
use-context-selector v1 (as well as reactive-react-redux v4) uses undocumented changedBits=0
behavior. It would have been wonderful if we found this hack while we are still at react-redux v6.
However, react-redux v7 is already here which is totally valid in legacy mode. For concurrent mode, we are expecting useMutableSource
to land in the near future. This is for external store like redux. It's likely that react-redux follows this pattern.
That said, I personally even think context is not a primary building block if we go with useMutableSource
. My proposal in reactive-react-redux (not react-redux) is not to use context at all: https://github.com/dai-shi/reactive-react-redux/pull/48
Turns out my original assumption here was wrong. The deck.gl-flickering issue was caused by a race condition that was fixed in react-redux v7.2.1. This also means I no longer have any desire/need for a React Context-based core in react-redux.
Motivation/problem
When using react-redux 7, I experience a race condition when using deck.gl.
What most likely causes the issue is that react-redux doesn't prevent unrelated code from running in between a dispatch and a rerender. (in contrast to built in state management in React).
My gut tells me this might also be why react-redux feels "smoother" than React Context (which I suppose is what people complained about with react-redux v6).
To see the issue, click the world map and press the plus (+) key to start a zoom transition in the two examples below. In the react-redux example - the map will flicker as the zoom transition starts.
Hotswapping React Context and Redux
From what I understand, react-redux v6 used React Context under the hood - but react-redux v7 switched back to using Redux under the hood due to performance issues with React Context.
Since react-redux v6 does not provide useSelector and useDispatch hooks - I can't just switch back to the older version.
It would be nice if I could somehow hotswap the "under the hood" implementation of react-redux (between React Context and Redux).
Even more interesting would be to mix redux and context for different slices of state. (although I realize this is kind of crazy)