-
-
Notifications
You must be signed in to change notification settings - Fork 3.4k
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
Investigate adding a proxy-based selector API #1502
Comments
One of the biggest concerns was about performance because Proxies have some overhead. |
As for CM, there's some relevance because reactive-react-redux tries it. |
Can we just adopt useTrackedState with some minor changes? @dai-shi Would you be OK with that? |
You mean like react-redux v7.2? Yes, that should be possible. I'm certainly OK with it. (Well, I'll draft a PR anyway.) |
hi! @dai-shi i would like to get a point:
in summary, we are replacing if (equalityFn(newSelectedState, latestSelectedState.current)) {
return
} with if (
latestTracked.current.state !== nextState &&
isDeepChanged(
latestTracked.current.state,
nextState,
latestTracked.current.affected,
latestTracked.current.cache
)
) {
forceRender()
} the main drawback is that user cannot use selectors, and so derivated states. am i missing something? |
Yeah, like I said in the PR:
In general, I see it as a mostly simpler, higher-level abstraction alternative to The main points of concern are the caveats listed at https://github.com/dai-shi/reactive-react-redux#caveats . |
yes i agree. But the most drawback i read there is 'Proxied state shouldn't be used outside of render' because Nowdays, people think that Redux is complicated, so the main goal is "keep it simple!" 🎉 |
FWIW, we decided against the |
Hmm. Another good question is, what do the TS typings for this look like? |
useTrackedState<TState>() =>TState.? |
For beginners: Far easier to understand Redux and R-R without the notion of selectors For intermediates: Never needs to worry about memoized selectors For experts: No stale props issue
Correct. It can't provide the same behavior as this: const isYoung = useSelector(state => state.person.age < 12); But, that doesn't mean users cannot use selectors. They can use them, but don't get the same behavior. (extra re-renders) The other way of saying would be selectors in R-R have two purposes: separation of concerns and render optimization. You could create useSelector from useTrackedState. const useTrackedSelector = (selector) => selector(useTrackedState());
Honestly, I don't know what will happen if we leak Proxies outside render. Technically, only the problem from the useTrackedState side is Proxying already Proxied objects.
Well, believe it or not, that's my major motivation for |
Ideally |
I'm wondering if this would mean dropping IE11 support 🙈 (no Proxies on IE11) or how the fallback API may look like 🤔 |
There was a debate before, and proxy-polyfill is not included now. So, it's up to developers to add proxy-polyfill for IE11 support and RN support. There's also caveats when using the polyfill. (See dai-shi/reactive-react-redux#30 ) |
You can also use useSelector just the same as before, which doesn't rely on Proxies to work. This API is entirely additive, so it will show up in a minor release. |
Does anyone know if it would be possible to achieve something similar with babel macros? const state = useSelector(state => state)
return <div>state.foo</div> rewrite it with babel const state_foo = useSelector(state => state.foo)
return <div>state_foo</div> it would have zero runtime cost. |
I have thought about it before too. The build time approach would be interesting. Let me point out one difference with the runtime approach. const fooOrBar = useSelector(state => isFoo ? state.foo : state.bar); // isFoo comes from props This simple case might be possible in the build time, but more complex case would be difficult. |
Hiya, folks. At this point, I don't think we're likely to add a specific new proxy-based selector API to React-Redux. We do have a mention of I'm going to go ahead and close this. |
In case someone misses it, here's my final comment in the PR: #1503 (comment) |
Both our
connect
anduseSelector
APIs rely on the user manually specifying what pieces of state to extract to determine re-renders.@dai-shi has done some amazing work with using ES6 Proxies to manage tracking the pieces of state that are actually needed by a component, particularly in his https://github.com/dai-shi/reactive-react-redux library.
I've said several times that I kinda see that lib as a sort of alpha test for whether an approach like that should be added to React-Redux itself. Now that our hooks API has been stable for a while, it might be time to start discussing that idea more seriously.
This may also have some relevance to the Concurrent Mode questions over in #1351 .
Ideally, anything we add would be completely tree-shakeable by default, so that it doesn't increase bundle size unless the API is actually used.
The text was updated successfully, but these errors were encountered: