Revert/add 43862 dynamic template names descriptions #44645
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
cbravobernal
requested review from
dream-encode,
jorgefilipecosta,
ockham and
priethor
|
Size Change: -397 B (0%) Total Size: 1.26 MB
ℹ️ View Unchanged
|
|
@c4rl0sbr4v0 I reverted the revert of https://core.trac.wordpress.org/changeset/54333 in https://core.trac.wordpress.org/changeset/54370. The JS side of this is in the GB packages, and will need another build sync for 6.1 Beta 3 tomorrow. |
cbravobernal
added
the
Backport to WP 6.7 Beta/RC
|
This morning I was looking through #43862 and forgot to add the comment 🤦 I've some concerns about how this change might affect the Based on my simple tests, I see increased query numbers when running the Testing instructions
Simple method to test query numbersadd_action( 'init', function() {
global $wpdb;
// Reset query count.
$wpdb->num_queries = 0;
gutenberg_get_block_templates();
error_log( "Queries: $wpdb->num_queries" );
} ); |
cbravobernal
removed
the
Backport to WP 6.7 Beta/RC
|
I'm removing the backport label to prevent the automation to cherry pick this today. |
|
cc/ @jorgefilipecosta @priethor Can I ask y'all to have a look into those potential performance issues that @Mamaduka found? |
I think it is expected that if we want the template titles to reflect the real titles of a post, author or taxonomy, we will need queries to retrieve these titles from the database and in the end there is no way around that. To reflect the more updated information and react to changes on the base entity we need to query the entities. So having more queries in that function is something expected. What we can do is thing like adding a parameter to gutenberg_get_block_templates where we can disable title computation if the caller of the function does not requires it (e.g: just wants to render a template), or see if get_user_by is more performant than get_user as @Mamaduka suggested. I guess we can do these optimizations on the next release if we see there is an impact we can minimize. By default on most websites (where no entity specific templates exist) there is no change on the number of queries at all. When the user creates entity specific templates we have some additional queries to compute the names on the next release according to the feedback we receive we can see if we can find some improvements. |
|
Thank you for addressing those concerns @jorgefilipecosta! |
|
I'll add back the "Backport to WP Beta/RC" label. Since I've already published npms for Beta 3 (which is due in about 1.5 hrs), I'd rather not rush it in. Instead, we'll include it in RC1 next week. (Note that the PHP code has already been backported -- which shouldn't be a problem.) |
spacedmonkey
reviewed
Oct 4, 2022
| function _gutenberg_build_title_and_description_for_single_post_type_block_template( $post_type, $slug, WP_Block_Template $template ) { | ||
| $post_type_object = get_post_type_object( $post_type ); | ||
|
|
||
| $posts = get_posts( |
spacedmonkey
reviewed
Oct 4, 2022
| $posts = get_posts( | ||
| array( | ||
| 'name' => $slug, | ||
| 'post_type' => $post_type, |
There was a problem hiding this comment.
Also do not prime term and meta caches, if not used.
spacedmonkey
reviewed
Oct 4, 2022
| array( | ||
| 'taxonomy' => $taxonomy, | ||
| 'hide_empty' => false, | ||
| 'slug' => $slug, |
There was a problem hiding this comment.
Limit to 1.
Do not prime term meta caches.
ockham
added
the
Backport to WP 6.7 Beta/RC
|
Per discussion here, I'll merge this and will re-publish the npm package. |
ockham
deleted the
revert/add-43862-dynamic-template-names-descriptions
branch
|
@spacedmonkey's suggested PHP changes are being addressed in WordPress/wordpress-develop#3404. |
ockham
removed
the
Backport to WP 6.7 Beta/RC
This was referenced Oct 4, 2022
|
This was included in WP 6.1 Beta 3, see WordPress/wordpress-develop#3405. |
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?
#43862 will finally be landed in 6.1. This PR adds the review comments done in the PR by @dream-encode