WordPress.org

Make WordPress Core

Opened 5 years ago

Last modified 2 months ago

#19631 reopened enhancement

Consider a doing_it_wrong call for query_posts()

Reported by: johnbillion Owned by:
Milestone: Future Release Priority: normal
Severity: normal Version:
Component: Query Keywords: has-patch needs-docs
Focuses: Cc:

Description

The query_posts() function affects the main query, so things (like template conditionals) can break if plugins or themes use it without calling wp_reset_query() immediately afterwards.

Unless there is a situation I haven't considered, query_posts() shouldn't be used at all on requests where the main query is run (query_posts() of course has a use in situations like AJAX calls when the main query isn't run).

  • For secondary loops, get_posts() or the WP_Query() class should be used.
  • To modify the main query, there are hooks all over the place, such as 'request', 'parse_request', 'parse_query', and 'pre_get_posts', in addition to the SQL filters such as 'posts_where', 'posts_join', etc.

Maybe doing_it_wrong() should be called if query_posts() is used after the main query is run, with pointers toward a relevant Codex article on secondary loops or filtering the main loop.

Attachments (1)

19631.patch (960 bytes) - added by SergeyBiryukov 2 years ago.

Download all attachments as: .zip

Change History (14)

#1 @scribu
5 years ago

  • Cc scribu added

#2 @johnbillion
3 years ago

kovshenin just released a plugin for this: http://wordpress.org/plugins/stop-query-posts/

#3 @kovshenin
3 years ago

  • Cc kovshenin added

#4 follow-up: @wonderboymusic
3 years ago

  • Keywords close added
  • Milestone Awaiting Review deleted

I am going to suggest close on this - there are a lot of people who haven't seen @nacin's talk and don't know it is evil. For instance, Dealbook uses query_posts() 4 times on the homepage. Would I like to refactor all of that code? YES, but it is not in the cards for me to do so right now. I'd rather the logs not be flooded.

#5 @DrewAPicture
3 years ago

  • Resolution set to wontfix
  • Status changed from new to closed

#6 follow-up: @markoheijnen
3 years ago

Why not just deprecate it? We hate it and we think people shouldn't not use it. So why keeping it?

#7 in reply to: ↑ 4 @johnbillion
3 years ago

Replying to wonderboymusic:

there are a lot of people who haven't seen @nacin's talk and don't know it is evil. For instance, Dealbook uses query_posts() 4 times on the homepage.

That's the entire reason for adding the doing_it_wrong() call...

#8 in reply to: ↑ 6 ; follow-ups: @nacin
3 years ago

  • Milestone set to Future Release
  • Resolution wontfix deleted
  • Status changed from closed to reopened

Replying to markoheijnen:

Why not just deprecate it? We hate it and we think people shouldn't not use it. So why keeping it?

Because it is really easy to use and the alternatives are tougher to articulate and code, especially to people just hacking together a theme.

Re-opening for consideration down the line. I've been thinking about a new pre_main_query hook that parallels pre_get_posts but only runs $query->is_main_query() and ! $query->is_admin(). That might take some of the magic out of using pre_get_posts.

#9 in reply to: ↑ 8 ; follow-up: @rmccue
3 years ago

Replying to nacin:

Because it is really easy to use and the alternatives are tougher to articulate and code, especially to people just hacking together a theme.

Re-opening for consideration down the line. I've been thinking about a new pre_main_query hook that parallels pre_get_posts but only runs $query->is_main_query() and ! $query->is_admin(). That might take some of the magic out of using pre_get_posts.

Along those lines, maybe we could have something like:

function wp_modify_query($args) {
    add_filter('pre_get_posts', function ($query) use ($args) {
        return array_merge($query, $args);
    });
}

(Obviously with 5.2 syntax, this is just for demonstration)

This would work similarly to how we do admin menu items by hiding the fact that we have a filter at all, which might be a bit easier for themers to use than the hook, and they can use this directly in their functions.php. (And maybe a corollary function as wp_modify_main_query.) Just a thought if we do want to push away from query_posts() but still keep it simple for themers.

#10 in reply to: ↑ 9 @SergeyBiryukov
2 years ago

Replying to rmccue:

Along those lines, maybe we could have something like:

function wp_modify_query($args) {
    add_filter('pre_get_posts', function ($query) use ($args) {
        return array_merge($query, $args);
    });
}

(Obviously with 5.2 syntax, this is just for demonstration)

Looks like this function would have a very limited use, since there's no way to use conditional tags with it.

#11 in reply to: ↑ 8 @SergeyBiryukov
2 years ago

  • Keywords has-patch needs-docs added; close removed

Replying to nacin:

I've been thinking about a new pre_main_query hook that parallels pre_get_posts but only runs $query->is_main_query() and ! $query->is_admin(). That might take some of the magic out of using pre_get_posts.

Let's do this.

Attached a patch for consideration. The docs might need some work, I've just copied them from pre_get_posts.

#12 @chriscct7
9 months ago

  • Severity changed from minor to normal

#13 @SergeyBiryukov
2 months ago

#36874 was marked as a duplicate.

Note: See TracTickets for help on using tickets.