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
Editor: Memoize 'getInsertionPoint' selector #60015
Conversation
The following accounts have interacted with this PR and/or linked issues. I will continue to update these lists as activity occurs. You can also manually ask me to refresh this list by adding the If you're merging code through a pull request on GitHub, copy and paste the following into the bottom of the merge commit message.
To understand the WordPress project's expectations around crediting contributors, please review the Contributor Attribution page in the Core Handbook. |
Size Change: +26 B (0%) Total Size: 1.71 MB
ℹ️ View Unchanged
|
|
||
return EMPTY_INSERTION_POINT; | ||
}, | ||
( state ) => { | ||
const [ postContentClientId ] = | ||
select( blockEditorStore ).getBlocksByName( | ||
'core/post-content' |
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.
Worth noting that this can lead to deoptimization: as long as there is a valid state.blockInserterPanel
, the original selector never selected from blockEditorStore
and didn't create a subcription to it. But the memoized version does always does the selection when calculation dependants.
There's probably no way around this other than abandoning the getInsertionPoint
selector and inlining the code into the only place where it's used.
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.
Thanks for the feedback, @jsnajdr! We can't remove getInsertionPoint
because of BC. We can stop using it in the core, but that just avoids the issue.
I'm planning to merge this. If we start neglecting the |
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.
Let's ship
Co-authored-by: Mamaduka <mamaduka@git.wordpress.org> Co-authored-by: jsnajdr <jsnajdr@git.wordpress.org>
What?
PR memoized editor's
getInsertionPoint
registry selector. Which is now possible thanks to the #57888.Why?
This prevents the selector from returning different values when called with the same state.
Testing Instructions
useSelect
doesn't log warnings.Testing Instructions for Keyboard
Same.
Screenshots or screencast