WordPress.org

Make WordPress Core

Opened 20 months ago

Closed 8 months ago

Last modified 5 months ago

#21676 closed enhancement (maybelater)

Pass a variable to get_template_part()

Reported by: sc0ttkclark Owned by:
Milestone: Priority: normal
Severity: normal Version: 3.4.1
Component: Template Keywords: has-patch close
Focuses: Cc:

Description

Having the ability to pass a variable into get_template_part would be invaluable, it would really clean up the issues with using globals and to further cement it's use rather than the alternative of a normal PHP include/require.

It would be the third variable passed into get_template_part, which I could pass it a string, array, object, or whatever I want really. Here's an example of passing all the current scope's variables:

<?php get_template_part( 'twitter', 'feed', compact( array_keys( get_defined_vars() ) ) ); ?>

and then catching them in my feed-twitter.php file:

if ( !empty( $_data ) && is_array( $_data ) )
	extract( $_data, EXTR_SKIP );

I can pass other things in, but that's a really valuable example in my uses for this new variable.

Attachments (4)

21676.patch (2.7 KB) - added by sc0ttkclark 20 months ago.
21676.2.patch (2.9 KB) - added by sc0ttkclark 19 months ago.
Refreshed with extract / array usage as discussed with scribu
21676.3.patch (2.9 KB) - added by sc0ttkclark 15 months ago.
Refreshed, using $args instead of $data for future-proofing additional functionality
21676.4.patch (2.9 KB) - added by sc0ttkclark 15 months ago.
Require $args[ 'data' ] to be an array before extracting

Download all attachments as: .zip

Change History (51)

sc0ttkclark20 months ago

comment:1 sc0ttkclark20 months ago

This is a ticket split off of #21673 to make it more viable for inclusion.

comment:2 scribu20 months ago

+1. Currently, you're forced to implement your own version of load_template() if you want to pass additional variables, without polluting the global namespace.

comment:3 knutsp20 months ago

  • Cc knut@… added

comment:4 sc0ttkclark20 months ago

  • Type changed from feature request to enhancement

comment:5 sc0ttkclark20 months ago

  • Keywords has-patch dev-feedback added

comment:6 DrewAPicture20 months ago

  • Cc xoodrew@… added

comment:7 sc0ttkclark19 months ago

For devs -- should I update the patch to switch get_template_part to accept $args wp_parse_args with the new '_data' variable?

comment:8 scribu19 months ago

  • Keywords needs-testing added; dev-feedback removed
  • Milestone changed from Awaiting Review to 3.5

For anyone else confused by that question, it was in reference to this:

http://core.trac.wordpress.org/ticket/21673#comment:4

As Mike said, adding one extra parameter to get_template_part() is fine.

But checking if the $slug is an array and then treating the whole function as having associative args would be really confusing.

In conclusion, the patch looks fine. Let's try to land it in 3.5.

Last edited 19 months ago by scribu (previous) (diff)

comment:9 sc0ttkclark19 months ago

Sounds great! Thanks for the review and feedback, just wanted to be sure I did everything expected / wanted of me for this ticket and for future-proofing things.

comment:10 nacin19 months ago

  • Milestone changed from 3.5 to Future Release

I am not a fan of this.

$_data should always be an array and should be extracted. It should be for passing variables, not arbitrary data.

Globals and query variables should not be extracted, as they are now. It's confusing and lame. That means this should not be get_template_part(), and it should instead be something like wp_load_template() (similar to the old bb_load_template()).

This would essentially be a new utility function, a clean break from everything else. get_template_part() is designed for template modularization and child theme inheritance, let's not complicate it further.

comment:11 wonderboymusic19 months ago

Want a dirty workaround?

function get_template_part_data() {
	$func = wp_list_filter( debug_backtrace(), array( 'function' => 'get_template_part' ) );
	if ( $func )
		return $func[key($func)]['args'][2];
}

$data = array( 'foo' => 'bar' );
get_template_part( 'content', 'whatever', $data );


exit();

And then in content-whatever.php:

print_r( get_template_part_data() );

comment:12 scribu19 months ago

I think nacin's implicit objection is this:

Adding a $data parameter to get_template_part() is wrong because it mixes two ways of "templating":

1) The "WordPress" way, where you call the_title(), the_content() etc. + globals like $wp_query.

2) The "bare-bones" way, where you pass an array of data, which gets converted into some variables:

function wp_load_template( $path, $data ) {
  extract( $data );
  include $path;
}

@wonderboymusic: Do you chat with your mother using those fingers? :P

comment:13 wonderboymusic19 months ago

Nacin just Skype'd me his disgust as well. I made sure the word "dirty" was in there! but +1 for using wp_list_filter, no?

comment:14 sc0ttkclark19 months ago

The end goal of this ticket, is to provide a way to pass data and additional context into a template file, that's child/parent theme aware, and includes that file.

get_template_part does part of that, the proposed wp_load_template would do part of that, why not go all the way or at the very least offer another function that does handle child/parent theming?

sc0ttkclark19 months ago

Refreshed with extract / array usage as discussed with scribu

comment:15 sc0ttkclark19 months ago

  • Keywords dev-feedback 2nd-opinion added

I went ahead and refreshed the patch with extract / array usage as discussed with @scribu

I think we should have another dev hop in on this conversation, it seems like including a file with get_template_part or load_template and having the ability to scope variables in, is something that people work around with dirty global variables.

It would seem like this would improve usage of core theming functions like these as opposed to include/require.

Even with @wonderboymusic's idea, that could work as a work-around, but then everyone has to add that function and it'll be another thing every themer bakes differently.

The inclusion of this patch seems rudimentary, it holds great value, and I'd like to open this discussion up to any core developer or theme developer who may have other points to make.

comment:16 GeertDD16 months ago

  • Cc geertdd@… added

I definitely do like the proposed functionality. Being able to pass on data to a template allows you to separate your markup from your logic. It would work much like a View (as in MVC).

comment:17 stephenh198816 months ago

  • Cc contact@… added

comment:18 beatpanda15 months ago

This comes up a lot in my work, and some of my plugins and themes would be drastically better if this was core functionality. Right now I'm having to implement my own version of WordPress core functions to do this.

I know this isn't likely to happen, but if the $data array used the EXTR_OVERWRITE option instead of EXTR_SKIP, that would be extremely helpful, because then I could pass in a custom set of posts to a template as $posts or $post in the $data array and the template would still "just work".

I came here to submit roughly this exact patch, which is roughly what my hacky implementation looks like. Please implement this.

sc0ttkclark15 months ago

Refreshed, using $args instead of $data for future-proofing additional functionality

comment:19 sc0ttkclark15 months ago

Just attached a new patch which proposes a slightly different and more future-proof approach. It uses an $args array, to provide the ability to add additional features later on down the road, instead of merely adding another $var to the parameters list.

As part of the new $args array, if you set $args[ 'data' ] to an array of 'key' => 'value', then it will extract() that (like the previous patches).

sc0ttkclark15 months ago

Require $args[ 'data' ] to be an array before extracting

comment:20 sc0ttkclark15 months ago

Also, in my refreshed patch, $args[ 'data' ] is now extracted above $wp_query->query_vars so you can easily override any of those.

For overriding globals as set in load_template, the best way there is to interact with the globals themselves like $GLOBALS[ 'posts' ] etc

comment:21 giuliom15 months ago

  • Cc giulio.mainardi@… added

comment:22 johnzanussi15 months ago

  • Cc johnzanussi added

comment:23 iandunn14 months ago

  • Cc ian_dunn@… added

comment:24 follow-up: wonderboymusic14 months ago

  • Keywords close added; needs-testing dev-feedback 2nd-opinion removed
  • Milestone changed from Future Release to Awaiting Review

I think passing $data or $args down the chain is worse than importing globals. load_template() imports a bunch of globals, that's how function scope in PHP works. Is it better to import those variables using an argument to load_template()? That's not even the question being asked, the question is how to jam more data in there, and the answer is simple: import those globals in your function. If you hate global scope, limit your app's global pollution to one object - for instance, I use an $emusic global variable (stdClass), and I have a get_emusic() function to retrieve it, so I import zero globals, yet always have access to them:

function where_are_my_vars() {
     $vars = get_emusic()->right_here;
}

Suggesting close / wontfix

comment:25 sc0ttkclark14 months ago

  • Cc lol@… added

The idea behind this patch was to give get_template_part the ability to work more like a normal include/require works, passing variables from the current scope (or whatever is wanted) directly to the new scope of load_template()

Using globals and doing the custom code for each template was something I explicitly set out not to have to do for every single template. There's just no other cleaner way that I can see to bring variables in scope (without making them globals) with the template being included.

Passing $args to get_template_part also opens things up to further patches that could also enhance how it works and functions.

comment:26 ocean9013 months ago

#23924 was marked as a duplicate.

comment:27 mbijon11 months ago

  • Cc mike@… added

comment:28 amaschas8 months ago

  • Cc amaschas added

The fact that imported globals are widely used in Wordpress doesn't mitigate the inherent drawbacks of the overuse of globals and the global singleton pattern in development. We need to be able to scope variables to templates for the same reason we need to scope variables anywhere, because the global scope makes knowledge of the state of a variable difficult, and reduces the modularity of code. At this point, getting variables into template parts seems to involve building a maze of functionality around managing imported globals, just to duplicate the functionality of local scope.

I'm currently working on multiple projects that require building frontend implementations of complex data structures, many of which contain repeated nested elements. I end up wrestling with a choice of either building enormously complex templates to handle the data object, or breaking the elements into smaller templates and relying on imported globals in some fashion. I've run into a number of situations where tracking the state of multiple globals made testing and debugging much more difficult than they needed to be. While I'm able to solve these problems, the solutions are often byzantine, which is frustrating given that scoping is a solved problem that helps code conform to the principles of OO design.

comment:29 mintindeed8 months ago

  • Cc gabriel.koen@… added

+1 for this. It doesn't make sense to expose all your vars in the global scope, or have to write a custom accessor, just so you can use a core function. Not to mention the whole mess of global name collisions.

comment:30 buffler8 months ago

  • Cc jeremy.buller@… added

comment:31 iamfriendly8 months ago

  • Cc richard@… added

comment:32 c3mdigital8 months ago

I am opposed to adding arbitrary data arguments to get_template_part(). I'm curious to know what kind of data you need to pass to to a template? If your passing an array of posts or an instance of WP_Query that's not the main query I would say your doing it wrong.

comment:33 sc0ttkclark8 months ago

@c3mdigital - Not everything is a post or query. Example: Advanced term loops which have considerable template html for them that needs to be used in more than one place. User loops, etc..

comment:34 follow-up: amaschas8 months ago

c3mdigital - Post meta data and related objects. The post types I'm working with often have many custom fields, options that enable or disable features or markup and related or child post objects that need to be populated. Past a certain level of complexity, you really need some sort of controller object to build the set of meta data for a post type, and there's currently no good way to get that data into the template without using globals. As you can imagine, using globals for this purpose in a sufficiently large problem quickly becomes problematic.

comment:35 ysalame8 months ago

I second amaschas reasons with massive force. Multiple times I had the need to go around get_template_part() because of this limitation. Abuse of globals is bad practive and multiple times I just need small bits of data on partial content.

comment:36 follow-up: wonderboymusic8 months ago

Multiple times I had the need to go around get_template_part() because of this limitation

I go around XML all the time by using JSON

comment:37 in reply to: ↑ 24 jeremyfelt8 months ago

Replying to wonderboymusic:

I think passing $data or $args down the chain is worse than importing globals.

and

If you hate global scope, limit your app's global pollution to one object

Those sum up my feelings.

+1 for close/wontfix

comment:38 in reply to: ↑ 34 c3mdigital8 months ago

Replying to amaschas:

there's currently no good way to get that data into the template without using globals. As you can imagine, using globals for this purpose in a sufficiently large problem quickly becomes problematic.

The WordPress way of doing that is with template tags. There is no reason you can't call get_post_meta on the same data more than once if you need it, it's already cached for you so there is really no additional overhead. You can also cache your own data for use in any template part. I don't discredit your need but I'm still not convinced that a basic template function should be used for it.
+1 close/wontfix

comment:39 wonderboymusic8 months ago

  • Milestone Awaiting Review deleted
  • Resolution set to wontfix
  • Status changed from new to closed

Since the original example proves that data be accessed inside function scope already - compact( array_keys( get_defined_vars() ) ) without being passed down the river

comment:40 GeertDD8 months ago

  • Cc geertdd@… removed

comment:41 in reply to: ↑ 36 ysalame8 months ago

Replying to wonderboymusic:

Multiple times I had the need to go around get_template_part() because of this limitation

I go around XML all the time by using JSON

The choice between XML and JSON is an "implementation" choice not a "best practice" choice. You cannot argue best practice against JSON or XML or any other structured data since they depende on your environment. But putting any data not needed within a global scope inside the globals variable just to avoid passing a variable, for me, is bad practice.

comment:42 caiocrcosta8 months ago

I've been using my own get_template_part equivalent for nearly two years now, mainly because I need a controller function and to repeat the same HTML block (with variable data) accross the site. The function scope allows me to use any variable name I want to and to keep the PHP code inside the template file at a minimum.

function get_block( $file, $data = array() ) {
	$dir = trailingslashit( get_stylesheet_directory() );
	$blocks_dir = 'blocks/';
	$filepath = $dir . $blocks_dir . $file . '.php';

	if ( is_array( $data ) )
		extract( $data );

	return include( $filepath );
}

comment:43 follow-up: sc0ttkclark8 months ago

I believe I've put some strong arguments for passing a variable/args and/or scoping variables for get_template_part. I haven't heard a compelling enough argument against all of these potential options, and it seems like there are others who have been having the same concerns.

Core Devs: Is there not enough merit in this function having this ability, or another function created with child/parent theme aware including of a file and passing that file an arg array or to scope vars?

comment:44 in reply to: ↑ 43 nacin8 months ago

  • Resolution changed from wontfix to maybelater

Replying to sc0ttkclark:

I believe I've put some strong arguments for passing a variable/args and/or scoping variables for get_template_part. I haven't heard a compelling enough argument against all of these potential options, and it seems like there are others who have been having the same concerns.

Core Devs: Is there not enough merit in this function having this ability, or another function created with child/parent theme aware including of a file and passing that file an arg array or to scope vars?

I don't think the reasoning behind the votes for close/wontfix are very strong. But I do think this should be closed for now.

Is there merit in a function having this ability? Actually, yes.

bbPress 1.x has similar functionality: http://bbpress.trac.wordpress.org/browser/branches/1.1/bb-includes/functions.bb-template.php#L3. bb_load_template() essentially allows someone to pass an array of variables that then get pulled in. I wouldn't implement it exactly like this, and I'd probably lean off global scope a bit, but either way, this isn't a new idea.

But is there merit in this function having this ability? That I am less sure about — get_template_part() is designed to do one thing, and in order to do that, it requires two arguments. Adding another argument that does something goofy like passing scoped arguments is probably *not* the best way to do this. It's possible, based on type-juggling, to let the second *or* third argument (if the current second argument isn't used) to be an array of variables.

Whether we introduce a new function or overload this function's arguments, there is still the issue of education. Passing variables around is not an easy concept to explain. Many theme developers are not developers at all. I would hesitate to add anything to this API without clearing setting a new standard for theme development, much the way get_template_part() did. For example, if we did this, I would want a clean break from all global scope. That means sidestepping locate_template() and load_template(), which would also mean a significantly simpler call stack and less of a rabbit hole for loading a template.

And since you don't want all of that globals magic, it's very simple to roll your own function for now, for however it may suit you.

Once nice thing about get_template_part() is it is a relatively new function (2-3 years) and people are still trying to figure out how to best leverage it. Even core has opted for a few different patterns through four default themes. There are also various theme frameworks and platforms out there that allow for ample opportunity for experimentation.

I don't think this is a wontfix. I do however think it is a maybelater. I think it is something that we should put in our back pocket and come back to it in a few months or longer and see how it feels then.

comment:45 mboynes8 months ago

  • Cc mboynes@… added

comment:46 helen6 months ago

#25558 was marked as a duplicate.

comment:47 jonathanbardo5 months ago

  • Cc bardo.jonathan@… added
Note: See TracTickets for help on using tickets.