Use load_template in template-loader.php

[elyobo]

There may be a good reason for this I'm unaware of, but the template loader (wp-includes/template-loader.php) does not use the appropriate sounding load_template() function.

The only differences are

• A potentially smaller number of globals included, as load_templates() globalises a smaller list.
• Query vars are extracted automatically.

In my plugin, I'm trying to make things easier for our theme folks by populating the appropriate data automatically. This is possible through the load_template() function because I can push data into $wp_query->query_vars and it will be extract()ed into local variables that are available in the template.

This is *not* possible in things like single-page.php, because template-loader.php just includes the template file it finds instead of load_template()ing it.

I have attached the one line patch to change this. If the reduced number of globals would be a problem, it is also easy to include an "extract($GLOBALS);" in load_template() as well.

[elyobo]

Patch against current trunk

[scribu]

I'm curious about this as well.

[nacin]

I have an immediate negative reaction to extract($GLOBALS);. Suggest wontfix.

[nacin]

The other problem is that extract($GLOBALS); would simply create local variables of those values -- references aren't maintained to the global variables.

While this sounds like a great idea in theory -- no more stomping the $post or $wp globals, etc. -- it would break a LOT of existing code that makes edits to core globals on the fly.

So it's more than just a negative reaction. We can't do load_template() without the globals, and we can't extract the globals, so this is a wontfix.

[elyobo]

That's not entirely true, you could do something like

foreach (array_keys($GLOBALS) as $glob) {
    global $$glob;
}

I'm not sure about the performance implications of that though, WP has a lot of globals.

If we don't do that, how about adding an extract($wp_query->query_vars) to template-loader.php; this way the behaviour is consistent for the templates at least, even though the approach is inconsistent. I expect that performance would be better than manually reglobalising all the globals.

[nacin]

I never even realized that load_template() did an extract on query_vars. Because load_template() with $load = true is never used in core, this would more or less be new behavior, and it will break things. Some query var globals are guaranteed to step over variables used in template files.

We don't want more globals, we want less. If you need a query var, use get_query_var(). Again suggesting wontfix.

[elyobo]

I completely agree on the globals front, I did a check the other day and identified 272 distinct variables called globalised with the "global" keyword in WP core :)

However, load_templates doesn't take a $load argument; it always extracts the query vars if they're there (the boolean argument is whether to include or or require_once), so it is definitely doing this in core already.

[nacin]

Sorry, I meant that it's the $load = true argument of locate_template() that triggers load_template(). That is never used in core.

load_template() is used directly in only one situation: feed templates. (Also theme-compat files, but those don't really count.)

Otherwise, load_template() is just used by plugins.

[scribu]

It would have been nice if load_template() was used (so that there wouldn't be an inconsistency between templates loaded by plugins vs. templates loaded by Core), but as nacin said, we can't do that, due to backwards compatibility.

I can push data into $wp_query->query_vars and it will be extract()ed into local variables that are available in the template.

That doesn't sound like a very good practice. The way WP handles this is by using template tags (i.e. functions) like the_title(). Your plugin should define it's own template tags that access globals in the background, as necessary.

[nacin]

A better WP implementation would have been bbPress's version of load_template():

function bb_load_template( $files, $globals = false ... );

Pretty nifty, allows things like this:

bb_load_template( 'login.php', array( 'user_exists', 'user_login', 'remember_checked', 'redirect_to', 're', 'bb_login_error' ) );

The bbPress code is full of code like this -- WordPress-inspired, but improved in the slightest of ways.

[elyobo]

nacin, yeah, that makes more sense.

That said, get_header(), get_footer() and get_sidebar() all call locate_template with $load = true. While these are called from templates, so may technically not be core, every site must have a template, so every single site is seeing this behaviour already in their sidebars, headers and footers... but not in their pages, errors, 404s etc which are loaded by template-loader.php.

scribu, I'm not sure how extracting the query vars would break backwards compatibility; if EXTR_SKIP is used it will not stomp on any other variables and if the globalising approach was used all the expected globals would be present - either solution should be backwards compatible.

nacin, the bbPress's version sounds like exactly what I was looking for when I went digging in to find out how to inject my variables into theme templates :) It seemed like the query_vars approach worked, but then I discovered it only worked some of the time (as above) and not the rest, which just seemed inconsistent.

[scribu]

scribu, I'm not sure how extracting the query vars would break backwards compatibility

That's not what I meant. I was trying to say that using query_vars to pass arbitrary values to a template is not an elegant solution.

[scribu]

As such, I don't think we should be encouraging it by using load_template() in template-loader.php (even if it's done in a backward-compatible way).

[elyobo]

Sorry scribu, I misunderstood then.

It seems a much more elegant solution (to me) than making my front end guys learn new functions to access bits of my data, rather than the $variables that they already know and love. Given that this data is already extracted into header, footer and sidebar templates, it seems like it would also improve consistency and usability.

You're right that there should be a proper way to do this; but there currently isn't one.

[scribu]

Replying to nacin:

A better WP implementation would have been bbPress's version of load_template():

function bb_load_template( $files, $globals = false ... );

Even more flexible would be this:

function load_template( $files, $data = array() );

Then you could pass any data you want, including globals:

load_template( 'login.php', wp_array_slice_assoc( $GLOBALS, array( 'user_exists', 'user_login', 'remember_checked', 'redirect_to', 're', 'bb_login_error' ) ) );

[elyobo]

Replying to scribu:

From the look of his example, I think that's how the bbPress version works as well.

[scribu]

No, bb_load_template() only makes available existing globals, like global $$key, whereas my example would extract( $data ).

[elyobo]

Ah, my wordpress ignorance showing through :) Should have looked up wp_array_slice_assoc.

Anyhow, I have to sleep. Thanks for the discussion folks. I guess we'll have to wait for an elegant solution to the problem to turn up. Would there be any interest in me opening a feature request to make it possible to do what I want (injecting variables into templates)?

In the meantime, I think I'll just add something like the following line at the top of my templates and use this to inject the data I want into each template; it requires me to modify each template, but it prevents the front end guys from having to learn stuff about the internals of how the data is generated, so it's most of the way there.

<?php extract(apply_filters('template_data', array()), EXTR_SKIP); ?>

[scribu]

Since in your own theme you don't have to worry about backwards-compatibility, you could force template-loader.php to call load_template():

function force_load_template( $template ) {
	load_template( $template );

	return false;
}
add_filter( 'template_include', 'force_load_template', 999 );

Re-closing as wontfix. Feel free to leave further comments, though.

[nacin]

You could also extract your variables into the global namespace on the template_redirect or template_include hooks.

[scribu]

You could also extract your variables into the global namespace on the template_redirect or template_include hooks.

That wouldn't work, since you would be in the callback's scope, not in load_template()'s.

[scribu]

Nevermind, yes, it would work, since the template is loaded in the global scope.

[scribu]

Thinking more about the ideal load_template() function, even if it did accept a $data arg, it would still be akward to use, since it's called from WP Core, not your own code.

But with the 'template_include' filter you can do pretty much whatever you want: check basename( $template ), use conditional tags like is_single(), use your own version of load_template() etc.

[elyobo]

Globalising variables would work, and is effectively doing the same as what extracting the query vars in the template loader would have done, but nacin is right that we don't need need even more globals floating around.

I feel pain every time I'm forced to use an exit or die statement; it makes unit testing a pain and feels like a hack. That said, it looks like simply returning a value that evaluates to false will stop the original load from happening, without having to resort to a die statement, so it seems like a workable approach; all that needs to be done is to remove the die statement from scribu's suggestion.

Thanks for the help.

[scribu]

Hey, you're right. Updated my code.

[elyobo]

Well, I'm right so long as nothing else further down in the filter chain returns a non-false value, which calling die guarantees won't happen ;) But as this is an internal project, I think I can make sure that's the case.