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
Footnotes: combine format store subscription #58129
Conversation
Size Change: +10 B (0%) Total Size: 1.7 MB
ℹ️ View Unchanged
|
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.
Makes sense to me. I hope it performs better.
@mcsf honestly not sure. The typing metric has also magically come down a bit over the last few commits, so maybe it was a fluke. But since we already have this we might as well do it. |
Yeah, but even just conceptually this is sounder, so I'm glad. |
What?
I'm not sure if this is the cause of the small recent typing performance regression, but it's worth optimising anyway I think.
Instead of using
useEntityProp
, we can directly use thegetEntityRecord
. Under the hook,useEntityProp
is a bit heavier because it also checks thegetEditedEntityRecord
selector. Additionally it will trigger component re-renders when any meta value changes. We only need the unedited entity record here to quickly check if footnotes is supported. The type of footnotes meta shouldn't change.Additionally we can combine all
useSelect
calls into a single one that returns a boolean value, so the component will only re-render when this value changes.Why?
How?
Testing Instructions
Testing Instructions for Keyboard
Screenshots or screencast