Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

passing an Args instance to WP_Query without toArray() silently yields subtle bugs #40

Closed
montchr opened this issue Jul 8, 2024 · 3 comments
Milestone

Comments

@montchr
Copy link

montchr commented Jul 8, 2024

I spent the past few hours trying to figure out why two nearly-identical queries in my project yielded wildly different results. Both used the same tax_query->clauses, but only one of the resulting queries actually respected the clause. It turns out that I had forgotten to call toArray() on the instance of \Args\WP_Query prior to passing it to the \WP_Query constructor. I would have expected that to cause an obvious error.

The query with new WP_Query($args->toArray()) correctly filtered posts by tax_query, while the query with new WP_Query($args) did not respect tax_query but also did not fail entirely.

@johnbillion
Copy link
Owner

Thanks for the report.

WordPress core is very inconsistent with how it handles these array arguments. I had hoped that with args I could construct an object which could be passed directly to parameters in WordPress which expect an associative array, but this isn't possible because of guard conditions such as is_array() in some functions. The underlying issue is that the arguments must be an array, hence the toArray() method. An object that implements ArrayAccess isn't enough.

A while ago I was thinking about removing the array-like behaviour of the args classes. That should mean that in this situation you get an immediate fatal error rather than a mysterious bug. I'll look into it.

@johnbillion
Copy link
Owner

I think that was a roundabout way of saying that implementing ArrayAccess, Countable, and IteratorAggregate on the base args class was a mistake.

@montchr
Copy link
Author

montchr commented Jul 9, 2024

That makes perfect sense. Thank you!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants