Ensure that post thumbnail is cached in post template block.

[spacedmonkey]

What?

Ensure that post thumbnails are correctly primed when using the post template block.

Related to #40571.

Why?

If this function is not called, it results in one query to post meta table. Calling update_post_thumbnail_cache primes all caches in one.

How?

Testing Instructions

Screenshots or screencast

[ntsekouras]

Since a Query Loop block might not contain the Post Featured Image block, wouldn't this result in extra db queries in that case? Can we incorporate this in the Post Featured Image block somehow and have some performance improvements?

[spacedmonkey]

Fair point. I have moved the logic to the feature image block.

I tried to get the query as part of context, why I am not sure if this is even possible. My worry is the the feature image is used in a query block or other place, where the query is different than the main query on the page.

Any ideas @ntsekouras

[ntsekouras]

👋 Post Featured Image knows if is in a Query Loop if the queryId prop is filled from its context. An example would be here, but we would need to check for queryId. Noting that the queryId could be zero.

I'm not really familiar with this optimization, but would it make sense in the block itself or is it more about passing a query?

[spacedmonkey]

The function update_post_thumbnail_cache accepts the query object as a param. When it does, all thumbnail caches are primed in one request. It uses the WP_Query object to get the posts and then loop the thumbnails.

I think we should pass the query in here, as a feature image block could be used in different query loops.

[ntsekouras]

I took a closer look here and I think this could work well in Post Template block.

foreach ( $block->parsed_block['innerBlocks'] as $inner_block ) {
	if ( 'core/post-featured-image' === $inner_block['blockName'] ) {
		update_post_thumbnail_cache( $query );
		break;
	}
}

Can you think of any implications with this code @gziolo?

[gziolo]

@ntsekouras, that would be similar to what @spacedmonkey proposes in #40752 for the Navigation block.

Anyway, it largely depends on whether the Post Featured Image is the immediate child in the Post Template or it can be included at any level of nesting. I'm not sure whether $block->parsed_block['innerBlocks'] is a flat list of inner blocks, but it would have to be further investigated if you plan to go this route.

[ntsekouras]

Anyway, it largely depends on whether the Post Featured Image is the immediate child in the Post Template or it can be included at any level of nesting.

Oh.. that's true. It would make sense to have some metrics about this optimization whether it's better to recurse through innerBlocks to find if featured image exists in comparison to the two requests.

[spacedmonkey]

It would not be 2 requests, if you have 10 feature images on a page, it this could result in 10 times 2 ( one for post one for meta ) for each thumbnail. I have code in here to recursively loop through blocks to find nested navigation link blocks. I can confirm, that the inner is, is not a full list. The inner block, have inner blocks. So you have to recursively loop through blocks. It is a pain, but I have hidden the logic, so we could adapt it here.

The check here would be much simpler, as we just been a boolean of has feature image block.

@ntsekouras @gziolo Should I move the logic back to the post template block with a recursively lookup for featured-image blocks?

[spacedmonkey]

It is also worthing noting, I dont have an issue with having this function in both the feature image and post template block. I don't think it hurts having it in both places.

[ntsekouras]

@ntsekouras @gziolo Should I move the logic back to the post template block with a recursively lookup for featured-image blocks?

Yes, I think it's better to be there and pass the specific query. Also without some metrics I'm not even sure if it worths it to recurse through innerBlocks or just call the function 😄

[spacedmonkey]

@felixarntz @hellofromtonya Unit test added here too.

[spacedmonkey]

@draganescu This PR maybe helpful for #39658. We need to ensure that thumbnail is primed in cache so that there is a performance regression here.

[ntsekouras]

Besides the nit comments, this LGTM. Thanks @spacedmonkey !

[draganescu]

Excuse my WP cache noobness :D but I wonder what is the improvement we gain? Is this going to save a few db calls for a few editors open that happen to have blocks which query for featured image? If there is no persistence all different editors will benefit little. Same applies to the optimisation in #40752 for the Navigation block.

Again, I am probably not seeing the full picture. But just looking at the priming functions, in the absence of persistent cache, it saves querying in one request. Which is impactful on the front end, rendering classic PHP content. But on the backend does it improve anything significant?

How can we test there is any performance gain?

Later edit: right after submitting I realised I was missing the point :D these are rendering optimisations. In this case for the featured image, it's similar to how get_the_post_thumbnail primes the cache, so the same would need to happen to blocks that get the featured image.

However the cover block calls get_the_post_thumbnail_url which does not seem to internally call update_post_thumbnail_cache like get_the_post_thumbnail does. Why? And considering how featured image URL is only injected in the cover block do we need to prime a cache for that?

[spacedmonkey]

@draganescu I use a plugin called query monitor to check for the performance of the page. Here are the results.
Before

After

This is just on a page with 3 featured images. The improvement scales, do it primes all thumbnails at once. Priming them all at once, as a number of benefits, including few queries and better performance persistent object cache is enabled.

However the cover block calls get_the_post_thumbnail_url which does not seem to internally call update_post_thumbnail_cache like get_the_post_thumbnail does. Why?

See #40853 for more context. Calling this function multiple times is it not a problem, so I go out of my way to call it there as well. This PR pairs with #40853. But fixes have to done in both places, as cover blocks maybe calls outside of post template block context.

[spacedmonkey]

Besides the nit comments, this LGTM. Thanks @spacedmonkey !

@ntsekouras Feedback actioned.

[draganescu]

Let's go ahead and merge @spacedmonkey . With the change in the cover block and the potentially compounding improvement there is no downside only upside 👍🏻

[costdev]

Some notes on this:

1. The function is called block_core_post_template_uses_feature_image(). Shouldn't this be block_core_post_template_uses_featured_image()? This occurs on lines 15, 23 and 66.

---

2. The docblock description is inaccurate:

Loop through recursively to find blocks that use featured images.

This function tries to find one block that uses the featured image, then returns immediately.

Suggest this instead:

Determines whether a block list contains a block that uses the featured image.

---

3. The docblock for this function has no description for the @return value.

Suggest this:

@return bool Whether the block list contains a block that uses the featured image.

---

4. This conditional:

if ( 'core/cover' === $block->name && $block->attributes && isset( $block->attributes['useFeaturedImage'] ) && $block->attributes['useFeaturedImage'] ) {

can be shortened to:

if ( 'core/cover' === $block->name && ! empty( $block->attributes['useFeaturedImage'] ) ) {

[draganescu]

@costdev made a PR with your suggestions.

[adamziel]

I cherry-picked this to wp/6.0 branch to be included in WordPress 6.0 RC2 later today 482d1b1c54