Make WordPress Core

Opened 11 years ago

Closed 11 years ago

Last modified 10 years ago

#23329 closed enhancement (fixed)

Issue _doing_it_wrong() for is_main_query()

Reported by: ethitter's profile ethitter Owned by:
Milestone: 3.7 Priority: normal
Severity: normal Version: 3.3
Component: Query Keywords: has-patch
Focuses: Cc:

Description (last modified by SergeyBiryukov)

The function is_main_query() is misleading, and rather useless. In practical situations, it is necessary to reference the method in the current query object, such as when used at the pre_get_posts action.

The example in the Codex is even wrong:

  add_action( 'pre_get_posts', 'foo_modify_query_exclude_category' );

  function foo_modify_query_exclude_category( $query ) {
      if ( ! is_admin() && is_main_query() && ! $query->get( 'cat' ) )
          $query->set( 'cat', '-5' );

I propose deprecating the function as an encouragement for users to refer to the query object method instead.

Attachments (3)

23329.patch (971 bytes) - added by ethitter 11 years ago.
23329.2.patch (525 bytes) - added by ethitter 11 years ago.
23329.3.patch (595 bytes) - added by c3mdigital 11 years ago.

Download all attachments as: .zip

Change History (20)

#1 @SergeyBiryukov
11 years ago

  • Description modified (diff)

Also suggested in ticket:13961:29.

11 years ago

#2 @SergeyBiryukov
11 years ago

  • Keywords has-patch added
  • Milestone changed from Awaiting Review to 3.6

Corrected the Codex example.

#3 @nacin
11 years ago

Originally, used the function rather than the method. When I protested, JJJ came up with a fairly good explanation for why using the function actually made more sense in that situation. I disagreed, but it wasn't edited tat the time. It has since been changed.

I kick myself for introducing the function. I really do. It had its purpose, to be a compliment to a WP_Query method the same way a function like is_author() is.

It also does have a legitimate use. It means, simply: "Is $wp_query the main query?" This is false whenever query_posts() gets used. Given this function's purpose was to go up against query_posts(), it seemed like it could be useful. I didn't realize at the time how easily it'd be abused.

It is worth noting that we could actually detect, inside is_main_query(), if pre_get_posts is the current hook. We could even go so far — not easily, but it is possible — to forcibly call the method on the query object when the function is summoned inside pre_get_posts. That's going overboard, but still, it's possible.

I don't entirely see what benefit deprecating this function will have. Maybe just a _doing_it_wrong() when used inside pre_get_posts.

#4 @johnjamesjacoby
11 years ago

This is an okay function with a unique use case.

Like nacin noted, is_main_query() is explicitly intended to answer the question: "is the current global $wp_query object the one that WordPress created in the main query loop, or is it otherwise altered?"

People misusing it isn't the reason to deprecate it (I.E. query_posts()) -- neither is being able to use it out of order (I.E. current_user_can() before $wp->init()).

Better error-handing/developer-feedback when misusing functions is great, but is_main_query() isn't any worse than query_posts() in this regard.

If the day ever comes where we deprecate query_posts(), we would want to deprecate is_main_query() at the same time. Without query_posts() there's no natural way in WordPress core to stomp the main query.

#5 @toscho
11 years ago

  • Cc info@… added

#6 @sirzooro
11 years ago

  • Cc sirzooro added

#7 @alex-ye
11 years ago

  • Cc nashwan.doaqan@… added

11 years ago

#8 @ethitter
11 years ago

Okay, nacin and johnjamesjacoby made good points, so _doing_it_wrong() seems like a better idea to me.

I've added a new patch that reflects this, taking nacin's recommended approach of checking the current action; the wording could probably use some work.

#9 @ryan
11 years ago

  • Milestone changed from 3.6 to Future Release

#10 @wonderboymusic
11 years ago

  • Keywords needs-refresh added

11 years ago

#11 @c3mdigital
11 years ago

  • Keywords needs-refresh removed
  • Version set to 3.3

Changed the version and added suggestion to use the current query object method instead.

#12 @wonderboymusic
11 years ago

  • Milestone changed from Future Release to 3.7

#13 @nacin
11 years ago

  • Keywords needs-codex added
  • Summary changed from Deprecate is_main_query() to Issue _doing_it_wrong() for is_main_query()

This Codex page needs some work to actually explain what is going on. It documents only the function, yet is linked from the WP_Query page, and the only example is using the method.

I'm committing a _doing_it_wrong() message but this Codex page needs a lot of love in order to keep this in 3.7. Otherwise we're just telling users what they're doing wrong without giving them any information to help them correct it, which would be very lame.

#14 @nacin
11 years ago

In 25605:

When is_main_query() is called during the pre_get_posts hook, issue _doing_it_wrong().

The is_main_query() WP_Query method should be used in these instances instead.
see #23329.

#15 @nacin
11 years ago

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

Closing this out, thanks to the needs-codex keyword still being on here, and given

#16 @bcworkz
10 years ago

  • Keywords needs-codex removed

Veraxus fixed the Codex 25 Sep 13 and I added more clarification 1 Nov 13. I was unaware of this ticket at the time and I just happened to stumble upon it today.

Note: See TracTickets for help on using tickets.