WordPress.org

Make WordPress Core

Opened 4 years ago

Last modified 14 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: minor 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 14 months ago.

Download all attachments as: .zip

Change History (12)

comment:1 @scribu4 years ago

  • Cc scribu added

comment:2 @johnbillion2 years ago

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

comment:3 @kovshenin2 years ago

  • Cc kovshenin added

comment:4 follow-up: @wonderboymusic2 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.

comment:5 @DrewAPicture2 years ago

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

comment:6 follow-up: @markoheijnen2 years ago

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

comment:7 in reply to: ↑ 4 @johnbillion2 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...

comment:8 in reply to: ↑ 6 ; follow-ups: @nacin23 months 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.

comment:9 in reply to: ↑ 8 ; follow-up: @rmccue23 months 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.

@SergeyBiryukov14 months ago

comment:10 in reply to: ↑ 9 @SergeyBiryukov14 months 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.

comment:11 in reply to: ↑ 8 @SergeyBiryukov14 months 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.

Note: See TracTickets for help on using tickets.