-
Notifications
You must be signed in to change notification settings - Fork 4.7k
useBlockControlsFill: avoid unneeded store subscriptions #55340
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
|
Size Change: 0 B Total Size: 1.65 MB ℹ️ View Unchanged
|
tyxla
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.
Nice find 👍
It's a pretty straightforward change and I'm fairly confident it's safe.
While it doesn't bring amazing perf improvements, it's still a nice little one.
Thanks 🚀
tyxla
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.
Something that came up while discussing these kinds of optimizations is that it could be worth adding a comment to describe what we're doing. Otherwise, it could be very easy to lose this optimization in a refactor.
When a block renders a
<BlockControls>markup, its contents are rendered in the slot only if the block is selected. TheuseBlockControlsFillensures that, returning a non-null fill only when the block is selected.This PR optimizes the
useBlockControlsFillhook: ifshareWithChildBlocksis false, it can return early without creating any subscriptions to theblock-editorstore. And reducing number of subscriptions helps fix #54819.On a large post with 1000 blocks, I'm seeing (using the debug code from #55337) a reduction by 1500 subscriptions, from 77.5k to 76k.