-
Notifications
You must be signed in to change notification settings - Fork 4.3k
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
Block editor: prevent isSubtreeDisabled call if not needed #58349
Conversation
Size Change: +7 B (0%) Total Size: 1.7 MB
ℹ️ View Unchanged
|
isSubtreeDisabled: isBlockSubtreeDisabled( clientId ), | ||
isSubtreeDisabled: | ||
blockEditingMode === 'disabled' && | ||
isBlockSubtreeDisabled( clientId ), |
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.
Prevents isBlockSubtreeDisabled
from calling getBlockEditingMode
again.
getBlockEditingMode( state, clientId ) === 'disabled' && | ||
getBlockOrder( state, clientId ).every( isChildSubtreeDisabled ) | ||
); | ||
return getBlockOrder( state, clientId ).every( isChildSubtreeDisabled ); |
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.
Note that this is a private selector, so we can change it. There's also only one used case which is the above.
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.
This only affects the initial calculation right? Because createSelector
should cache subsequent calls?
At any rate, LGTM 👍 Thank you!
It gets cached, but by clientID, so it's only useful after page load I guess. Also, the problem seems to be more getBlockEditingMode and calling that multiple times. Thanks @noisysocks! |
In fact, the cache may actually be a problem at load because a lookup becomes a loop over all blocks, see #58355 |
What?
See #58315, calling this selector seems to be responsible for a good chunk of work done on load, especially in the site editor. This is because getBlockEditingMode recursively checks parent blocks. Let's replace three of these calls with one.
Why?
Site editor load performance: -5% (1st run), -3.75% (2nd run), -3.2% (3rd run)
How?
Testing Instructions
Testing Instructions for Keyboard
Screenshots or screencast