-
Notifications
You must be signed in to change notification settings - Fork 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
Data: Add useSelector hook #21057
Data: Add useSelector hook #21057
Conversation
Size Change: -16 B (0%) Total Size: 856 kB
ℹ️ View Unchanged
|
Tangentially, in thinking about this, I've been contemplating whether it'd actually be difficult to implement in Pseudo-code: function useSelect( mapSelect ) {
const stores = useRef( new Set );
const registry = useRegistry();
const [ mapOutput, setMapOutput ] = useState( () => mapSelect( ( storeKey ) => {
stores.current.add( storeKey );
return registry.select( storeKey );
} );
useLayoutEffect( () => {
function onStoreChange() { /* ... */ }
onStoreChange();
registry.subscribe( Array.from( stores.current ), onStoreChange );
} );
return mapOutput;
} |
I had started to consider another alternate take on this signature before its fatal flaw eventually revealed itself to me. The general idea was to use property descriptors (getters) as a flagging mechanism in the destructuring of a function MyComponent() {
const { getCurrentPostType } = useSelector( 'core/editor' );
// ...
} ...the mere act of destructuring the result of the hook would be enough to know that this component has a dependency on the result of Once it can be known which selectors a component depends upon, it enables a few key optimizations:
There is a lot of similarity between this and...
Now, the fatal flaw: Selector arguments. Just as I eventually discovered with #6458, this only works if a selector receives no arguments. If a selector is expected to receive arguments, the store can't know how to re-evaluate the result, particularly if that selector may only be called later in the component. I'm still intrigued by the idea, and there may be some elements of this which could be leveraged into something more viable. Hence sharing for posterity's sake, despite not being feasible. |
This was intended as a prototype. I think the API interface is definitely more pleasant to work with than the current implementation. That said, there are some very concerning implications of this sort of change which need careful consideration and consensus. It may be good to revisit or at least reference this in the future for prior art. |
Related prior art: #12877 |
Related Slack discussion (link requires registration): https://wordpress.slack.com/archives/C5UNMSU4R/p1584734770269400
This pull request seeks to try an alternative function signature to
useSelect
.Where what was previously expressed as...
...could instead be expressed as:
In its current state, it's intended to be explored as a prototype. Some observations:
+26 -72
revision.selector
functions are removed.useSelect
callback using same names as that of their parent scope.&&
and||
conditions [1] [2] [3] [4] [5]useSelect
, we're never performing a shallow comparison.useSelector
would not only needlessly re-render itself, but also all of its descendents... every time state changes.Note: Strictly speaking, this could be implemented as an overloaded form of
useSelect
. I don't hold strong feelings about this, but implemented it as a separateuseSelector
for the proposal due to:useSelect
must make up-front decision on how to handle logic of its hookuseSelect
, as indicated by its name, provides a callback with theselect
function. By contrast,useSelector
implies some intention to gain access to selectors specifically. More accurately, it could beuseSelectors
, since it returns an object of a store's selectors. To me, it feels more readable in the singular form.