-
Notifications
You must be signed in to change notification settings - Fork 4.2k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
FSE: Add template loader unit tests #31498
Conversation
Size Change: -10 B (0%) Total Size: 1.31 MB
ℹ️ View Unchanged
|
This comment has been minimized.
This comment has been minimized.
…over_equally_specific_parent_theme_php_template
19289a0
to
9761fd8
Compare
Okay, I think this should be ready for review now. |
The cases look good to me, I'll try to test this more thoroughly later. Thanks for doing this! |
Follow-up to simplify the template loader logic: #31604 |
Is this related? #31652 |
Somewhat, yeah. Thanks for notifying; I'll reply there. |
I'll go ahead and merge to unblock the two follow-up PRs, #31604 and #31671. #31671 has already been approved and contains a bugfix that I'd like to get in for WP 5.8 (i.e. GB 5.7 RC1). Furthermore, this PR only adds a (passing) unit test, so it's fairly low-risk; it just captures the behavior of the template loader and guards it against regressions. |
The template loader logic has grown a bit convoluted due to the recent addition of the concept of hybrid themes (i.e. both PHP and block templates allowed in one theme) and subsequent bug fixes (some of them related to child themes, others to custom page templates). #31498 had added unit test coverage to (hopefully!) cover exemplary cases from all those categories. This allows us to more confidently simplify (and later, if needed, further extend) the template loader algorithm. The core idea for simplifcation in this PR is as follows: `gutenberg_override_query_template( $template, $type, $templates)` is fed the results of Core's own template resolution algorithm. It provides a list of candidate `$templates` (e.g. `array( 'page-home.php', 'page-123.php', 'page.php' )`), and, if a matching template PHP file could be found in the current theme (including child and parent themes), that file's path in `$template` (e.g. `/var/www/html/wp-content/themes/test-theme/page-123.php`). This means when attempting to find a block template, we iterate over `$templates`, but we cut off at the entry corresponding to our fallback PHP template (in the example case, we only pass `array( 'page-home.php', 'page-123.php' )` to `gutenberg_resolve_template` (the function that retrieves block templates)), since we're only interested to find block template counterparts for those candidate slugs (which have higher or equal specificity as our fallback PHP template), but not in any matches with lower specificity (e.g. `/var/www/html/wp-content/themes/test-theme/block-templates/page.html`). The new logic is further explained in code comments.
Description
Follow up from #31399 (comment).
The block template resolution algorithm is quite complex, as it has to cover quite a lot of different possible cases resulting from various combinations of
Things become especially complex when there are e.g. "oldschool" PHP templates with higher specificity than the available block templates; or when a parent theme
For more discussion on this, see #31399. Examples of previous iterations that sought to refine the algorithms' behavior include #29026, #30599, #31123, and #31336. This PR's goal is to eventually cover all of those cases.
Once we have full coverage of those scenarios, we can consider to refactor the template loader, e.g. akin to https://github.com/WraithKenny/gutenberg/commit/8a70acd76f2c23fece9658166dc37e04ae71a05e.
How has this been tested?
Verify that CI goes green. Or locally:
cc/ @WraithKenny @vdwijngaert @MaggieCabrera @aristath