#54541 closed enhancement (fixed)
Add actions to template loading to assist with collecting debug info
Reported by: | rmccue | Owned by: | johnbillion |
---|---|---|---|
Milestone: | 6.1 | Priority: | normal |
Severity: | normal | Version: | |
Component: | Themes | Keywords: | needs-dev-note has-patch commit |
Focuses: | performance | Cc: |
Description
locate_template()
is the low-level workhorse that handles finding template parts; with the second parameter ($load = true
), it also handles actually loading those templates. This is used throughout the template hierarchy loading system; get_template_part()
calls it, as well as get_header()
, get_footer()
, etc.
In order to provide timing data to debug tools like Query Monitor, having actions before and after the template itself loads would be super useful. We could do this within each usage of locate_template()
, or inside it.
Potentially, this could be a shortcircuit filter, allowing for alternative loading patterns (#13239) or automatic fragment caching. However I don't want to derail this ticket, as there are a lot of strong opinions on the template hierarchy, so I would suggest we do not provide a filter at this stage; right now, just having the timing data would be useful.
(I offer no opinions at this time on the relation to the Gutenberg FSE templating.)
Change History (20)
This ticket was mentioned in PR #2462 on WordPress/wordpress-develop by Tabrisrp.
3 years ago
#1
- Keywords has-patch added; needs-patch removed
#2
@
3 years ago
- Milestone changed from Awaiting Review to 6.0
- Owner set to johnbillion
- Status changed from new to reviewing
- Type changed from defect (bug) to enhancement
#3
@
2 years ago
- Keywords needs-unit-tests needs-dev-note added
- Milestone changed from 6.0 to 6.1
With 6.0 Beta 1 released on Tuesday, we are now past Feature Freeze for the 6.0 cycle. Moving to the 6.1 milestone.
- This ticket should have unit tests accompanying it. Adding
needs-unit-tests
. - This ticket should be included in a dev note to alert extenders to the new action hooks. Adding
needs-dev-note
.
martinkrcho commented on PR #2462:
2 years ago
#4
@Tabrisrp , would you be able to apply the changes suggested by @peterwilsoncc? Then we could get @johnbillion to review and I would be more than happy to contribute with some unit tests.
#6
@
2 years ago
- Keywords commit added
Marking the linked pull request as ready for commit as of 5bc0c5a.
johnbillion commented on PR #2462:
2 years ago
#8
Committed in https://core.trac.wordpress.org/changeset/53560
#9
@
2 years ago
- Resolution fixed deleted
- Status changed from closed to reopened
If I understand the intentions of this ticket and its hooks correctly, locate_template()
may be the wrong function location for these hooks, or these hooks would perhaps benefit from a slightly different naming convention.
Is it possible that load_template()
is where these exact hooks belong (with the $template_names
param removed)? Obviously, that is the function responsible for requiring the located file with the intended variables available to it, and the measurable code execution for Query Monitor is the extraction of the query_vars
and the rendering/inclusion of the template file.
In my research & experience, load_template()
is used by many of the most popular WordPress plugins – not locate_template()
– which means their template parts will never see these hooks fire and be unable to benefit from any future fragment caching implementation that may want to use them.
Scopecreep
The Feeds feature in WordPress also uses load_template()
via the do_feed()
function (through do_action( "do_feed_{$feed}" )
) which kind of seems to me like it should have its own special case inside of locate_template()
the way that theme-compat
does so that Feeds can use locate_template()
while also enabling Query Monitor to use the same hooks.
Plugins
Mostly, the plugins below use load_template()
to get around the fact that locate_template()
is strictly locked down to the parent/child theme hierarchy, so they write their own "locators" with extra paths to add to the core hierarchy under certain controlled conditions.
- bbPress & BuddyPress for theme compatibility
- EDD for loading template parts
- WooCommerce for loading template parts
- WPForms for loading template parts
- Events Calendar for loading template parts
- Jetpack for loading
inline-help-template.php
- A bunch more... 😅
Those (any) custom locator will not benefit from the actions being added here.
In total, there are around 10,085 usages in the Plugins repository (according to wpdirectory.net) that will not know what is happening before or after their own calls to load_template()
because they are not using locate_template()
.
Asking respectfully for your friendly additional thoughts & feedbacks 💫🤝
(Edit: reworded some stuff for improved tone 💟)
#11
@
2 years ago
- Keywords commit removed
@johnbillion Are you able to add some thoughts in response to ticket:54541#comment:9?
Would be great to get clarification prior to beta 1.
This ticket was mentioned in Slack in #core by jeffpaul. View the logs.
2 years ago
#13
@
2 years ago
As discussed ahead of today's scheduled 6.1 Beta 1 release party (see the Slack link above), it was agreed that @johnbillion would review @johnjamesjacoby's comment#9. With no action on this ahead of the updated 6.1 Beta 1 release party tomorrow, this will be punted to Future Release.
#14
@
2 years ago
- Keywords needs-patch added; has-patch needs-unit-tests dev-feedback removed
- Status changed from reopened to accepted
@johnjamesjacoby I agree that moving these actions into load_template()
makes sense. We do lose the $template_names
context but the existing get_template_part
action includes this and I'm planning on proposing a new action that fires after locate_template()
is called inside get_template_part()
which provides an extra debugging opportunity that's specific to template parts.
I'll open a PR with your suggestion.
@JeffPaul This should remain in the 6.1 milestone as it already has a commit.
This ticket was mentioned in PR #3298 on WordPress/wordpress-develop by johnbillion.
2 years ago
#15
- Keywords has-patch added; needs-patch removed
Trac ticket: https://core.trac.wordpress.org/ticket/54541
Per feedback from J³ on the ticket, moving these new actions inside load_template()
makes sense.
johnbillion commented on PR #3298:
2 years ago
#16
This amends the change introduced in #2462. cc @Tabrisrp
#17
@
2 years ago
- Keywords commit added
Follow up pull request addressing @johnjamesjacoby's concerns approved for commit as of c70fe21d9585e.
peterwilsoncc commented on PR #3298:
2 years ago
#19
Merged in https://core.trac.wordpress.org/changeset/54270 / 77e65eace541663ac1875aa00688943d5d8bd446
This patch adds new actions before and after loading the located template in
locate_template()
.The template filename is passed as an additional parameter to the action, as that can be useful for callbacks on these hooks.
The hooks name and the
@since
value are suggestions, I couldn't find if there is a standard for hooks names in core, as there is some prefixed withwp_
, and others that are not.Trac ticket: https://core.trac.wordpress.org/ticket/54541