WPTT / theme-sniffer

Theme Sniffer plugin using sniffs.
MIT License
269 stars 3 forks source link

setup_postdata() and global $post #177

Open dashkevych opened 5 years ago

dashkevych commented 5 years ago

It seems like setup_postdata() does not work correctly if we does not use a global $post.

For instance:

The output will be correct when using this:

foreach ( (array) $featured_posts as $post ) :
    setup_postdata( $post );
    get_template_part( 'content', 'featured-post' );
endforeach;

wp_reset_postdata();

The output will be not correct when using this:

foreach ( (array) $featured_posts as $featured_post ) :
    setup_postdata( $featured_post );
    get_template_part( 'content', 'featured-post' );
endforeach;

wp_reset_postdata();

Example of using setup_postdata() with $post: https://github.com/WordPress/WordPress/blob/master/wp-content/themes/twentyfourteen/featured-content.php#L23

When using $post, the displays the following error: "Overriding WordPress globals is prohibited. Found assignment to $post".

What would you recommend in this case to avoid the error? Are we allowed to use setup_postdata()?

dingo-d commented 5 years ago

The context matters in this case 🙂

In which file are you using the $post variable? If it's in one of the templates, then you can use it just fine. In other files (like functions.php), since they are in global namespace, you risk of tampering with the global $post variable, and potentially affecting other places where it's used in WordPress.

You may wonder why this doesn't work, but it's actually because of the get_template_part and setup_postdata functions. What get_template_part function does is it loads the template - tries to find it in different places before using locate_template which then uses load_template() to load the template while providing some globals:

    global $posts, $post, $wp_did_header, $wp_query, $wp_rewrite, $wpdb, $wp_version, $wp, $id, $comment, $user_ID;

Also the important part to note is the setup_postdata() function, that will set up global post data, won't override the global $post variable. For that to happen you would have to assign it to the $post variable which would trigger the message you've seen.

So, long story short: we currently have some issues with providing the context to determine if the 'globals' are actually global. When using templates, this can be ignored. In other places it's a legitimate concern.

TRT reviewers are aware of this, and they know when to ignore these issues.

joyously commented 5 years ago

If it's in one of the templates, then you can use it just fine. In other files (like functions.php), since they are in global namespace, you risk of tampering with the global $post variable

Be careful what you say here. Template files (single.php, archive.php, index.php, etc.) are in the global namespace. Look in template_loader.php and see that it just uses include ( $template ); from a file that isn't inside of any function. It's template-parts that are loaded from a function and so their variables are local to that template-part. (but that function defines the globals as stated, so you can't use those names)

dingo-d commented 5 years ago

I stand corrected 🙂