-
Notifications
You must be signed in to change notification settings - Fork 4.2k
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
Template part: avoid pattern fetch on mount #60297
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. |
templatePartId | ||
); | ||
const blockPatterns = useAlternativeBlockPatterns( area, clientId ); | ||
const hasReplacements = !! templateParts.length || !! blockPatterns.length; |
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.
I removed the templateParts
check because it doesn't seem relevant? These templates are not included in the list.
Size Change: +205 B (0%) Total Size: 1.72 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.
Thank you, @ellatrix! The changes work as expected.
Do you think it would be helpful to add an inline comment somewhere to avoid similar regression in the future?
Performance is clearly better:
|
@Mamaduka Any idea where to leave a comment? Not sure if it would help tbh, the edit function is big and you could add a regression in many places 🤔 |
Maybe near General question: Do you think having documentation about similar best practices should be helpful for contributors? |
@Mamaduka Yes, I think in general it could be said that you should always put logic and hooks (especially heavy hooks) as far down the tree as possible. This is especially good for components that render the children tree conditionally, such as: |
Even more generally: always return early. |
Thanks for fixing this. |
Co-authored-by: ellatrix <ellatrix@git.wordpress.org> Co-authored-by: Mamaduka <mamaduka@git.wordpress.org>
What?
This was accidentally re-introduced in #55128. See #57856 where I had also removed it for the replace button.
Why?
We shouldn't be fetching all patterns when mounting a template part. Only do it when the block is selected.
How?
Make sure the hooks are called in a subcomponent of
InspectorControls
, which only mounts children if the block is selected.Testing Instructions
See testing instructions of #55128.
Testing Instructions for Keyboard
Screenshots or screencast