Block template is located twice in `get_query_template()`

[dlh]

get_query_template() currently calls locate_block_template() twice instead of once when a template is rendered, leading to a second call to resolve_block_template(), get_block_templates(), and so on.

The first call is hardcoded directly after locate_template() here: ​https://github.com/WordPress/wordpress-develop/blob/c40e0c/src/wp-includes/template.php#L66

This call was added in [51003], the original commit to add support for block templates to classic themes.

The second call occurs during the {$type}_template filter here: ​https://github.com/WordPress/wordpress-develop/blob/c40e0c/src/wp-includes/block-template.php#L25

This second call was added in [52316] for #54553. However, as far as I can tell, the only thing that was needed to fix the issue in #54553 was the callback added to pre_get_posts.

Adding locate_block_template() as a filter on {$type}_template would have been necessary in the Gutenberg plugin, but not in core, given the call that was added directly to get_query_template().

You can test this using the linked PR, which would remove the {$type}_template filters and leave just the pre_get_posts addition. With that PR applied, the verification steps in ticket:54553#comment:7 should still pass.

Trac ticket: https://core.trac.wordpress.org/ticket/58299

[spacedmonkey]

Some promising early numbers here.

	Trunk	PR
Response Time (median)	121.83	114.41
wp-before-template (median)	65.38	59.6
wp-template (median)	50.68	48.99
wp-total (median)	116.48	108.98

[spacedmonkey]

Wonder what @oandregal thinks about this one.

I ran a profiling test against the current trunk, and it showed that removing some function calls improved performance. This might be of interest to @felixarntz, @spacedmonkey, @eclarke1, and @joemcgill.

@ockham Do you mind providing some context why this function needs to called twice?

[spacedmonkey]

@noisysocks @bernhard-reiter Can you provide some context here. Why this function is called multiple times. Does this change look good? How can we test it?

[Bernhard Reiter]

Replying to dlh:

Adding locate_block_template() as a filter on {$type}_template would have been necessary in the Gutenberg plugin, but not in core, given the call that was added directly to get_query_template().

This seems like a plausible explanation; we might've overlooked the filters when backporting the code from Gutenberg to Core.

I'll try and test this in a bit; I'd say that most importantly, we'll need to verify that the correct template is used when rendering a given page on the frontend, and that the site editor also continues to work as before (with a special focus on what templates are displayed as available, and that the correct template is loaded and also saved when making changes).

[spacedmonkey]

Thanks @Bernhard-Reiter. For now, I am going mark you as owner. After your testing and you say you are happy, then I will take over and hopefully commit.

I looked around a bit, and this looks indeed like an oversight. As I said in this comment on the corresponding Trac ticket:

Adding locate_block_template() as a filter on {$type}_template would have been necessary in the Gutenberg plugin, but not in core, given the call that was added directly to get_query_template().

This seems like a plausible explanation; we might've overlooked the filters when backporting the code from Gutenberg to Core.

To unfurl the call stack a bit: The code that this PR removes loops over all template types and ​adds `locate_block_template` as a filter to all their corresponding `{$type}_template` hooks (e.g. home_template, single_template, etc). The ​`{$type}_template` filters are only being applied in one spot: ​https://github.com/WordPress/wordpress-develop/blob/88de9a9d8efe13a21ebdcdfbf6f87d78875c8f0e/src/wp-includes/template.php#L103

The LOC _directly before that line_ (and its comment block) is the "other" call to locate_block_template(), passing the same arguments: ​https://github.com/WordPress/wordpress-develop/blob/88de9a9d8efe13a21ebdcdfbf6f87d78875c8f0e/src/wp-includes/template.php#L66

So it looks pretty safe indeed to remove the filters from the hook! I've done a bit of smoke testing, and things seem to continue to work fine.

Apologies, @felixarntz, but could you elaborate about the template-loading outcome(s) that should be tested?

@dlh01 Fair question. Taking another look, this change is probably irrelevant to any testing.

However, as far as I can tell there are no tests at all for the block template specific bits of the get_query_template() function. This is not strictly responsibility of a PR like this, but those should have been added whenever the locate_block_template() call was added to get_query_template().

To unblock this PR, I'm happy to approve this, but it would be great if you could use a separate PR to add at least some basic test coverage to ensure that get_query_template() returns the expected result for some block templates use-cases, as otherwise we can't really make such changes safely, without test coverage.

Sure thing! I'll open a new ticket when I have those tests ready.

[spacedmonkey]

In 56060:

Themes: Block template is located twice in get_query_template().

When the function _template_loader_filters was ported to core from gutenberg, it retained the filter to load block templates. However, the function locate_block_template is called manually in get_query_template, so this filter is not needed. Calling locate_block_template twice, results in performance issue, as locate_block_template is a expensive function to run, as it does database and file lookups.

Props dlh, mukesh27, flixos90, SergeyBiryukov, bernhard-reiter, spacedmonkey.
Fixes #58299.

Committed ​https://github.com/WordPress/wordpress-develop/commit/02ea2f1d75a77d014708fd0d38677e85065742e9