Make WordPress Core

Opened 5 years ago

Last modified 20 months ago

#52386 new enhancement

Should WP_Query::get_posts be a private method?

Reported by: rebasaurus's profile rebasaurus Owned by:
Milestone: Awaiting Review Priority: normal
Severity: normal Version:
Component: Query Keywords:
Focuses: performance Cc:

Description

Sometimes, users call WP_Query::get_posts() incorrectly, as it is a publicly accessible method:

<?php
$args = [
// do args here
];
$my_query = new WP_Query( $args );
$my_posts = $my_query->get_posts();

Unfortunately, this ends up doing a double query to the DB, as the WP_Query::get_posts() is mistaken for a getter when used outside the scope of the constructor. The ideal way to efficiently query would be:

<?php
$args = [
// do args here
];
$my_query = new WP_Query( $args );
$my_posts = $my_query->posts;

...Or in the case where no arguments are directly passed into a new WP_Query:

<?php
$args = [
// do args here
];
$my_query = new WP_Query;
$my_posts = $my_query->query( $args );

I just want to preface that I actually don't have the historical context behind why WP_Query::get_posts() remains a public method (whether it's left for backwards-compatibility reasons or whatnot).

@jrf suggests a potential approach to this in https://github.com/WordPress/WordPress-Coding-Standards/issues/1860#issuecomment-768296206:

it may be more appropriate to see if this can be solved in WP Core by checking if the $query is the same as previously used and if so, short-circuiting the function to return the previously retrieved results as saved in the $posts property.

Would love to see some discussion around this if it can get resolved within WP Core.

Thanks!

Change History (2)

#1 @rebasaurus
5 years ago

  • Type changed from defect (bug) to enhancement

#2 @oglekler
20 months ago

This method cannot be made private because it will break a lot of themes and plugins actually leading to a disaster, but I believe that there is an easy fix — this method is not getting any arguments, so, there is no way that the result can be changed (apart from a possibility that before WP_Query() object creation and this method call some data will be changed by something else, but I think it should be fine not to count in most cases this method call is the next line after object initialization. And $posts property initially is equal to null, so, if $this->posts is an array, even an empty one, we can safely return this array on the spot without processing the query again.

@costdev what do you think? 🙏

Version 0, edited 20 months ago by oglekler (next)
Note: See TracTickets for help on using tickets.