-
Notifications
You must be signed in to change notification settings - Fork 4.7k
Edit Site: Optimize loading useSelect call
#50546
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
Conversation
Co-authored-by: Jarda Snajdr <[email protected]>
|
Size Change: -4 B (0%) Total Size: 1.39 MB
ℹ️ View Unchanged
|
jsnajdr
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good 👍
If we wanted to optimize even more, we could note that even after loaded becomes true, we still keep the subscription to coreStore and execute the useSelect callback on each change, although we are guaranteed to never need it.
We could do this:
const [ loaded, setLoaded ] = useState( false );
const registry = useRegistry();
useEffect( () => {
if ( loaded ) {
return;
}
let timer;
let prevInPause = false;
const unsub = registry.subscribe( () => {
const nextInPause = ! registry.select( coreStore ).hasResolvingSelectors();
if ( ! prevInPause && nextInPause ) {
clearTimeout( timer );
timer = setTimeout( () => setLoaded( true ), 1000 );
}
prevInPause = nextInPause;
} );
return () => {
unsub();
clearTimeout( timer );
};
}, [ loaded ] );This way the subscription exists only during the time before loaded becomes true.
We don't have to do this now, but it will be a good idea when we discover we have hundreds of idle store subscriptions in the editor that only call dummy callbacks and do nothing. Then it's time to start refactoring to this pattern.
|
Thanks, @jsnajdr. I wonder if we should handle those at a framework level somehow; it feels like a burden if the |
There are ways how to abstract away most of the effect code. There is a proposal in #41169 for a |
What?
This PR is a follow-up to #50511 (comment) which optimizes the
useSelectcall of the site editor loading experience.Why?
Reduces the number of re-renders, as explained in the comment.
How?
We're only calling's
useSelectwhenloadedisfalse, once the site editor is marked as loaded,useSelectwill no longer trigger rerenders.Testing Instructions
Same as testing instructions of #50222.
Testing Instructions for Keyboard
None
Screenshots or screencast
None