Request: filter for parse_blocks() result

[dougwollison]

I'd like to be able to access and if needed modify the parsed blocks for a page before it's rendered on the frontend. My particular use case invovles ensuring pages have a certain structure with some exceptions that can't reliably be address by simply using hasBlockType or filtering an individual block's data.

My current solution is to hook into the block_parser_class filter and return my own extension that simply filters the ouptut of WP_Block_Parser:parse(). It works great but it's a bit of a round-about way.

I think for sanity/caching friendliness it should be added in the return logic of parse_blocks().

[dougwollison]

Proposed 'parse_blocks' filter on parse_blocks() return value.

[johnbillion]

I think this is the intention of the block_parser_class a few lines above. If you need to heavily customise the block parsing and result then you can use this filter to specify your own class that does so. If you extend the WP_Block_Parser class then you can reuse its methods.

Does that suit your needs @dougwollison or do you think there's still value in having a filter applied to the parsed result?

[dougwollison]

Replying to johnbillion:

I think this is the intention of the block_parser_class filter a few lines above. If you need to heavily customise the block parsing and result then you can use this filter to specify your own class that does so. If you extend the WP_Block_Parser class then you can reuse its methods.

Does that suit your needs @dougwollison or do you think there's still value in having a filter applied to the parsed result?

That's my current solution, and it works fine for my use case with a custom-tailored theme, but I'm worried this isn't very extensible should a 3rd party plugin need the same capabilities. If my code and some 3rd party plugin are replacing the parser class, then only one of ours will be used, depending on what priority our filters are.

I guess it could work as-if is if both implementations use a higher-order-component style aproach, instantiating the previous class inside another one to wrap/proxy the parse() method. Even then, that feels like it overcomplicates the normally simply act of adding a hook to filter an array. I think there's some potential messiness involved in having 2+ differently named filters that all do the same thing in such a scenario, but I'm already overthinking it.

[justlevine]

Dusting this off after 5 years.

While replacing the block_parser_class is suitable when you "need to heavily customize the block parsing", it's a very uncomfortable DX.
In support, we need look no further than the overreliance of the render_block/render_block_data hook for Block Supports and the resulting headaches when trying to work with that data in PHP, implement caching, using over REST/GraphQL or otherwise outside the template canvas, etc.

The attached diff is still valid all this time later, though I think we should pass the original $content as well, since that's a safer target than whatever the results of a filtered block_parser_class may be.

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

Adds parse_blocks filter to parse_blocks().

Based on https://core.trac.wordpress.org/attachment/ticket/49442/49442.diff but with a second parameter for the original, unparsed $content.

[justlevine]

Related: https://core.trac.wordpress.org/ticket/45471

[audrasjb]

Moving to milestone 6.9 as per today's devchat.

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

Added since version number 6.9.0

[pmbaldha]

In my patch ​https://patch-diff.githubusercontent.com/raw/WordPress/wordpress-develop/pull/8941.diff, I have added since version 6.9.0.

[justlevine]

Test cases for this are pretty simple, since we already test render_block() which directly uses this.

Should be covered by using the filter to update a prop, and then checking the results of render_block to see if the rendered block reflects the changes from the filter, along with edge cases around someone providing a bad value to the filter, and comparing that the second param $content is the same as what's passed pre-parsing.

As soon as I can figure out where in phpunit/tests/blocks this should live, I'll update the PR accordingly.

[justlevine]

With some helpful pointers from @SirLouen , I added the following PHPUnit tests. It's possible they'd exist better as a manual "test use case" and my desire to get this into core pushed me overboard 😇.

Test use cases

1. Changing individual properties - the most straightforward use of add_filter( 'parse_blocks', ...) that will allow us to decouple rendering from parsing

2. An example caching implementation:
   1. parse_blocks filter adds a cache ID to the block atts
   2. render_block filter stores the rendered block in wp_cache_*()
   3. pre_render_block returns the cached block early, so it doesn't need to be re-rendered.

[dmsnell]

What @johnbillion said above is true: the class filter was added to allow folks to perform various kinds of parsing here. I think we are inviting trouble by encouraging filtering of parse_blocks() results, but also most people hopefully won’t be using the filter, so 🤷‍♂️

Here’s one thing I think would be important for this work: if we’re adding the filter, the current proposed docblock is under-helpful. For one, the type of the blocks is array, but we know that it passes a tree of block objects, and it would be incredibly helpful to people to show that in some examples inside the comment. The top-level of the tree is a list of blocks, may be empty, and the tree is recursive.

If we add this filter the way it is, I think it tends to give an impression that the result is a linear and flat list of blocks, which it is not. This is the trouble in exposing the filter here: it takes thoughtful intention to traverse the output properly, something not required when using pre_render_block, render_block, render_block_data, and render_block_context filters.

It works great but it's a bit of a round-about way.

If I can ask, @dougwollison — if it works great, and you found the intended filter to provide this extensibility, in what ways does it feel “round-about” to you?

In support, we need look no further than the overreliance of the render_block/render_block_data hook for Block Supports and the resulting headaches when trying to work with that data in PHP

@justlevine do you mind making some of these observations concrete? What is the over-reliance you are referring to? What are the headaches?

---

One more-recent development is the introduction of ​traverse_and_serialize_blocks(), which gives more direct control over the block tree as a filter. I know also that one can take the parsed block tree and modify it at will as the parsed data structure (for example, ensuring post structure is not necessarily a parsing-stage concern). Is it right to assume that by “ensure” you mean you are modifying the structure when something is missing or otherwise amiss?

It would be useful to know which aspects of modifying the parsed block tree are inconvenient leading to the desire to filter the output. Are you also wanting to ensure that every single time block content is parsed that the filter runs? On parsing widgets, on parsing excerpts, in processing post comments, Woo products, etc…?

[justlevine]

@dmsnell thanks so much for the feedback!

Things are a bit hectic in my part of the world ATM so I want to reply and mind dump while I can to keep the conversation (finally 🥳) going. I'll be back to backfill the rest as soon as possible:

if we’re adding the filter, the current proposed docblock is under-helpful.

100-thousand percent, although that really needs to come from improvements to WP_Block_Parser::parse(), which might be beyond the scope or might be enough for us to infer from pre_render_block hook 🤔. I'll dive back here when I can if nobody beats me to it.

---

This is the trouble in exposing the filter here: it takes thoughtful intention to traverse the output properly, something not required when using pre_render_block, render_block, render_block_data, and render_block_context filters.
[...] One more-recent development is the introduction of traverse_and_serialize_blocks(), which gives more direct control over the block tree as a filter.

First I wanna take a quick second to talk semantics, and possibly fix an XY problem I inadvertently created by using caching as a test case (and sharing over on #45471) - apologies 😅

• From within the function: the parse_blocks() function extracts data from a string using a specific Parser class.
• From the function's implementation: the parse_blocks() function prepares saved block data that has been stored as a string for use - usually, but not always, before rendering.

It's the latter that's the primary concern, even if borne out from DX issues extending WP_Block_Parser (demonstration pending 🙇). It's also why (to my understanding) traverse_and_serialize_blocks() doesn't help here, because it's even _more removed_ from the parse/render knot I'm hoping to decouple.

<mind_dump>

So with that in mind, by adding this's filter we're essentially proposing :

1. tweaking the hook lifecycle so it's parse_blocks => foreach { pre_render_block -> render_block -> render_block_data -> render_block_context. (Assuming the doc-block gives some better context on the array shape), I don't think requires any more "thoughtful intention" than the current lifecycle, even if the existing project tech debt unfortunately forces it to act on the array<TRenderBlockArgs>, instead of the array of $args for a single block.
2. Allowing for the possibility that current GB/3rd-party code that currently needs to:
   • use render_block or render_block_data to parse and hydrate block data (because they rly use *_Block_Parser* ).
   • entirely render a block to have its data be fully parsed, (and as a consequence, feel the need to rely on weird caching solutions to make getting the block data more performant.).

Tl;dr: we need to separate the act of rendering blocks from the act of hydrating block-data from parsable-strings, and this is the easiest way to incrementally start doing that, is back+forward-compat and has a small enough footprint + offers immediate value, and therefore stands a semblance of a chance of making it into core (any other attempt to improve separation of concerns here will IMO either affect lifecycle or need a muuch bigger footprint on the codebase)

</mind_dump>

---

@justlevine do you mind making some of these observations concrete? What is the over-reliance you are referring to? What are the headaches?

This [prob the most important part] I need to come back to when I'm back at my desk, since it involves real code snippets and references. Promise to fill this in as soon as possible - but please people continue to chime in without me.

<mind_dump>
Basically I owe:

• [ ] List of add_filter( 'render_block_data'* GB plugin -> core items that avoided WP_Block_Parser_*, as well as those that more generally are about parsing, not rendering vs places in the codebase we use parse_blocks() and ideally want access to that.
• [ ] Pick a popular extends WP_Block_Parser and show what's required as a plugin to safely add your custom block attribute to support it + core blocks.
• [ ] Point uses of the render_block() to safely get all the block data + performance smells / related tickets.
  • 3rd-party from headless theres the WPGraphQL + WP-Rest plugins + WooComm's new checkout. IIRC there were some REST APIs headed to core/gutenberg too that (as proposed) have no choice but to needlessly render too.
• [ ] and then just grep a list of those abusing add_filter( 'pre_render_block', * showing how devs currently work around this.

</mind_dump>

[dmsnell]

Thanks for sharing the mind dump @justlevine — all good, it’s okay for things to take time, especially design-oriented things since the repercussions of a design carry far longer than the effort spent building them.

that really needs to come from improvements to WP_Block_Parser::parse(), which might be beyond the scope or might be enough for us to infer from pre_render_block hook

there very well could be need to improve the parser’s docblocks with examples and outputs. still, this is more important to me if adding a parse_blocks filter than it is in the parser itself, because someone implementing a different parser is more likely to investigate, while someone reaching for a filter with array is likely going to assume that the situation is as simple as the name and documentation suggest it is

but we know it’s not that simple, and characteristically different than pre_render_block. all of the existing filters have intentionally focused on the block level, leaving traversal up to WordPress itself. this proposed filter is introducing an interface which leaves traversal up to the plugin author when I don’t think there’s precedent in Core of doing so.

that is, if we introduce this new way of traversal, we owe it to the ecosystem to highlight how basic assumptions will be broken and how to avoid falling into those pits.

although I shared many words here the proposal is neither lengthy nor complicated: at a minimum we could list of edge cases in Example sections of the filter’s docblock and leave WARNING notes that the blocks are a tree and require tree traversal.

why (to my understanding) traverse_and_serialize_blocks() doesn't help here, because it's even _more removed_ from the parse/render knot I'm hoping to decouple

this makes sense, and I wondered if that was where you were going. in such a case let’s focus on establishing the motivating use case for all this. parse_blocks() is universal in WordPress and changing its behavior is going to bring potential risks in a lot of unexpected places, even if that change comes from a plugin.

this is where I am curious about enforcing constraints on certain post types, as perhaps there are other ideas for exposing extensibility points later in the flow from database to browser, or content-type-specific extension points which are guaranteed to be limited by nature instead of expansive by nature.

even if the existing project tech debt unfortunately forces it to act on the array<TRenderBlockArgs>, instead of the array of $args for a single block.

when you get around to it, I would love to understand better what you mean here. I do not think I follow what you are trying to say.

feel the need to rely on weird caching solutions to make getting the block data more performant

for sure, almost every time I see caching introduced it’s usually due to a felt need to rely on weird caching solutions. transparently, this only makes me more curious when I hear caching proposals. block parsing is quite fast, much faster than almost every other step of the block rendering pipeline, and I have yet to seen empirical evidence showing caching the parse to have any meaningful impact. if we want to talk about memory bloat that’s definitely an issue, but one that caching only exacerbates.

we need to separate the act of rendering blocks from the act of hydrating block-data from parsable-strings

perhaps we can focus on fleshing out what you mean here, as I think it probably makes more sense to you when you write it in the context of the work you’ve been doing than it does to me when I read it and am trying trying to guess what you mean.

where I was going with traverse_and_serialize_blocks() is another effort in Core to build semantic tools for operating on block trees. perhaps there are similarly-easy-to-add-and-similarly-impossible-to-remove filters or stages that help you accomplish what you want more easily than exposing the list of parsed blocks. I don’t know. I’m not opposing this work, but I don’t quite understand it.

Pick a popular extends WP_Block_Parser and show what's required as a plugin to safely add your custom block attribute to support it + core blocks.

Noting that on the ​WPDirectory there are three plugins extending WP_Block_Parser. of those, hCaptcha is actually using it for its intended purpose, where they supplement failed parses with some additional best-effort parsing. Shout out to them! The other two both appear to extend the block parser and introduce a few additional layers of complexity when it seems to me like a more basic render_block filter would serve the purposes in a much easier way. One has a number of stages of transforming the parser output, and every one of them demonstrates the nuanced issues I mentioned above, and this is no critique to them. If capable teams with huge plugin install bases overlook some of these details, then making it even more normative to use such an interface increases my skeptical intuition on this.

IIRC there were some REST APIs headed to core/gutenberg too that (as proposed) have no choice but to needlessly render too.

This is a misunderstand I suppose we could support more. It should be trivial to skip all rendering, either by calling parse_blocks() directly or by one of my favorite methods: the pre_render_block filter

<?php
define( FIRST_PRIORITY, /* whatever you typically use */ PHP_INT_MIN );

function my_block_thingamajig( $content, $parsed_block, $parent_block ) {
        // do something with block
        // visits parents before children, children before siblings
        // can rely on static vars or globals for output, or return actual output (e.g. arrays, JSON) through $content
        // top-level blocks have `null` as a parent block

        return asset( $parent_block ) ? '' : null;
}

add_filter( 'pre_render_block', 'my_block_thingamajig', FIRST_PRIORITY, 3 );

do_blocks();

remove_filter( 'pre_render_block', 'my_block_thingamajig' );

Please don’t feel obliged to rapidly reply. It’s worth taking time to ponder, refine, and write out stuff. Wishing peace amidst the hectic times.

[SirLouen]

@justlevine I've added some review changes to your testing block

[wildworks]

This ticket was featured on today's 6.9 Bug Scrub. What would it take to
move this ticket forward?

[wildworks]

This ticket was featured on today's 6.9 Bug Scrub again.

This ticket has had no activity for several months, so let's punt it to 7.0.