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

Query block: Add a way to inherit the global $query #25377

Closed
aristath opened this issue Sep 16, 2020 · 10 comments · Fixed by #27128
Closed

Query block: Add a way to inherit the global $query #25377

aristath opened this issue Sep 16, 2020 · 10 comments · Fixed by #27128
Assignees
Labels
[Block] Query Loop Affects the Query Loop Block [Feature] Themes Questions or issues with incorporating or styling blocks in a theme. [Status] In Progress Tracking issues with work in progress

Comments

@aristath
Copy link
Member

Discussed briefly in a meeting on Slack so creating a ticket for it.

The query block should have an option to inherit its options from the global $query. This will allow us to avoid creating a separate template in FSE for each category/tag etc, and instead, have a single template for all categories.
The blocks will still render the same way the user defines them, what changes is that if that query-block inherits the global query, it will get the posts from the context of the URL by inheriting the global $query.
Settings for posts-per-page, offset etc can still work, but not the filtering aspect of the block's settings.

@aristath aristath added [Feature] Themes Questions or issues with incorporating or styling blocks in a theme. [Feature] Full Site Editing [Block] Query Loop Affects the Query Loop Block labels Sep 16, 2020
@tomjn
Copy link
Contributor

tomjn commented Sep 24, 2020

imo this should be the default, otherwise every blog post on WP performance and quicktips from 2022 onwards will start with making sure you change your query blocks to do this

@mariohamann
Copy link

mariohamann commented Sep 25, 2020

In FSE there definetely shouldn't be any possibilities for filtering. The offset and pagination settings could have their reason (e. g. making the first post bigger than the others etc.) but as you described above, the loop would break completely with filters (especially with filters to select the Post Type).

After having read the Slack archive you linked and some thoughts I had yesterday, I see another big problem: As far as I understand (and as I described more detailed in #25607 (comment)) the Query Block uses the REST-Endpoint wp-json/wp/v2/posts to fetch the posts. That's fine for blog posts, and the default taxonomies categories and tags (see the docs).

BUT: At the moment I can't see any chance that the loop + Query Block + Custom Post Types work together. I want to explain it: Of course we can have the routes wp-json/wp/v2/name_of_post_type_route (if the developer decides to show the post type in REST, what could become an interesting security discussion, but let's ignore this at this point). But at the moment there is no chance (in core) to filter Custom Taxonomies.
→ The main loop for Custom Post Types could work but every secondary loop for Custom Taxonomies-Archives does not work from the current state of the REST API.

  • Example WooCommerce: It works, if the Query Block automatically switches to /wp-json/wc/v3/products. It even works for the default categories and tags as WC implemented those in the API. But I can't see any chance to make it work with custom taxonomies, e. g. brands as provided by those plugins: https://de.wordpress.org/plugins/search/woocommerce+brands/ (of course you can use WC attributes, but that doesn't fit every case)
  • Example Portfolios: Take the plugin Portfolio Post Type (>100.000 activate installations). The descriptions says it it registers it's own "portfolio taxonomies for tags and categories". That's best practive. But these tags and categories can't be used with the query block as it has no possibility to filter them.
  • Example Blog Posts: Sometimes even blog posts could have custom taxonomies, e. g. "Food Blogs": They could have taxonomies like "Meal Typ", "Dish Type", "Seasons", "Ingredients", etc. – they don't work either with Query Block and Taxonomy-Archives.

Maybe I still have a fundamental misconception of the Query Block and please (!) correct me if I'm wrong. :)

@aristath
Copy link
Member Author

In FSE there definetely shouldn't be any possibilities for filtering. The offset and pagination settings could have their reason (e. g. making the first post bigger than the others etc.) but as you described above, the loop would break completely with filters (especially with filters to select the Post Type).

I agree with this assessment... The query block should have an option to just inherit everything from the main query. If that option is enabled (which should be the default as mentioned by @tomjn above), then most options should be hidden.

Of course we can have the routes wp-json/wp/v2/name_of_post_type_route (if the developer decides to show the post type in REST, what could become an interesting security discussion, but let's ignore this at this point). But at the moment there is no chance (in core) to filter Custom Taxonomies.
→ The main loop for Custom Post Types could work but every secondary loop for Custom Taxonomies-Archives does not work from the current state of the REST API.

It's not that simple... Yes, the editor uses the REST API. But the frontend does not, it's a normal WP_Query when the block gets rendered on the front for visitors. So while we currently do have an issue with the block preview in the editor due to the issues mentioned above regarding the REST API, that will not impact end users (and by "end users" here I mean site visitors) or how the block is used.
Granted, the REST API should be improved to allow for better previews of the query block. But that doesn't diminish the need for a "Default" query option in the query-block.
The frontend does not - and will not - use the REST API.

@aristath
Copy link
Member Author

aristath commented Sep 25, 2020

As a sidenote, I just tried something extremely simple just to test how well it works.
Right above this line:

$posts = get_posts( $query );

I added this:

global $wp_query;
if ( $wp_query && isset( $wp_query->query_vars ) && is_array( $wp_query->query_vars ) ) {
	$query = $wp_query->query_vars;
}

In my FSE theme I only have an index.html template with a loop, and then a single.html template for single posts.

The result is that the query block then behaves properly and it shows categories, search results, blog indexes etc for everything when browsing the site.

Now, I'm not saying it will be as simple as pasting that code in the query block, it just goes to show that it is possible to inherit the global query without things falling apart.
Of course there will need to be a switch to toggle global query arguments inheritance on/off (maybe default to on), and then the arguments for the query will need to be merged between the global query and any overrides in the block.
But it is very possible to do... and it's the main piece missing so we can start building FSE themes.
Pinging @ntsekouras in this discussion since he's been doing a lot of work (excellent work too if I might add) in the query block, and also @jasmussen.

@ntsekouras
Copy link
Contributor

Hey all - Thank you for your explorations here! Much appreciated and needed for going forward. Also thank you @aristath for pinging (and you kind words) :).

First of all the Query block will have to inherit the global $query, as this is what the main issue here. I don't know all the technicalities yet, but it just needs to and will be implemented :).

In FSE there definetely shouldn't be any possibilities for filtering. The offset and pagination settings could have their reason (e. g. making the first post bigger than the others etc.) but as you described above, the loop would break completely with filters (especially with filters to select the Post Type).

I agree with this assessment... The query block should have an option to just inherit everything from the main query. If that option is enabled (which should be the default as mentioned by @tomjn above), then most options should be hidden.

Both @mariohamann and @aristath remarks seem right and from some early explorations here are some thoughts (implementation doesn't exist yet).

Of course there will need to be a switch to toggle global query arguments inheritance on/off (maybe default to on)

Fist of all as mentioned above we should find a way to understand the context we are at (ex archive-posts) and enable/disable filters and/or set/unset values of the Query block. Something that occurred to me is that maybe during on creation of Query block (or some other place) we should allow the user to select whether to inherit the detected context or not.

This should exist because we should not limit the user to have only on Query block. Simple use case would be to have a CPT archive page (like products) and on the bottom to show a list of posts. First one (products) should inherit $query, second one not. This raises other issues of course like, do we hide pagination of other Query blocks if one inherits global $query etc..

I think a big missing piece that I've started to explore is CPTs and Taxonomies. I know @mapk and @mariohamann work on this design wise as well.

Granted, the REST API should be improved to allow for better previews of the query block. But that doesn't diminish the need for a "Default" query option in the query-block.
The frontend does not - and will not - use the REST API.

Unfortunately for development speed's sake we need to enhance REST API even if it's not used on front-end. FSE has the goal to be as what you see is what you get, so I think we shouldn't build things out of that scope.

@mariohamann
Copy link

Thank you again, @ntsekouras for your insights and explanations!

This should exist because we should not limit the user to have only on Query block. Simple use case would be to have a CPT archive page (like products) and on the bottom to show a list of posts. First one (products) should inherit $query, second one not. This raises other issues of course like, do we hide pagination of other Query blocks if one inherits global $query etc.

Just a quick thought:

  • I think it could be a way to have only ONE single Query-Block (maybe even undeletable + not addable) on every archive, which uses pagination and inherits the args from context (therefore, no filters! :)). This FSE-only block would preserve the current behaviour of archives.
  • Everything else could be variants (Posts Query Block, Products Query Block), which wouldn't interfere with the main Query-Block on archive pages, could use filtering etc. and moreover could be added on the whole website. (A bit like described over here Provide API to create Query Block Variations #25607 (comment) – without the API part. ;)).

Looking forward to coming explorations! 👍🏻

@bobbingwide
Copy link
Contributor

In my experimental theme I've been working on an alternative approach to @aristath's #25377 (comment).
which satisfies @tomjn's requirement for the main query to be the default.

When I'm creating a template for category, tag, taxonomy, archive then I don't want to run the main query that has already been determined by WordPress. I just want to run the loop to render the inner blocks.
This can be achieved by using the wp:query-loop block directly, not within the wp:query block.

eg. My main-query template part might be:

<!-- wp:query-loop -->
<!-- wp:post-title /-->
<!-- wp:post-featured-image {"isLink":true} /-->
<!-- wp:post-date /-->
<!-- wp:post-excerpt /-->
<!-- /wp:query-loop -->

gutenberg_render_block_core_query_loop() can check for the value of $block->context['queryId'].
If it's set then we need to run the query from the wp:query block, else we run the main query.
Here I've implemented the main query function with the same prototype as the block's rendering function.

/**
 * Renders the `core/query-loop` block for the main query on the server.
 *
 * @param array    $attributes Block attributes.
 * @param string   $content    Block default content.
 * @param WP_Block $block      Block instance.
 *
 * @return string Returns the output of the query, structured using the layout defined by the block's inner blocks.
 */
function gutenberg_render_block_core_query_loop_main_query( $attributes, $content, $block ) {
    if ( have_posts() ) {
        $content = '';
        while ( have_posts() ) {
            the_post();
            $post = get_post();
            $content .= (
            new WP_Block(
                $block->parsed_block,
                array(
                    'postType' => $post->post_type,
                    'postId' => $post->ID,
                )
            )
            )->render(array('dynamic' => false));
        }
    } else {
        $content = __( "No posts found." );
    }
    return $content;
}

For more detail see bobbingwide/fizzie#11
I'll await your comments on this before creating a PR.

@gziolo
Copy link
Member

gziolo commented Nov 20, 2020

When I'm creating a template for category, tag, taxonomy, archive then I don't want to run the main query that has already been determined by WordPress. I just want to run the loop to render the inner blocks.
This can be achieved by using the wp:query-loop block directly, not within the wp:query block.

I'm wondering if it was an intended behavior or some sort of unexpected use case caused by the fact the Query Loop block can exist without the Query block. I personally find it confusing that you have a choice to insert Query, Query Loop and Query Pagination blocks in any place. To give an example, it's unclear what would Query Pagination do on its own on the single post page.

@bobbingwide
Copy link
Contributor

bobbingwide commented Nov 20, 2020

the fact that the Query Loop block can exist without the Query block.

Intended or not, without the capability to extend the query loop block, I would not have continued experimenting with FSE.
Assuming the query loop block is intended to replace the loop then it doesn't depend on the query block as for most requests WordPress has already determined the main query.

Both the query loop and pagination should work against the main query.

It's unclear what would Query Pagination do on its own on the single post page.

At present ( well, Gutenberg 9.3 ) the query pagination block does nothing on the front end and crashes in the editor.
See #26831 for where I've used it without the query block in the index.html template file.

My investigation led me to wonder what happens if a post contains something like this:

<!-- wp:query {"queryId":1,"query":{"perPage":2,"pages":1,"offset":0,"postType":"oik-plugins","categoryIds":[],"tagIds":[],"order":"desc","orderBy":"date","author":"","search":"","exclude":[1220],"sticky":""}} -->
<!-- wp:query-loop -->
<!-- wp:post-title {"level":5,"isLink":true} /-->
<!-- /wp:query-loop -->
<!-- wp:query-loop -->
<!-- wp:post-title /-->
<!-- /wp:query-loop -->
<!-- /wp:query -->

or, how I'd intended to try it.

<!-- wp:query {"queryId":1,"query":{"perPage":2,"pages":1,"offset":0,"postType":"oik-plugins","categoryIds":[],"tagIds":[],"order":"desc","orderBy":"date","author":"","search":"","exclude":[1220],"sticky":""}} -->
<!-- wp:query-loop -->
<!-- wp:post-title {"level":5,"isLink":true} /-->
<!-- /wp:query-loop -->
<!-- /wp:query -->

<!-- wp:query-loop -->
<!-- wp:post-title /-->
<!-- /wp:query-loop -->

The answer, in my development environment with this logic implemented is:

  • for the first example the second query loop uses the query block's query
  • for the second example it uses the main query.

This is what I would have expected.

@ntsekouras
Copy link
Contributor

I'm wondering if it was an intended behavior or some sort of unexpected use case caused by the fact the Query Loop block can exist without the Query block.

For context it was not allowed at first implementation: https://github.com/WordPress/gutenberg/pull/22199/files#diff-c1c5029a4cbcea0a6e41a1119a377d89f6ba86d683a9e654015a6b0f72103362R20. (I think this will be reverted and Pagination and Loop should only live under Query).

In some later PRs this changed in a first and not in depth attempt to support just that. To be able to use context, set in other ways (in compat.php). Specifically it changed when categories were added in Query block and categories where set in a different context through first iterations of FSE navigation. Since then, many changes have been made to FSE navigation (sidebar introduced), Query changed and these changes to context where not followed.

IMO this is okay because it wasn't a complete solution and I think it's better to decouple the context detection in site editor. A promising approach of this is here: #27128 and design is being discussed to make this work well.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
[Block] Query Loop Affects the Query Loop Block [Feature] Themes Questions or issues with incorporating or styling blocks in a theme. [Status] In Progress Tracking issues with work in progress
Projects
None yet
6 participants