Opened 5 years ago
Last modified 20 months ago
#52386 new enhancement
Should WP_Query::get_posts be a private method?
| Reported by: |
|
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!
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? 🙏