Hello Nigel, I was trying to craft a POC for the WC Memberships Support, because if the issue is "they do it wrong" then it should not require a paid Plugin to trigger the issue (toolset in this case).
Doing so I struggled. I am not convinced the issue is only on them, but rather on the plugin calling the tags.
See below what I found.
If we add a very simple code to our Theme that simply checks if is_home, is_search or is_archive on pre_get_posts, and enable the WC + WC Membership plugins the error won't happen at all.
function hook_into_pre_get_posts_with_template_tag( $query ) {
if(is_search() || is_home() || is_archive()){
error_log( 'Is home, or search or archive' );
}
else{
error_log( 'anything else' );
}
}
add_action( 'pre_get_posts', 'hook_into_pre_get_posts_with_template_tag' );
When we call for example site.com/?s=anything, with this code in the theme, we get the proper Debug log (Is home, or search or archive) and no error.
We could even (but it is not necessary to) check "if ( is_admin() || !$query->is_main_query() ) {" which is pretty much what WP Core "doing it wrong" notice suggests in this case (to check if the query is ready), as far I understand.
But again, it is not needed. The error is not happening with this simple code.
So this brought me to the idea of trying this in a plugin, because since those are loaded in a different hook than a theme file, it could be happening there - and indeed.
As soon you call the exact same code in a plugin the error will happen!
And it is solved by adding this line of code which is what WP suggests to do anyway, and as far I see, Toolset does not do it:
if ( is_admin() || !$query->is_main_query() ) {
return;
}
This resolved the issue for me, and my code still works.
I actually had a plugin lying around that I made myself and it uses another hook (parse_query) and I run into the precise same error lol...
So I fixed it with adding the check if is_admin and if is_main_query and it was solved and the logic is still working, because logically, it can and will fire as soon the query is ready anyway. And that is all we need.
So I think toolset has to review the code where it hooks into pre_get_posts without checking if the query is actually ready.
This is also what the WP Core message indeed suggests under the hood, without explicitly saying it, they say "it may not be ready so you need to check". The 'location' of pre_get_query or other query hooks are not mandatory.
To summarise the POC is:
1. Create a simple WP Plugin
2. In the main plugin file or any class of the plugin call a simple function which checks on any tag such as is_home, is_search, is_archive
3. Hook that function to either pre_get_posts, or parse_query, or similar
4. Activate WC and WC membership
5. Load home or any other page and see the error logged
6. Edit your Function to check if ( is_admin() || !$query->is_main_query() )
7. See the function still works but the error is gone.
The issue has to be fixed in Toolset as far I see it.
Please don't hesitate to correct me if wrong, but above POC pretty much shows that it is on "us" and not "them" ("us" because as said I had a plugin that run into the precise same issue when used tgt with their WC Membership, and there are several reports to other plugins in forums about this as well)
It may be unfortunate that WC Membership is doing what it is doing, but on the other hand it is on us to craft code as safe as possible.
Thanks.
