query_posts()
While the clock has started ticking on the 3.5 release grace period for Guidelines updates, I would like to discuss formalizing one other _doing_it_wrong() issue that has been implicitly enforced for some time now: use of query_posts().
Would anyone be opposed if we formally added query_posts() to Theme Check as a required stopper?
To understand why query_posts() is always inherently _doing_it_wrong(), see @nacin‘s WordCamp presentation You Don’t Know Query, and this great WordPress StackExchange answer by @rarst.
In short, if you are creating secondary queries, you should use WP_Query(), and if you’re modifying the main query, you should filter pre_get_posts.
Thoughts?
Hirak Santra, 
Andrew Nacin 3:23 pm on December 14, 2012 Permalink |
I’d say recommended, until WordPress itself declares that query_posts() is`_doing_it_wrong().
Chip Bennett 3:25 pm on December 14, 2012 Permalink |
What would the benefit be of making it only *recommended*?
Whether core flags the function itself as _doing_it_wrong() or not, it is always a bad practice, leads to unexpected results, and causes problems for the end user.
scribu 3:37 pm on December 14, 2012 Permalink |
Using the ‘pre_get_posts’ filter incorrectly can also lead to unexpected results and cause problems for the end user.
It’s actually easier to use query_posts() correctly than to add all the appropriate checks to a callback hooked into ‘pre_get_posts’.
Daniel 3:40 pm on December 14, 2012 Permalink |
This is a good point. The ‘request’ filter was built for the specific reason to alter the main query and the main query only; only problem is its implementation is still not complete. I propose we don’t push this as required until we are sure we have a bulletproof workaround.
Chip Bennett 3:53 pm on December 14, 2012 Permalink
$query->is_main_query()works as intended…Chip Bennett 3:52 pm on December 14, 2012 Permalink |
True. But then, so can calling
wp_nav_menu()incorrectly (e.g. by passing ‘menu’ instead of ‘template_location’). But we don’t allow Themes to usewp_list_pages()/wp_page_menu()/wp_list_categories()in place ofwp_nav_menu(). Instead, we educate about proper usage ofwp_nav_menu().Konstantin Kovshenin 7:04 pm on December 14, 2012 Permalink |
That’s so true!
Konstantin Kovshenin 3:25 pm on December 14, 2012 Permalink |
I’d go for recommended too, but huge +1 on the initiative!
Chip Bennett 3:28 pm on December 14, 2012 Permalink |
My problem with *recommended* is that, when query_posts() is used, it is almost *never* used *properly*. So, in order to pass the review process, the reviewer will have to help the developer rehabilitate the use of query_posts(), instead of simply requiring the cleaner, proper implementation via WP_Query or pre_get_posts.
Instead of teaching developers how to rehabilitate query_posts() usage, wouldn’t it be a far better use of our time to teach *proper* usage of best-practice methods?
Maor Chasen 8:29 am on December 21, 2012 Permalink |
I agree. I think that for some developers it will take a while to realize that
query_posts()is not the correct method, and if we don’t make it required now, then by the time it is being marked as_doing_it_wrong()in core — developers will still use it. IMHO, there are things that you don’t postpone, and this one is among them. Because if we don’t, it’s not just that we’re not teaching developers how they *should* do it, but end users might suffer from endless bugs that will be directly caused by the misuse ofquery_posts().Daniel 3:29 pm on December 14, 2012 Permalink |
The only problem I see is with pagination. Functions like next_posts_link() and previous_posts_link() work directly with the $wp_query and $paged globals so to get them to work one needs to alter the main query.
Konstantin Kovshenin 3:31 pm on December 14, 2012 Permalink |
That’s easy, just delete 404.php and pagination will work again, haha! How about wasted resources?
Daniel 3:32 pm on December 14, 2012 Permalink |
Is this a joke? 404 HTTP status is triggered in the page headers before templates are loaded.
Konstantin Kovshenin 3:35 pm on December 14, 2012 Permalink |
Of course it’s a joke, while I’ve seen a few people do it. You should never, never, never use query_posts. Period.
scribu 3:40 pm on December 14, 2012 Permalink
You should never, never, never use query_posts(), unless there’s no better way to do it. That’s why it’s not deprecated in Core, yet.
Chip Bennett 4:01 pm on December 14, 2012 Permalink
@scribu: examples of cases where there is no better way to do something than via
query_posts()?That’s the paradigm shift I’m struggling with, here.
scribu 4:10 pm on December 14, 2012 Permalink
The exact use-case that query_posts() was designed for: using a custom page template as an archive of posts:
Just call query_posts() at the very beginning and pretend it’s an arhive. query_posts() takes care of completely replacing the main query and the effect is limited to that custom page template.
You don’t have to do any workarounds for conditional flags or for pagination.
scribu 4:11 pm on December 14, 2012 Permalink
Chip Bennett 4:12 pm on December 14, 2012 Permalink
So, pagination works “out of the box” on a custom page template where the default query is stomped by
query_posts()?Chip Bennett 4:18 pm on December 14, 2012 Permalink
Also, I note that even you say that using
WP_Query()is the better method for creating secondary loops:nofearinc 6:05 pm on December 14, 2012 Permalink
what’s the preferred way to handle pagination though? It’s indeed using the $wp_query global inside of the pagination definition and passing arguments from the outside is a bit odd. There is no valid way to overload the query variable used for the pagination functions.
Chip Bennett 3:34 pm on December 14, 2012 Permalink |
Isn’t that an argument in *favor* of using
pre_get_postsinstead ofquery_posts()? If you alter the main loop query *before* the query is set up, pagination just works.Daniel 3:37 pm on December 14, 2012 Permalink |
That works for most cases with one exception I’m aware of: custom page templates. If you want to display a custom loop in a page template you need the main loop unaltered before the template is loaded to make sure the correct one is loaded. Otherwise the function is_page_template( ‘some-template.php ) may not return the correct value.
Chip Bennett 3:49 pm on December 14, 2012 Permalink |
If you want to display a custom loop in a custom page template, then instantiate a custom query, via
new WP_Query().Pagination still requires a bit of a work-around, but I would contend that work-around is far less intrusive than what is required to ensure that the main loop query is replaced cleanly.
Daniel 4:14 pm on December 14, 2012 Permalink
The
paginate_links()function could be used which accepts custom pagination arguments, and simple prev/next links could be generated by setting all numbered pagination values to 0; though I see that more as a hack rather than a workaround and I wouldn’t like to see it as recommended.Daniel 4:17 pm on December 14, 2012 Permalink
What I would like to see is functions like
next_posts_link()andprevious_posts_link()accept query objects as optional parameters. I believe that would be a huge enhancement in core.This post about using query posts kinda surprised… « GlacierAgWeb 3:34 pm on December 14, 2012 Permalink |
[...] post about using query_posts kinda surprised me: http://make.wordpress.org/themes/2012/12/14/query_posts/ . It’s a fairly commonly used WP function, so check it out if you work with [...]
Edward Caissie 3:36 pm on December 14, 2012 Permalink |
I’m not seeing ‘query_posts’ currently mentioned in the guidelines and if that is the case and although use-cases would be rare and still not likely best practice I would say we proceed in our standard method of first RECOMMENDED NOT to use then proceed to REQUIRED NOT to use.
Chip Bennett 3:47 pm on December 14, 2012 Permalink |
No, query_posts() is not explicitly listed in the Guidelines. But then again, neither are dozens of other functions that we exclude, based on this principle (and similar principles expressed in the guidelines):
Whether implementing required, recommended, or optional features, Themes are required to support proper WordPress core implementation of all included features.
My contention is:
1. Creating secondary loop queries is properly implemented via
new WP_Query()2. Modifying the main loop query is properly implemented via filtering
pre_get_posts.scribu 4:15 pm on December 14, 2012 Permalink |
I agree with 1.
I only partially agree with 2. You can do it, but it’s very easy to mess up, when using pre_get_posts, even if you use is_main_query().
Chip Bennett 4:20 pm on December 14, 2012 Permalink |
But what are the vast majority of use cases for modifying the main query in publicly distributed Themes? Changing
posts_per_page? Excluding a special/featured category? Something similar? Those are all straight-forward and easy in apre_get_postscallback.Sergey Biryukov 6:46 am on December 15, 2012 Permalink
FWIW,
$wp_query->is_main_query()returns true in the admin as well.So, if a theme exludes a category or alters
posts_per_pageusing apre_get_postscallback, it should also include anis_admin()check to prevent unintended side effects in the admin.Frumph 5:00 pm on December 14, 2012 Permalink |
query_posts is a valid and acceptable method of adding additional loops on your home page; unless something has changed recently that I am not reading here
Frumph 5:05 pm on December 14, 2012 Permalink |
rather use query_posts with a wp_query_reset then get_posts with a foreach loop anyday
Justin Tadlock 5:36 pm on December 14, 2012 Permalink |
Additional loops should be made with a new
WP_Queryon any page.Justin Tadlock 5:35 pm on December 14, 2012 Permalink |
query_posts()should only be used in custom page templates designed to be an archive-type page of posts. That’s the only use case I can think of at the moment.Most themes shouldn’t have a need to touch the main query anyway. There are some valid reasons for themes to do so, but this is generally plugin territory.
Konstantin Kovshenin 5:36 pm on December 14, 2012 Permalink |
new WP_Query() + the_post() + wp_reset_postdata() is a good alternative.
Ryan Duff 5:58 pm on December 14, 2012 Permalink |
Except in cases you intentionally want to stop on $post. It’s an extremely fringe case though.
Ryan Duff 6:18 pm on December 14, 2012 Permalink |
I’d like to expound on this after I read up to the top… and say that this fringe case is something I’d never see happening in a public theme.
Andrew Nacin 8:16 pm on December 17, 2012 Permalink |
No, it specifically works for that case. the_post() stomps $post. wp_reset_postdata() restors it.
Drew Jaynes (DrewAPicture) 6:00 pm on December 14, 2012 Permalink |
+Like
nofearinc 6:01 pm on December 14, 2012 Permalink |
+1 for that
Frumph 6:33 pm on December 14, 2012 Permalink |
My understanding is that new WP_Query() adds to the stack overhead while query_posts uses the internal post cache.. makes a big difference, unless i’m mistaken
Konstantin Kovshenin 6:36 pm on December 14, 2012 Permalink |
Both do the exact same thing, but in addition to that, query_posts() also breaks things and causes kittens to die.
Frumph 6:39 pm on December 14, 2012 Permalink
Hrm. I don’t like kittens dying. ..
Konstantin Kovshenin 6:42 pm on December 14, 2012 Permalink
Just to clarify, there is no overhead because query_posts() is just a wrapper function that calls new WP_Query() so there really is no difference between the two, except the kittens of course.
When you want to alter the main query, both new WP_Query() and query_posts() are overhead because they both fire secondary queries, when you wanted to alter the main query, in which case you should have used the ‘pre_get_posts’ action or the ‘request’ filter.
Frumph 6:47 pm on December 14, 2012 Permalink
..so the point in not using query_posts is what? if they’re both the same? .. I mean yeah if it’s the front page and the loop has already fired of course you should use pre_get_posts, but if you have the parser ‘reject’ use of query_posts if it was used as it should as secondary loops, then well .. what then?
Konstantin Kovshenin 6:58 pm on December 14, 2012 Permalink
Frumph: The point is that while query_posts() is just a wrapper for new WP_Query() it also does that in a way that breaks things, which is why avoiding query_posts() and using WP_Query() is safer.
However, correct me if I’m wrong, but the reason this was brought up is because query_posts() is used in many themes to replace the main loop, so we should educate developers towards pre_get_posts, not secondary loops.
Chip Bennett 7:04 pm on December 14, 2012 Permalink
Actually, secondary loops are exactly the wrong reason for using
query_posts(). Per the Codex:Konstantin Kovshenin 7:08 pm on December 14, 2012 Permalink
query_posts() does not alter the main loop. In fact, get_posts(), new WP_Query() and query_posts() do the exact some thing in slightly different ways, so IMO saying query_posts() should be used to alter the main query is completely wrong, because query_posts() is fired after the main query has already been made, unless it’s called in functions.php
Chip Bennett 7:12 pm on December 14, 2012 Permalink
Yes, it does: alter the main Loop – by replacing the query used to generate the data for that loop.
I think that’s probably a semantic difference. The Codex says “alter” (the main Loop); in reality, the correct term is “replace” (the main Loop query. The end result, however, is the same: what gets output in the main Loop has changed.
Konstantin Kovshenin 7:14 pm on December 14, 2012 Permalink
Exactly, which is why it’s so confusing and should be removed
Frumph 8:52 pm on December 14, 2012 Permalink
That should probably be notated over to the codex editors yeah? .. hey whatever happened to our ‘best practices’ page someone was supposed to be working on?
Chip Bennett 6:38 pm on December 14, 2012 Permalink |
Just pass:
'update_post_meta_cache' => false,
'update_post_term_cache' => false,
…as part of your argument array for
new WP_Query( $args )to eliminate that overhead.Andrew Nacin 8:19 pm on December 17, 2012 Permalink
No, definitely not. All query instances consult the same cache before deciding if additional calls to populate the cache is necessary.
The flags `update_post_meta_cache`, `update_post_term_cache`, and `cache_results` should be immediate red flags in a theme. Chances are, they are doing it wrong, and trying to avoid one query but generating a bunch in the process (because some operation they are doing does consult meta or term data, even if they don’t realize it).
Hirak Santra 6:59 am on December 24, 2012 Permalink |
query_posts() can be used not only for custom posts. It can also bring data of a respective page by page_id or page_name.
Hirak Santra 7:01 am on December 24, 2012 Permalink |
query_posts() can not only bring data of posts page but also it can bring data from a page by page_id or page_name.
Chris Olbekson 6:47 pm on December 14, 2012 Permalink |
I would agree with the others that `query_posts()` should only be used in a page template that wants to work as an archive. It shouldn’t be used in an archive, header, widget, sidebar etc..
Andrew Nacin 8:43 pm on December 17, 2012 Permalink |
I went out on the WordCamp “circuit” and educated developers on query_posts() in three different cities. This was a carefully calculated initiative designed to boost developer education and awareness. I specifically gave the talk at three veteran WordCamp cities: Portland on the west coast, New York on the east coast, and in Europe (Netherlands). And, the talks have gotten quite a bit of play online after. The impact has been large.
And yet, core still does not deprecate query_posts(), even though I added
is_main_query()to core while on stage at WordCamp Portland. Why, despite recommending against its use, do we not argue for its alternatives?My goal was to focus on education and awareness beyond
query_posts(). There are a lot of things WordPress developers do out of force of habit, without understanding what actually goes on under the hood. In the case ofquery_posts(), this was a particularly bad habit caused by lack of understanding.But,
pre_get_postsmay be the correct solution, but it isn’t a good one. It can break the admin. It can break secondary queries. And, it is not easy for developers to use. For a number of situations,query_posts()is sufficient, despite, in many of those situations, having edge cases. We led with a carrot, but if it tastes awful, is it really fair to continue with the stick? I my book, edge cases where it may not be perfect trumps side effects that break other things.I like to think I am very pragmatic when it comes to core’s development. I subscribe to “why walk when you can run” almost as much as the next 20-something, but learning to recognize when to walk instead of running is a major key to my own evolution as a developer of a large open source project. It is often important to let the core development process work at a more deliberate speed.
So, I am often bearish on the adoption and evolution of newer APIs, and bullish on the deprecation of old ones. I could pushed the deprecation of
query_posts()at any time since I first gave the talk in September 2011. That timeline includes three major releases, including one I led. Yet, despite kicking off this education initiative, I haven’t. That, I think, should speak volumes. That is why the theme reviewers should follow core’s lead on this.Toru 5:13 am on March 25, 2013 Permalink |
I (or Japanese community while translating…) came a cross that on Codex’s http://codex.wordpress.org/Function_Reference/wp_reset_query
word “deprecated” is used.
I have not been able to follow much of the progress on this topic recently, but I believe Nacin’s above post was the final say for now.
Can I check that Am I correct? In which case I feel this is very misleading line in Codex.
Codex’s query_posts entry is however, I think very clear and in agreement with what has been discussed in this thread.
Thanks.