Opened 12 years ago
Closed 4 years ago
#21676 closed task (blessed) (fixed)
Pass a variable to get_template_part()
Reported by: | sc0ttkclark | Owned by: | SergeyBiryukov |
---|---|---|---|
Milestone: | 5.5 | Priority: | normal |
Severity: | normal | Version: | |
Component: | Template | Keywords: | has-patch needs-testing has-dev-note |
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 (13)
Change History (128)
#2
@
12 years 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.
#7
@
12 years ago
For devs -- should I update the patch to switch get_template_part to accept $args wp_parse_args with the new '_data' variable?
#8
@
12 years 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.
#9
@
12 years 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.
#10
@
12 years 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.
#11
@
12 years 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() );
#12
@
12 years 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
#13
@
12 years 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?
#14
@
12 years 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?
#15
@
12 years 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.
#16
@
12 years 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).
#18
@
12 years 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.
#19
@
12 years 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).
#20
@
12 years 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
#24
follow-up:
↓ 37
@
12 years 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
#25
@
12 years 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.
#28
@
11 years 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.
#29
@
11 years 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.
#32
@
11 years 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.
#33
@
11 years 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..
#34
follow-up:
↓ 38
@
11 years 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.
#35
@
11 years 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.
#36
follow-up:
↓ 41
@
11 years 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
#37
in reply to:
↑ 24
@
11 years 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
#38
in reply to:
↑ 34
@
11 years 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
#39
@
11 years 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
#41
in reply to:
↑ 36
@
11 years 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.
#42
@
11 years 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 ); }
#43
follow-up:
↓ 44
@
11 years 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?
#44
in reply to:
↑ 43
@
11 years 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.
#48
@
11 years ago
Is it time to re-examine this? I definitely prefer the approach of passing $data
or $args
as an associative array into get_template_part()
; but whether it's get_template_part()
or another similar function, I strongly recommend this be implemented with core.
I agree with Nacin's point about the data being extractable -- not $args['data']
from Comment 10 :
$_data should always be an array and should be extracted. It should be for passing variables, not arbitrary data.
This ticket was mentioned in Slack in #themereview by emiluzelac. View the logs.
10 years ago
#53
@
9 years ago
I definitely think being able to pass variables to template parts is a good idea. Would be good to have core functionality for this, be it the get_template_part() function or a different method.
#54
@
8 years ago
How is this not a thing yet? There are limitless reasons to pass variables into have nothing to do with the global scope. The very simplest being a template that needs to load another template, based on a variable.
So here I go writing the same custom workaround into my functions.php file that everyone has.
#55
@
8 years ago
:+1: to adding a third optional array() variable to get_template_part() function. it is time to re-open this ticket as we are already 1 major version late.
#56
@
8 years ago
Well, meanwhile, this can be used which will do the same thing
/** * Load a template part into a template * * @param string $slug The slug name for the generic template. * @param string $name The name of the specialised template. * @param array $params Any extra params to be passed to the template part. */ function get_template_part_extended( $slug, $name = null, $params = array() ) { if ( ! empty( $params ) ) { foreach ( (array) $params as $key => $param ) { set_query_var( $key, $param ); } } get_template_part( $slug, $name ); }
I use this in my projects sometimes, it makes extra params available in the template part.
#57
@
8 years ago
I have been using:
include( locate_template( 'template-parts/content.php' ) );
I think it works well…
#58
@
8 years ago
- Resolution maybelater deleted
- Status changed from closed to reopened
I would to get back on track with this long awaited and requested enhancement.
I just updated the patch for current trunk in my previous ticket #36187 (even if was marked as duplicate).
The patch affects not only get_template_part()
but, other than load_template()
, locate_template()
, all get_*
template family functions (I think that also it could be useful for example in get_header()
). Unit test has been also added.
I used $params
as scoped variable name.
Even if there are workarounds for that, like use of globals, or include(locate_template())
pattern and so on, the patch like that would really to clean up those approaches.
In my opinion, abuse of globals is not too good practice and multiple times the people just need to repeat bits of data on partial content (for example loops which have considerable template HTML).
Moreover they, like me of course, would like to reuse this template components across the site(s) in more than one place and or in different themes creating a sort of templates codebase repository (not plugins nor themes).
@
8 years ago
This patch replace previous one (where I accidentally removed an argument to search_form_format filter)
This ticket was mentioned in Slack in #core by enrico.sorcinelli. View the logs.
7 years ago
This ticket was mentioned in Slack in #core by enrico.sorcinelli. View the logs.
7 years ago
#64
@
7 years ago
I just updated the patch to the current trunk.
I used $params
as scoped variable temporary name, but we should chose a more specific name (for example $wp_tmpl_args
) in order to avoid possible conflicts.
#66
@
7 years ago
I'm personally not a fan of using extract()
as part of the solution to this problem. I think it will ultimately lead to confusing templates and undefined variables. Particularly for newer developers, I believe the ambiguity will quickly become frustration, especially given the existing state of affairs where some core variables are already loaded into the template part scope. Imagine someone pulling a template part out of a future twentyeighteen
or twentynineteen
theme and wondering why they're getting errors on one variable, e.g. $page_title
but not another, $paged
.
About 5 years ago my company tried out several approaches, this one included, and eventually landed on a solution where we created a template tag to retrieve the variables passed to the template part. That template tag takes the variable name (array key for what gets passed to the template part, same as in the patches in this ticket). It also takes an optional default value, if the variable wasn't set. In core, this might look something like:
<?php // single.php get_template_part( 'content', get_post_type(), array( 'image_size' => 'thumbnail' ) );
<?php // content-post.php the_post_thumbnail( get_template_var( 'image_size', 'small' ) );
If others have interest in considering this pattern, I could put together a patch.
#67
@
7 years ago
Hi @mboynes did you take a look on the latest patch?
It doesn't use extract()
nor introduces globals at all, but allows to pass variables scoped to template files, for example:
<?php get_template_part( 'parts/template', '', array( 'foo' => 'baz' ) );
and in parts/template.php:
<?php echo $params['foo'];
or:
<?php get_header( 'header', '', array( 'foo' => 'baz') );
and so on.
The $params
scoped variable name can be replaced with a more specific name, for example $wp_tmpl_args
.
#68
follow-up:
↓ 69
@
6 years ago
I second @enrico.sorcinelli - it's indeed a really good pattern and allows for more robust templating system. Much more robust than what we have now.
@SergeyBiryukov - anything we can do to help this move forward? This is as a solid improvement for front-end devs just like G is for the editors :)
#69
in reply to:
↑ 68
@
5 years ago
Replying to mihai2u:
I second @enrico.sorcinelli - it's indeed a really good pattern and allows for more robust templating system. Much more robust than what we have now.
@SergeyBiryukov - anything we can do to help this move forward? This is as a solid improvement for front-end devs just like G is for the editors :)
+1
This would be an extremely useful feature for developing themes
Ps is this ticket dead?
#70
@
5 years ago
- Keywords close removed
Removing the close
keyword as this should be re-evaluated now considering recent comments.
@mboynes Would you be able to create a patch with your suggested approach so that both can be tested and considered when deciding the direction for this one?
#71
@
5 years ago
Hey @desrosj, I'd be happy to do so. Should be able to get something up here in about a week.
This ticket was mentioned in Slack in #core by enrico.sorcinelli. View the logs.
5 years ago
#73
@
5 years ago
I agree with the core devs that get_template_part
should not have variables passed. Everything needed in a template part should already be retrievable through functions or you need to be doing something else entirely. The theme has to be fairly generic to work in all situations. Custom details belong in plugins.
+100 for close or wontfix
#74
@
5 years ago
After a brief conversation on slack during last CD, I just refreshed the patch to the current trunk.
I used $wp_tmpl_args
as scoped variable name, but we could chose another one in order to avoid possible conflicts.
Core devs already added this approach in 5.2.0 to get_search_form()
function. For example, twentytwenty theme uses it by passing array( 'label' => ... )
as scoped arguments to searchform.php template, so I cannot see any reasonable why to don't apply it also to other get_*
template family functions.
#75
@
5 years ago
I agree with @enricosorcinelli, for consistency we need to add this feature.
It isn't uncommon in WordPress to have functions that support to pass custom variables or have filter that let to do so.
Considering also that there is a lot of people asking for that https://www.google.com/search?q=get+template+pass+variable+wordpress
I remember also a composer package that add a similar function just as replacement for the native one.
#76
@
5 years ago
I usually work around this limitation by using set_query_var()
before calling get_template_part()
so as not to pollute global scope. It does pollute the query vars array - but is generally safe as template_include
fires after the main query/request has been made.
This is a very annoying limitation to the WordPress templating system. Especially the fact that get_template_part()
does not behave like PHP's native templating system - ie: include
- which does recognize variables set in the scope where it was called. Leading to hack-ish workarounds like include(locate_template())
.
As for the latest patch. I don't much like the naming convention $wp_tmpl_args
. I much prefer descriptive readable variable names: $wp_template_args
, $wp_template_vars
.
Also since WP is moving towards integrating React perhaps $props
would be a better naming convention - alluding to React component props. React components are roughly equivalent to WP's template_part
, and the usage of $props
within the template/component is pretty much the same (PHP <? ?>
tags instead of curly braces {
} ).
#77
@
5 years ago
I know, there are workarounds for that, like use of globals, get_query_var()
/set_query_var()
(that internally use global $wp_query
and aren't made specifically for that purpose) or include( locate_template() )
pattern and so on, but a patch like that would really clean up those approaches.
Even if the patch lost a little bit of its charm (the view part is moving toward on the client side), IMHO I think it could be still useful.
And about the variable name, $wp_tmpl_args
it's only a proof of concept. We can choose a better appropriate name
#78
@
5 years ago
I'm all for moving this forward. A third parameter for variables/args/props seems workable to me. I'm not a fan of hack-ish workarounds.
ps. no need for get_query_var()
. Query vars are extracted in load_template()
right before the require:
extract( $wp_query->query_vars, EXTR_SKIP );
So set_query_var()
seems the cleanest workaround while this gets fixed (or moved back to wontfix land). Thought I'd mention it in this ticket.
This ticket was mentioned in Slack in #core by enrico.sorcinelli. View the logs.
5 years ago
This ticket was mentioned in Slack in #core by enrico.sorcinelli. View the logs.
5 years ago
#81
@
5 years ago
I agree with having a third parameter being used to pass variables through the get_template_part function.
This ticket was mentioned in Slack in #core by enrico.sorcinelli. View the logs.
4 years ago
#85
@
4 years ago
If this ticket is adopted I would very much like to see the variables extracted into the template's scope. In the same way that query_vars
are extracted directly into the template's scope.
This would go a long way towards ease of use of templates with this new functionality.
A decision should be made as to precedence order in case of a variable name conflict.
My view is that in case of a conflict with a query_vars
variable name, the variables passed to get_template_part
should take precedence.
#86
@
4 years ago
The problem with extracting the vars is you risk nuking global variables and causing breakage elsewhere. If I pass in a parameter called $post
for example, that would cause problems with any loop further down the page.
#87
@
4 years ago
The problem with extracting the vars is you risk nuking global variables and causing breakage elsewhere. If I pass in a parameter called $post for example, that would cause problems with any loop further down the page.
Understood.
In that case global variables should take precedence/be protected over extracted args, and passed parameter variables should take precedence over query_vars.
How does core handle this if a custom query_var is added that is also a global variable name?
#88
@
4 years ago
I think I'd like to use just $args
as the parameter name, for consistency with get_search_form()
.
Since this is new functionality, I don't expect that to cause any back compat issues. The initial value would be an empty array, but if someone already uses the variable locally, it should continue working as it did before.
If we do want to be specific, I'd suggest $template_args
, but I think $args
might just work. Thoughts?
On the other hand, $template_args
might be a bit more future-proof if these functions ever get an $args
parameter that should *not* be passed to the template, but that seems unlikely.
Still thinking about the naming, but this appears to be in good shape and I think can be tried for beta.
As already noted above, I believe we'd want to avoid the extract()
usage, as most instances were eliminated from core in #22400, except for one instance in load_template()
that was left for backward compatibility. The extract()
usage is also discouraged per the coding standards.
I would also like to further explore the idea of get_template_var()
from comment:66. Something like (get|set)_template_var()
that would work pretty much like (get|set)_query_var()
, but without polluting the main query object. Any thoughts on that?
#89
@
4 years ago
I'd like me too avoiding use of extract
.
A (get|set)_template_var()
approach would imply use of globals that's exactly what the ticket would like to avoid.
IMHO, moreover with such approach we should also add additional code to cleanup template variables after each template call.
#90
@
4 years ago
As already noted above, I believe we'd want to avoid the extract() usage, as most instances were eliminated from core in #22400, except for one instance in load_template() that was left for backward compatibility. The extract() usage is also discouraged per the coding standards.
As we are still within the specific context of load_template, where extract() is still allowed, I think this can still be allowed without regarding this as a new exception to the coding standards.
I find that usage of <img src="<?php echo $img ?>" />
(which can currently be achieved using set_query_var('img')
) should also be available to variables added through the new $args.
Adding this feature but handicapping its like so: <img src="<?php echo $args['img'] ?>" />
makes this feel like a half-measure and not a robust/smooth addition to WP templating system.
I think the extra work that would be required to protect global variables is certainly worth it.
#91
follow-up:
↓ 92
@
4 years ago
In other words, themes that currently use the set_quer_var();get_template_part();
pattern can be switched to get_template_part($args)
while the template-parts themselves can remain unchanged.
#92
in reply to:
↑ 91
@
4 years ago
Replying to apedog:
In other words, themes that currently use the
set_quer_var();get_template_part();
pattern can be switched toget_template_part($args)
while the template-parts themselves can remain unchanged.
Actually, they can't make that simple change without checking for WP version.
What is the actual goal? With the coming template parts in Full Site Editing, why do you want to complicate things like this?
#93
@
4 years ago
As noted above, the patch adds a new functionality, that don't cause back compat issues.
@apedog: templates using set_query_var()
still remain working as which using globals: the primary goal isn't to rewrite existing templates or patterns but to offer a cleaner/faster templating approach.
As I wrote, I know that the patch lost a little bit of its charm but IMHO I think it could be still useful and even if the view part is moving toward on the client side, there are a lot of situations where that it's not fully applicable or you don't want to do it.
#94
follow-up:
↓ 96
@
4 years ago
There are 2 discussions being had here at the moment.
The first is whether a third parameter should be added to allow passing variables to the template part.
On that discussion I am wholeheartedly a +1 for adding this feature. It is a useful pattern to me. And it is not detrimental to those who do not wish to pass variables to their template parts.
The second discussion is the implementation discussion.
The second discussion is only relevant if WP goes forward and implements this feature.
As a user of this pattern I would very much like to access my variables directly within the templates. ie. $var_name
and not $args['var_name']
As stated above by John and Sergey this poses some technical and standards issues that would need to be hashed out:
The first is the technical issue of protecting global variable names like $post
. This can and should be done.
The second is the coding standards issue of using PHP's extract()
which is discouraged outside of load_template()
. As this feature IS implemented in load_template()
and IS attempting to offer the same option as set_query_var()
currently allows - direct access to variables - it is my opinion that extract()
can be justified here as well.
This ticket was mentioned in Slack in #core by enrico.sorcinelli. View the logs.
4 years ago
#96
in reply to:
↑ 94
@
4 years ago
Replying to apedog:
The second is the coding standards issue of using PHP's
extract()
which is discouraged outside ofload_template()
. As this feature IS implemented inload_template()
and IS attempting to offer the same option asset_query_var()
currently allows - direct access to variables - it is my opinion thatextract()
can be justified here as well.
To my understanding, this is not entirely correct. Usage of extract()
is discouraged without exceptions. I'm not sure it's stated anywhere that it is discouraged only "outside load_template()
".
Quoting:
most instances were eliminated from core in #22400, except for one instance in load_template() that was left for backward compatibility.
I trust and support @SergeyBiryukov's judgment here:
this appears to be in good shape and I think can be tried for beta.
#97
@
4 years ago
@apedog: extract
has been left only for backward compatibilty, so its presence should not justify further use.
I agree with name's consistency choice ($args
) made by @SergeyBiryukov.
So, I hope it will be given an opportunity for beta :-)
#98
@
4 years ago
Hi,
Just to add my two cents on this:
- I tested
21676.13.diff
and it works fine on my side. - About the use of
$args
/extract()
: I don't have the full background on what is "discouraged" so far, but using$args
looks fine to me. - It would be really great to get this in WP 5.5. Can I suggest to move this ticket to the related milestone? We're still in Alpha cycle for few days and it would be a great enhancement. I don't think there is any security issue with the current approach, so in the worst scenario where we finally need to revert it, it's still safe for WordPress instances that run trunk.
ps: I didn't remove needs-testing
workflow keyword, since other may want to add the results of their tests as well…
#99
@
4 years ago
Backward compatibility in this case is the ability to use query_vars directly within the templates. It is an established way to access variables within the template parts.
It's my opinion that it would be beneficial (and less confusing) if this pattern of accessing variables directly within the templates is kept consistent with how query_vars are accesses within the templates.
If the issue is using extract()
, per se, then surely code can be written to achieve the same effect with a foreach
loop, no?
#101
@
4 years ago
I'm struggling to think of a single major cpt plugin (woo, Edd, moderntribe, buddypress, geodirectory) that doesn't implement their own version of get_template_part() that accepts args. And most premium themes I've dealt with too.
To misuse the 80/20 rule: this is the pattern of inclusion developers are using, so imho WP should standardize it and include it in core.
#102
@
4 years ago
Surreal to see so much interest after all this time, pumped for some args availability regardless of extract() or $args. Though I have become an $args believer as of late.
#103
@
4 years ago
- Version 3.4.1 deleted
Personally I would love to use $args
, but this looks like it will break any existing template part that is using a global $args
which would now get reset to an empty array with this patch.
Can we change the name of the parameter to e.g. $__wp_args
and then if any value is passed in, copy it into an $args
variable?
#104
@
4 years ago
Example code for my previous comment:
$args = [ 1, 2, 3 ]; get_template_part( 'foo' );
The foo.php
template part will now see $args
as an empty array instead of the expected value.
#105
@
4 years ago
- Version set to 3.4.1
It is not a good idea to use extract()
beacuse you may have bad moment while debugging your code.
Instead of $args
we could use $data
.
A while ago a wrote an adapter for get_template_part()
that pass $data
variable to a View object, this is the implementation I made https://github.com/ItalyStrap/view/blob/master/functions/helpers.php
#107
@
4 years ago
@johnbillion am I understanding the code wrong, because it looks to me that $args needs to be passed explicitly. And if you're using your own custom global $args
in your foo.php template part, then that will still overwrite what's passed, preserving back-compat.
Meaning:
<?php //foo.php global $args; // [4,5,6] echo $args[0];
<?php $args = [ 1, 2, 3 ]; get_template_part( 'foo'); // echoes 4. get_template_part( 'foo', $args ); // still echoes 4.
at least that's the behavior i get when I apply the diff...
#108
@
4 years ago
Before the patch, the only way to see $args
inside the template is to add global $args
inside template.
After the patch and with global
statement, you still continue to have access to original $args
, not to the empty array nor the third argument of get_template_part()
.
To be honest, in all other cases where template doesn't use global $args
, the only back compat issue by using a too generic variable name (like $args
could be), could be a conflict with a variable with the same name used in template that has been locally scoped in other way (i.e. using include( locate_template() ) pattern
) or it has been already instantiated/used in template code.
But IMHO this is a remote case and it will happen only if you will update your get_template_part()
occurrences with the third argument.
That's why initially I suggested to use $wp_
prefix (i.e. $wp_template_args
).
PS: @justlevine the correct code of the 3rd line should be:
get_template_part( 'foo', null, $args );
#109
@
4 years ago
It looks like there is enough consensus to try this during 5.5 beta and see if any adjustments are needed. Let's do that :)
This is a ticket split off of #21673 to make it more viable for inclusion.