Template part: avoid pattern fetch on mount #60297
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
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. |
ellatrix
commented
Mar 29, 2024
| templatePartId | ||
| ); | ||
| const blockPatterns = useAlternativeBlockPatterns( area, clientId ); | ||
| const hasReplacements = !! templateParts.length || !! blockPatterns.length; |
There was a problem hiding this comment.
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
|
|
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. |
cbravobernal
pushed a commit
to garridinsi/gutenberg
that referenced
this pull request
Apr 9, 2024
Co-authored-by: ellatrix <[email protected]> Co-authored-by: Mamaduka <[email protected]>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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