WordPress.org

Make WordPress Core

Opened 3 weeks ago

Last modified 2 weeks ago

#48175 new feature request

Allow Template Resolution Algorithm to be Replaceable

Reported by: dhurlburtusa Owned by:
Milestone: Awaiting Review Priority: normal
Severity: normal Version:
Component: Themes Keywords: has-patch dev-feedback 2nd-opinion needs-unit-tests
Focuses: template, performance Cc:
PR Number:

Description

While I was writing a custom theme that doesn't use the results of the template hierarchy algorithm (THA), I noticed that a lot of work is done for nothing. That is, ultimately, several calls to PHP's file_exists is made inside locate_template. But I have a template_include filter that ignores the result of the template hierarchy algorithm.

I was checking out the performance of the THA and found out that the calls to file_exists are relatively time intense. I wrapped some calls to microtime around the THA and found out that it takes about 1100 μs to run on my laptop with an SSD.

So, I investigated how I could minimize the time in the THA knowing that I ultimately don't need the result. I discovered the {$type}_template_hierarchy and {$type}_template filter hooks. I used the {$type}_template_hierarchy filter to return an empty array which causes all the file_exists calls to be avoided. I also used a {$type}_template filter to quickly short circuit the THA. This allowed the execution time of the THA to go to about 35 μs. Much improved!

Then I thought if there was a quicker way this could be done. So, I tried adding a filter called skip_template_hierarchy that returns false by default. When true is returned, then the entire THA is skipped. Therefore, false is passed to the template_include filter. In this filter is where I return the template based on my theme's template resolution algorithm. This made this section of code execute in about 5 μs. Even better.

Then I thought whether there is a better way to do this, and I thought "what if we can make the template resolution algorithm (TRA) replaceable?" By default, the current THA would be used as the TRA. But a filter could be put into place that lets the user/developer choose a different TRA.

So, I am writing a patch to add this functionality/feature. I am planning to name the filter template_resolution_algorithm. When the filter is applied, it will get 'resolve_template_hierarchy' which will be a reference to a new function of the same name to be placed in wp-includes/template.php. This function is the default template resolution algorithm which of course resolves by using the THA we've all known for the longest time.

After implementing this, I was surprised to see that it actually took about 6-7 μs to run. Not quite as fast as just skipping the THA with the skip_template_hierarchy.

Example

So, now I can use a custom template resolution algorithm like so:

// Add the following to the theme's function.php

add_filter( 'resolve_template_hierarchy', function () {
	return function () {
		$template = TEMPLATEPATH . '/templates/index.php';

		return $template;
	};
} );

The following works too:

function dh__resolve_theme_template () {
	$template = TEMPLATEPATH . '/templates/index.php';

	return $template;
}

add_filter( 'template_resolution_algorithm', function () {
	return 'dh__resolve_theme_template';
} );

Benefits

Obviously this can have performance benefits by being able to choose a different algorithm instead of running the THA and then ignoring its results. This would be ideal for implementing themes as single-page applications with minimal use of WP hooks. It would be useful if building a theme with an MVC-like architecture that utilizes a router mechanism.

Feedback

I'll upload my patch really soon. In the meantime, I am hoping I can get some feedback.

Please note that I've only been doing PHP development for about 3 years off and on. Also, I've only been working with WP for a little over two years but only off and on. So, I can use some help making sure I am following PHP and WP conventions/idioms.

But, I've been doing web development for 14 plus years. So, I am not a complete noob.

Testing

I can also use some guidance on where to add the unit tests for this particular feature. But before I write the tests, I'd like to get feedback first just in case this is something not even desired or maybe we come up with a better solution.

I do see that there is a tests/phpunit/tests/template.php file to put wp-includes/template.php related tests in.

Could someone who is familiar with the code base (core committers) point me at a good example of writing tests on filter hooks?

I also need a little help with getting the unit tests to run. I've read the instructions at https://make.wordpress.org/core/handbook/testing/automated-testing/phpunit/ but they seem to be outdated, along with tests/phpunit/README.md. I tried the instructions in the project's README.md. They seem to work.

The unit tests pass for commit 47643f7621..., but there are 6 new commits. The tests don't pass with these 6 new commits. I didn't try to narrow down which one is causing the error. Here are there errors:

There were 12 errors:

1) Tests_Rel_Ugc::test_add_ugc
Error: Call to undefined function wp_rel_ugc()

/var/www/tests/phpunit/tests/formatting/WPRelUgc.php:14

2) Tests_Rel_Ugc::test_convert_ugc
Error: Call to undefined function wp_rel_ugc()

/var/www/tests/phpunit/tests/formatting/WPRelUgc.php:23

3) Tests_Rel_Ugc::test_wp_rel_ugc with data set #0 ('<a href="">Double Quotes</a>', '<a href="" rel="nofollow ugc"...es</a>')
Error: Call to undefined function wp_rel_ugc()

/var/www/tests/phpunit/tests/formatting/WPRelUgc.php:31

4) Tests_Rel_Ugc::test_wp_rel_ugc with data set #1 ('<a href="https://wordpress.or...es</a>', '<a href="https://wordpress.or...es</a>')
Error: Call to undefined function wp_rel_ugc()

/var/www/tests/phpunit/tests/formatting/WPRelUgc.php:31

5) Tests_Rel_Ugc::test_wp_rel_ugc with data set #2 ('<a href='https://wordpress.or...es</a>', '<a href='https://wordpress.or...es</a>')
Error: Call to undefined function wp_rel_ugc()

/var/www/tests/phpunit/tests/formatting/WPRelUgc.php:31

6) Tests_Rel_Ugc::test_wp_rel_ugc with data set #3 ('<a href="https://wordpress.or...es</a>', '<a href="https://wordpress.or...es</a>')
Error: Call to undefined function wp_rel_ugc()

/var/www/tests/phpunit/tests/formatting/WPRelUgc.php:31

7) Tests_Rel_Ugc::test_wp_rel_ugc with data set #4 ('<a title="Title" href="https:...es</a>', '<a title="Title" href="https:...es</a>')
Error: Call to undefined function wp_rel_ugc()

/var/www/tests/phpunit/tests/formatting/WPRelUgc.php:31

8) Tests_Rel_Ugc::test_wp_rel_ugc with data set #5 ('<a data-someflag href="https:...es</a>', '<a data-someflag href="https:...es</a>')
Error: Call to undefined function wp_rel_ugc()

/var/www/tests/phpunit/tests/formatting/WPRelUgc.php:31

9) Tests_Rel_Ugc::test_wp_rel_ugc with data set #6 ('<a  data-someflag  title="Tit...ce</a>', '<a  data-someflag  title="Tit...ce</a>')
Error: Call to undefined function wp_rel_ugc()

/var/www/tests/phpunit/tests/formatting/WPRelUgc.php:31

10) Tests_Rel_Ugc::test_wp_rel_ugc with data set #7 ('<a href="http://example.org/s...p)</a>', '<a href="http://example.org/s...p)</a>')
Error: Call to undefined function wp_rel_ugc()

/var/www/tests/phpunit/tests/formatting/WPRelUgc.php:31

11) Tests_Rel_Ugc::test_wp_rel_ugc with data set #8 ('<a href="https://example.org/...s)</a>', '<a href="https://example.org/...s)</a>')
Error: Call to undefined function wp_rel_ugc()

/var/www/tests/phpunit/tests/formatting/WPRelUgc.php:31

12) Tests_Rel_Ugc::test_append_ugc_with_valueless_attribute
Error: Call to undefined function wp_rel_ugc()

/var/www/tests/phpunit/tests/formatting/WPRelUgc.php:81

ERRORS!
Tests: 9960, Assertions: 44191, Errors: 12, Skipped: 11.

Patch

Should I include changes to the package-lock.json even though I made no changes to the dependencies in package.json?

Attachments (1)

48175-allow-tra-to-be-replaceable.diff (3.8 KB) - added by dhurlburtusa 3 weeks ago.
Initial patch.

Download all attachments as: .zip

Change History (6)

#1 @davidbaumwald
3 weeks ago

  • Focuses template performance added

#2 in reply to: ↑ description @dhurlburtusa
3 weeks ago

I forgot to mention. The included patch is completely backwards compatible.

#3 @SergeyBiryukov
3 weeks ago

Hi there, welcome to WordPress Trac! Thanks for the ticket and the patch.

Just noting that similar approaches were previously suggested in #43597 and #16128.

#4 @dhurlburtusa
2 weeks ago

What's the possibility of getting this in WP 5.3? Is there anything else I can do to help with that effort?

#5 @apedog
2 weeks ago

+1 to this (or the solution in #43597)
I think this ticket can get some attention alongside #41362 and https://github.com/WordPress/gutenberg/issues/17512

Note: See TracTickets for help on using tickets.