Make WordPress Core

Opened 4 months ago

Last modified 2 months ago

#59131 assigned enhancement

Introduce filters to modify/inspect the blocks returned by `do_blocks()`

Reported by: luisherranz's profile luisherranz Owned by: luisherranz's profile luisherranz
Milestone: 6.5 Priority: normal
Severity: normal Version: 6.4
Component: Editor Keywords: has-patch has-unit-tests
Focuses: Cc:

Description

The do_blocks() function provides a way to process blocks including block parsing and their subsequent rendering. Currently, it is impossible for plugins (including Gutenberg) to inspect those values or customize that logic.

A couple of filters should be introduced that allow to modify or inspect the blocks before they are included in the final response:

  • The first one receives the array of parsed blocks returned by the parse_blocks function. This filter provides an easy way to inspect or even modify the blocks as a whole, and also gives the opportunity to know when a call to do_blocks has started.
  • The second one receives the final HTML rendered returned by the recursive call to render_block. This filter provides an easy way to inspect or even modify the final rendered output as whole, and also gives the opportunity to know when a call to do_blocks is about to finish.

With the combination of these two filters, it's also possible to ignore nested calls of do_blocks and avoid processing the HTML of blocks more than once, something that is not possible today using render_block alone.

Target is merging into 6.4.

Change History (35)

This ticket was mentioned in PR #5023 on WordPress/wordpress-develop by @luisherranz.


4 months ago
#1

  • Keywords has-patch added

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

#### Questions

  • Do I need to add tests for these filters?
  • Are the do_blocks_pre/post_render filter names ok?
  • @dmsnell: is this what you had in mind, or is there something missing?

@dmsnell commented on PR #5023:


4 months ago
#2

Do I need to add tests for these filters?

This would be nice. Check in phpunit/tests/blocks/render.php though I could easily see a valid use case for adding a new renderHooks.php test file. Something easy like filtering out image blocks or paragraphs containing Wordpress could suffice; just enough to show that the filter is called and works. Same for the _post_render filter.

@luisherranz commented on PR #5023:


4 months ago
#3

Thanks, Dennis!

I've added the tests and improved the comments. Let me know if there's something else that can be done.

#4 @gziolo
4 months ago

  • Keywords has-unit-tests added
  • Milestone changed from Awaiting Review to 6.4
  • Version set to trunk

#5 @gziolo
4 months ago

  • Owner set to gziolo
  • Status changed from new to assigned

@gziolo commented on PR #5023:


4 months ago
#6

Do I need to add tests for these filters?

Always, but I see you have it covered 😄

Are the do_blocks_pre/post_render filter names ok?

I don't think there is a hard rule for it, but the shorter the name, the better, as long as it expresses the intent. I usually look at existing names when trying to decide. Here I did a quick search at https://developer.wordpress.org/?s=pre_&post_type%5B%5D=wp-parser-hook:

https://i0.wp.com/github.com/WordPress/wordpress-develop/assets/699132/966ec585-affa-4c50-8591-8f30032cf33a
https://i0.wp.com/github.com/WordPress/wordpress-develop/assets/699132/1f4d8728-6cab-4f1e-8523-7f8fe1e89c96

There is `pre_render_block` in render_block called inside do_blocks, so you could as well follow and go with pre_do_blocks and post_do_blocks. I don't have strong preferences as the current naming used is also good.

@luisherranz commented on PR #5023:


4 months ago
#7

You could as well follow and go with pre_do_blocks and post_do_blocks.

I went with the extra "render" word because it leaves the door open for a future do_blocks_pre_parse filter where you can modify the $content before it gets parsed, but I'm not sure if that one makes sense, though.

@gziolo commented on PR #5023:


4 months ago
#8

You could as well follow and go with pre_do_blocks and post_do_blocks.

I went with the extra "render" word because it leaves the door open for a future do_blocks_pre_parse filter where you can modify the $content before it gets parsed, but I'm not sure if that one makes sense, though.

The name is fine, like I said before. We wouldn't need to introduce another filter unless the body changes drastically. You only need to pass the $content to achieve the same result:

$blocks = apply_filters( 'do_blocks_pre_render', $blocks, $content );

// usage
add_filter( 'do_blocks_pre_render', function( $content ) {
    return parse_blocks( my_plugin_modify_content( $content );
}, 10, 1 );

@luisherranz commented on PR #5023:


4 months ago
#9

Okay, so parse_blocks is fast enough and doesn't need to be optimized to run just once, right?

#10 @azaozz
4 months ago

  • Keywords 2nd-opinion added

Sorry but to me it seems that both of these filters are mostly useless considering that there already are pre_render_block, render_block_data, and render_block_context filters. Could you please provide some practical examples where and how they can be used?

In addition, looking at the proposed do_blocks_pre_render: it filters the output from WP_Block_Parser::parse(). However WP_Block_Parser::parse() is used in several places in core, for example in templates. If such filtering makes sense wouldn't it be better to filter the output from the parser in the actual parser? I.e. add a filter on the return value in WP_Block_Parser::parse() or in the parse_blocks() helper function.

Looking at the proposed do_blocks_post_render in do_blocks( $content ). How is that different than the_content filter?

Last edited 4 months ago by azaozz (previous) (diff)

@luisherranz commented on PR #5023:


3 months ago
#11

Ok, we won't add any additional arguments for the moment:

---

@gziolo, @dmsnell: is there anything else to do here?

@gziolo commented on PR #5023:


3 months ago
#12

@gziolo, @dmsnell: Is there anything else to do here?

I'd like to make sure you saw and responded to the comment that @azaozz left on WordPress Trac: https://core.trac.wordpress.org/ticket/59131#comment:10.

#13 @luisherranz
3 months ago

Hey @azaozz, thanks for the feedback 🙂

I should have given more context for the need for these filters in the description. Sorry about that.

the_content filters only the post content, whereas blocks are nowadays also used on the theme (templates and template parts) and other places where they are not filtered by the_content.

render_blocks filters are great, but they are limited in the order they run and the information they have: render_block_data and render_block_context are top-down and can only access the block instance information, not the final rendering. render_block has the final rendering, but it's bottom-up. The render is also difficult to inspect because each block contains not only its own render, but also the render of its inner blocks. The main need for these filters came from the Interactivity API directive processing, which needs to process things top-down and be able to separate the render of interactive blocks from their (non-interactive) inner-blocks. But according to @dmsnell, the need for this filter also arose during the work on footnotes (even though they finally took a different approach). This is also potentially useful for things like word counters, advertising placement and so on.

All in all, we don't have a good way to hook into the start and end of the blocks rendering, and these filters would provide a convenient way to modify blocks from a global perspective without having to resort to hacky and prone-to-error implementations.

@luisherranz commented on PR #5023:


3 months ago
#14

I'd like to make sure you saw and responded to the comment that azaozz left on WordPress Trac

Sorry, I somehow missed that. I answered now 🙂

@azaozz commented on PR #5023:


3 months ago
#15

Sorry, I somehow missed that.

Uh, sorry. Generally GH PRs are mostly for comments about code and/or implementation, Trac is more about architecture and why/why not. Both would work more or less so lets continue here :)

the_content filters only the post content, whereas blocks are nowadays also used on the theme (templates and template parts) and other places

Right. What worries me most about the proposed filters is the total lack of context. How would a plugin know when the blocks that are being rendered are for a post, a template, or some different purpose/place? That's why I asked for some code examples of how these filters are going to be used. Thinking it is pretty important to understand the intended usage, and to document it well in the docblocks.

This is also potentially useful for things like word counters, advertising placement and so on.

Frankly I don't think so. All of these would work much better by using the DOM from JS. Processing HTML as string in PHP is a really bad way to do things, really. Always always has edge cases and errors. This conclusion comes from many years of "trying to do things with HTML", not just my experience but of many many developers :)

@luisherranz commented on PR #5023:


3 months ago
#16

Thanks, Andrew.

How would a plugin know when the blocks that are being rendered are for a post, a template, or some different purpose/place?

These filters are intended to be used precisely when you need to process the rendered blocks no matter where they come from, which is the case of the Interactivity API. That is why I think do_blocks is the best place to hook, because it's the function used in all situations where blocks are being processed.

Frankly I don't think so. All of these would work much better by using the DOM from JS.

Yeah, you're probably right 🙂

But apart from that, this is still the only place we can use to inspect the render using a top-down approach, as I explained in the Trac comment:

render_blocks filters are great, but they are limited in the order they run and the information they have: render_block_data and render_block_context are top-down and can only access the block instance information, not the final rendering. render_block has the final rendering, but it's bottom-up. The render is also difficult to inspect because each block contains not only its own render, but also the render of its inner blocks. The main need for these filters came from the Interactivity API directive processing, which needs to process things top-down and be able to separate the render of interactive blocks from their (non-interactive) inner blocks.

It's just not possible to do so before this foreach loop has finished. And as the Interactivity API is powered by Preact in the frontend (which uses a top-down approach), we need to mimic that behavior or we would limit any directive that depends on a Provider component.

---

@mcsf, @youknowriad: you have a lot of experience with this, what do you think?

@dmsnell commented on PR #5023:


3 months ago
#17

There are things we cannot do with the existing filters and which filtering the block parser doesn't give us a good hook for. There's no current flow in PHP which indicates when the blocks have finished rendering. I think these new filters are somewhat self-evident in that regard, and I wish we had put them in earlier.

Are there any reasons to avoid putting these in? There have been multiple real needs for them over the years, and I understand the desire to do more work in JS where the costs are pushed to the site viewer, but I don't see that preference eliminating the need on the server to initialize filters before block rendering and follow-up with post-processing. I would be sad to see a personal preference block valuable and needed work - this happened with the spoiler/details block and it delayed that for two years to the detriment of the project.

I also hear the point about being able to filter the block parser but I would be extremely reluctant to suggest that; it's a very different semantic match, and since I imagine plugins wanting to perform some initialization before block parsing, it's very awkward for them all to replace the parser and I'm guessing that would be an easy place for things to break. Whereas I see the filterable parser primarily a way to transform _stored posts in some form_ into _the tree of block nodes_, do_blocks_pre_render has an inherent expectation that we don't have to mix block parsing and block transformation.

This does all get complicated and open up new risks, but that does seem to be WordPress' strength with its filters. What, specifically, is a reason to block this work?

This ticket was mentioned in Slack in #core by oglekler. View the logs.


2 months ago

#19 @oglekler
2 months ago

This ticket was discussed during bug scrub,

@hellofromtonya thinks that you can make in into the release if you will hurry up, so please do it :)

#20 @hellofromTonya
2 months ago

  • Milestone changed from 6.4 to 6.5

Update: Discussion is still happening.

With the 6.4 Beta 1 release party starting shortly, moving this ticket to 6.5.

@youknowriad commented on PR #5023:


2 months ago
#21

I've spent some time discussing this PR with @luisherranz and all the potential alternatives. I think this is the right call, these filters seem harmless and easy to maintain forward compatibility.

#22 @youknowriad
2 months ago

  • Keywords 2nd-opinion removed

#23 @youknowriad
2 months ago

To summarize, the main use-case for us here is that we don't have any way in Core to hook into the top-level do_blocks call: the rendering of blocks into the frontend because this can happen in multiple places:

  • the_content hook for classic blocks.
  • the template rendering for block themes.
  • "partials" for template parts in classic themes.

Having these hooks allow us to do this without major architecture refactor to the block rendering.

#24 follow-ups: @priethor
2 months ago

  • Keywords 2nd-opinion added

Could we include this in WordPress 6.4 Beta 2? These filters will be necessary to further develop the Interactivity API in the next WordPress release cycle, and having them in Beta2 is low-risk as they are new and not consumed by any core feature.

Last edited 2 months ago by priethor (previous) (diff)

#25 @priethor
2 months ago

  • Keywords 2nd-opinion removed

#26 @luisherranz
2 months ago

Could we include this in WordPress 6.4 Beta 2?

Yes, please. And sorry for missing the Beta 1. Riad and I have been discussing alternative approaches for the last weeks, but this still seems to be the only viable option to hook into the block rendering within the current architecture.

#27 in reply to: ↑ 24 @hellofromTonya
2 months ago

Replying to priethor:

Could we include this in WordPress 6.4 Beta 2?

No, I'm sorry it's too late for enhancements in the 6.4 cycle.

There are a couple of options:

  • Commit it early when 6.5 Alpha opens (which happens at 6.4-RC1).
  • If it's critical, present it to the 6.4 Release Squad for blessed task consideration.

What is a Tasks (blessed)? From the handbook:

Tasks (blessed): Feature development for the upcoming major release centers around task tickets, which are major features or important enhancements that have been blessed by the core team. A ticket should otherwise never receive this designation.

#28 in reply to: ↑ 24 ; follow-up: @hellofromTonya
2 months ago

Replying to priethor:

These filters will be necessary to further develop the Interactivity API in the next WordPress release cycle, and having them in Beta2 is low-risk as they are new and not consumed by any core feature.

I agree that the risk is low to the 6.4 release.

Note:
Though Core will not use these filters, once they are in Core, they are publicly available for extenders (plugin and theme developers) to use and must be maintained for back-compat (BC).

@priethor does the Interactivity API require these filters to be in a released version of Core? Or can it be in trunk for development usage? I assume released, i.e. 6.4.0, for use testing and feedback, but wanted to check for clarity.

#29 @gziolo
2 months ago

  • Owner changed from gziolo to luisherranz

#30 in reply to: ↑ 28 @priethor
2 months ago

Replying to hellofromTonya:

@priethor does the Interactivity API require these filters to be in a released version of Core? Or can it be in trunk for development usage? I assume released, i.e. 6.4.0, for use testing and feedback, but wanted to check for clarity.

As you correctly mentioned, it could be included in trunk for development purposes, but with the goal of releasing the directive processing in WP 6.5, having these filters in 6.4 will enable thorough user testing through the Gutenberg plugin.

#31 @hellofromTonya
2 months ago

While the Interactivity API needs these filters for continued development, testing, and feedback within the Gutenberg plugin / repo, are there any concerns about them being publicly available for plugins / themes to also hook in and modify the blocks?

I'm asking this question on purpose to better understand:

  • Are these filters necessary to only provide access for development in Gutenberg?
  • Or are these filters truly needed for any plugin including Gutenberg?
Last edited 2 months ago by hellofromTonya (previous) (diff)

This ticket was mentioned in Slack in #core by hellofromtonya. View the logs.


2 months ago

#33 in reply to: ↑ 24 @hellofromTonya
2 months ago

Replying to priethor:

Could we include this in WordPress 6.4 Beta 2? These filters will be necessary to further develop the Interactivity API in the next WordPress release cycle, and having them in Beta2 is low-risk as they are new and not consumed by any core feature.

As I previously noted, Beta 1 is the cut-off for introducing new enhancements and features into Core. See the release process handbook:

  • How the Release Cycle Works:

    Phase 3: Beta. Betas are released, and beta testers are asked to start reporting bugs. No more commits for new enhancements or feature requests are allowed for the rest of the release.

  • Releasing Beta and RC Versions

    For Beta releases, ensure that the milestone is cleared of enhancement and feature request tickets (e.g., Trac query for 6.1). If there is agreement within the release squad that a certain enhancement or feature request is critical for the release and thus needs to remain in the milestone, its ticket type should be changed to “Task (blessed)”. Also ensure with the Editor Tech Lead that enhancements/features merges from Gutenberg are similarly either cleared from the milestone or changed to a “Task (blessed)”.

Given the possible impact to 6.5, I started a consideration discussion Making WordPress slack #core channel. In doing so, it broadens the visibility to invite more discussion in case other Core contributors and committers think it is necessary to make an exception.

@luisherranz commented on PR #5023:


2 months ago
#34

Thanks for your reviews, @dmsnell and @costdev. I've now addressed all the feedback and rebased the PR.

This ticket was mentioned in Slack in #core by luisherranz. View the logs.


2 months ago

Note: See TracTickets for help on using tickets.