Ensure main query loop is entered for singular content in block themes

[felixarntz]

While initially it seemed that this had some overlap with WordPress/gutenberg#49904, a closer review of the underlying issues determined this problem being separate: The other PR addresses problems with the main query loop in general, while this PR and the below ticket only relate to singular templates in block themes and that they commonly lack the necessary blocks core/query and core/post-template which are used to start the main query loop.

This PR ensures that the main query loop is still started for block theme templates that lack those blocks. While alternatively we could consider introducing a new "invisible" wrapper block for this purpose, that would then require pretty much all block themes out there to adjust their templates, needless to say that any templates already modified in the database would not receive those updates. So while the fix in this PR is a workaround, it is reliable as there is by definition only a single post in the main query for singular content and thus we can wrap the entire template into the main query loop.

Trac ticket: https://core.trac.wordpress.org/ticket/58154

---

This Pull Request is for code review only. Please keep all other discussion in the Trac ticket. Do not merge this Pull Request. See GitHub Pull Requests for Code Review in the Core Handbook for more details.

[youknowriad]

This looks good to me. Is this dependent on the Gutenberg fix? Should we be waiting for the package update first before committing?

[felixarntz]

Thanks @youknowriad!

This looks good to me. Is this dependent on the Gutenberg fix? Should we be waiting for the package update first before committing?

No, in fact it would be better to first commit this one before backporting the other Gutenberg fix, see https://core.trac.wordpress.org/ticket/58154#comment:15.

[joemcgill]

This change makes sense to me. Left some questions inline as more of a thinking exercise than something that I think necessarily needs to be addressed.

[felixarntz]

@joemcgill Based on your other comment, what do you think about adding this? IMO it would make this code a bit safer, as it would ensure that, if something weird is going on and the main query was tampered with in a way that would change the number of posts in the result, we would fall back to the old code (else clause).

Suggested change

	if ( is_singular() ) {
	global $wp_query;
	if ( is_singular() && 1 === $wp_query->post_count && have_posts() ) {

[joemcgill]

I do think this is probably safer, but pretty unclear what's going on here unless someone does a git blame and finds this conversation (👋🏻 hi future person, if so!). Can you add an inline comment explaining why this is here and why you need to check the post_count?

[felixarntz]

@joemcgill Updated in 766d453.

[gziolo]

I did quite extensive testing using the Gutenberg plugin version downloaded from the linked PR:

https://github.com/WordPress/gutenberg/actions/runs/6003053680

https://github.com/WordPress/gutenberg/suites/15571281198/artifacts/888604674

I identified some edge cases in the block-based theme that I believe we saw together with @felixarntz last week during WC US:

Homepage rendered on the front

Everything works here as expected – "Third post with loop" renders the same loop as the one on the homepage and it correctly skips rendering the post content, which would lead to infinite loop.

Blog Home template in the editor

For some reasons, the nested query loop doesn't get displayed, but that might be good in this context.

Third post with loop on the frontend

The debugging message is only visible when WP is in the debug mode. More importantly, the query loop isn't correctly rendered.

Third post with loop in the editor

Everything works as expected in the editor though.

---

One more thing that I found surprising is that the featured image from the first post gets rendered on top of the homepage:

Edit: Ok, it's expected as it comes from the custom action included in testing instructions 😅 That would mean everything works correctly.

The same issue with the nested Query Loop exists on the post page though as in the block theme.

The homepage on the front doesn't render the nested query loop for the thirs post, too:

It sounds reasonable approach, but differs from how it is handled in the block theme. Maybe it uses the post excerpt though? 😅

[felixarntz]

Thank you for the detailed testing @gziolo! Some follow up feedback on your notes:

• I think your observations on the "third post with loop" aren't quite right: on the frontend, when viewed as the singular post in TT3 theme, its query loop only displays itself, which is correct, since the global query loop on that URL is only that post. I don't think there's any real use-case for doing that anyway, but it's working as it should. If anything, that part is off in the editor, where it should also display just that post, not the list of all posts. It should only display all the posts when looking at the post in an archive. Basically what is displayed in that post will differ depending on the current main query loop.
• Regarding your observation in the TT1 theme, as you've already suspected, in the archive view the loop inside "third post with loop" is not displayed as that template is only using the excerpt where any "rich content" is cut.

[gziolo]

@felixarntz, appreciate the detailed explanation to what I’ve incorrectly reported. I believe, it warrants at least a follow up issue in the Gutenberg repo to consider some UI improvements in the editor.

[felixarntz]

Thanks @gziolo. Based on the findings, I also left an update on WordPress/gutenberg#50790: I don't think it makes sense that we even allow inheriting the main query inside a specific post.

[mukeshpanchal27]

Thanks @felixarntz for PR.

#5104 (comment) LGTM. When you apply those changes please combine global variables.

global $_wp_current_template_content, $wp_embed, $wp_query;

[joemcgill]

A new concern I have after thinking about this some more, is that it essentially forces all singular block templates to include the entire template in "the loop", which can cause inconsistent behavior for things like headers or other template parts that are traditionally outside the loop. For example, if we have some logic that attempts to apply some image optimizations only when the image is inside the loop, with this change, a header image would be included on singular pages but not on list pages like archives. This is very edge case, but could be confusing. Would it be safer to only apply this logic to the full template if we are able to first do a has_block() check to see if a theme has omitted one of the blocks that would start the loop as expected?

[felixarntz]

@joemcgill Your concern is a fair point, and I've considered that initially when I started this PR.

Would it be safer to only apply this logic to the full template if we are able to first do a has_block() check to see if a theme has omitted one of the blocks that would start the loop as expected?

I did have that in here at the start (also see #5104 (comment)), but there are several limitations with that:

• There is no existing core block that would realistically be used on a singular template to trigger the main query loop. I thought originally that the core/query and core/post-template loop would also be usable for that purpose, but they unfortunately include additional markup forcing posts to be in a semantic list, which makes no sense for singular content and can be problematic for accessibility.
  • Figuring this out would require notably more discussion and changes. For example, would we introduce a new block for singular loops? Or somehow adjust the core/post-template block to conditionally not have wrapping markup? Those things may end up quite confusing from an end user perspective too.
• Even if there were singular-friendly variant of these blocks, we would need something more complex than has_block(), because we would only not need to trigger the main query if the core/post-template block was wrapped in a core/query block that had the inherit: true attribute set - something more costly to check for which there's no dedicated function available.

While what you're raising is indeed a potential concern, I see it more as a theoretical concern. While typically themes indeed only have the loop around the main content of a singular URL, I can't think of any actual problem that wrapping the whole content of such a URL would cause.

In other words: I think this is something worth revisiting in the future, but I don't consider it a blocker, since the current issue is certainly more severe, and properly fixing the additional concern would require more discussion and effort than what we could realistically tackle for 6.4.

[costdev]

Thanks for the PR @felixarntz! And for adding test coverage after our discussion in Slack. This looks good to me.

If the edge case mentioned above is a very rare and minor one, then I agree we should address it as soon as possible, and that it shouldn't be a blocker for this PR.

[felixarntz]

Committed in https://core.trac.wordpress.org/changeset/56507