Opened 6 years ago
Closed 10 months ago
#48175 closed feature request (wontfix)
Allow Template Resolution Algorithm to be Replaceable
| Reported by: |
|
Owned by: | |
|---|---|---|---|
| Milestone: | Priority: | normal | |
| Severity: | normal | Version: | |
| Component: | Themes | Keywords: | has-patch dev-feedback 2nd-opinion needs-unit-tests |
| Focuses: | template, performance | Cc: |
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)
Change History (8)
#2
in reply to:
↑ description
@
6 years ago
I forgot to mention. The included patch is completely backwards compatible.
#4
@
6 years 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
@
6 years 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
This ticket was mentioned in Slack in #core-performance by swissspidy. View the logs.
10 months ago
#7
@
10 months ago
- Milestone Awaiting Review deleted
- Resolution set to wontfix
- Status changed from new to closed
Given the lack of traction here, and after looking at the code base... I think if someone wants to completely override the template resolution with their own, the template_redirect action works just fine for this.
Initial patch.