`render_block_data` for nested blocks - WP_Block::render vs render_block

[gaambo]

The changes introduced in #49926 also mean that hooks from render_block (like pre_render_block, render_block_data and render_block_context) only get called for top-level blocks but not for nested blocks, because nested blocks are now just created in WP_Block::render and not passed through render_block.

This leads to multiple problems:

1. There's an inconsistency in rendering depending on if a block is used top-level or in another block (eg group). This may lead to bugs.
2. Also the Latest Posts block uses this hook to parse some data (or migrate it?) - see wp-includes/blocks/latest-posts:208. This method is not called if the Latest Posts block is inside a Group block.
3. I'm not sure I can give the same information to a block via block-context as with render_block_data before.

My use case:
I'm using the 'render_block_data' method to add some custom information (eg. a helper method which parses the is-style-{style} className from the attributes and sets it as variable on the block data.

I didn't see any documentation about this change, but have to admit I still have to read everything about block contexts.

Steps to reproduce the behavior:

1. Create a new Post and insert a Group block
2. Insert a nested Latest Posts Block
3. block_core_latest_posts_migrate_categories is not called (actually it's called but not for the core/latest-posts block)

This can be reproduced for any nested block and any hook to render_block_data (and the others mentioned above).

Expected behavior
I think all blocks should be passed through the same hooks (pre_render_block, render_block_data) - the same method should be used for nested and top-level blocks.

I opened an issue on the Gutenberg Github Repo (​https://github.com/WordPress/gutenberg/issues/25900) but didn't get any response yet. Since this affects the current WP version without the plugin as well, I thought I'd open an issue here.

[noisysocks]

#51708 was marked as a duplicate.

[noisysocks]

Thanks for the report @gaambo.

We should be applying these filters inside WP_Block::_construct as well.

To test, add these filters and then open a post that has some blocks inside a Group block. Note that the filters aren't called for the inner blocks.

<?php
add_filter(
        'pre_render_block',
        function( $pre_render, $parsed_block ) {
                var_dump( 'pre_render_block', $pre_render, $parsed_block );
        },
        10,
        2
);

add_filter(
        'render_block_data',
        function( $parsed_block, $source_block ) {
                var_dump( 'render_block_data', $parsed_block, $source_block );
        },
        10,
        2
);

add_filter(
        'render_block_context',
        function( $context, $parsed_block ) {
                var_dump( 'render_block_context', $context, $parsed_block );
        },
        10,
        2
);

[gaambo]

Thanks for answering and adding 👍
Do you think it should be called in WP_Block::construct or WP_Block::render?
I think render makes a little more sense, that would mean extracting this filters from render_block - which makes this function very thin. Would that be okay? Gotta admit I'm a little overwhelmed with this function and all the context stuff happing, so can't really help with a patch here, but can help with testing.

[gaambo]

I thought I'd clicked "leave as new", don't know why it's closed now 🤷‍♂️ sorry.

[noisysocks]

Do you think it should be called in WP_Block::construct or WP_Block::render?

No preference 🙂

I think render makes a little more sense, that would mean extracting this filters from render_block - which makes this function very thin. Would that be okay?

Maybe in an ideal world we'd delete render_block but because WordPress is committed to backwards compatibility we're forced to keep it around forever.

But that's OK. I view render_block as a convenience function that wraps new WP_Block(...)->render().

Gotta admit I'm a little overwhelmed with this function and all the context stuff happing, so can't really help with a patch here, but can help with testing.

No worries at all. Block context is definitely tricky and hard to reason about, though worth noting that there are unit tests for WP_Block which ought to catch any accidental regressions.

Move the pre_render_block, render_block_data, and render_block_context filters from render_block() to WP_Block. This ensures that they are called for all blocks, including nested blocks, not just top-level blocks.

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

[noisysocks]

@gaambo: Does ​PR #691 fix the problems you were encountering?

[gaambo]

Thank you, I can confirm that it works in my use case with multiple levels of nested blocks and dynamic as well as static blocks.

@noisysocks - nice one, it would be really helpful to add unit tests to prevent future regressions.

[gziolo]

@gaambo – can you retest the updated patch? The changes applied shouldn't have an impact but as you have a real-life usage to test with you are the best person to verify :)

Code-wise everything looks great.

[gaambo]

The original issue (filters not being called) is now solved, but there are a few things (which you also commented in GitHub):

1. render_block_data (WP_Block::constructor) now get's called before pre_render_block (WP_Block::render) - which may make sense, but it's still different to before, so I think that should be documented somewhere in the changelog (dev-note?).
2. pre_render_block get's called multiple times for top-level items. Again: May make sense, but I've run into a couple of problems in the past when filters/actions were being called multiple times for the same object. In my case (see below) I just check if it's the same $block['blockName'] as run before, that's only needed for top level blocks.

Maybe add an extra argument whether it's a top-level/it's in render_block? Or is that to confusing?

To better describe my usecase:
It's on a very customized site - some of the blocks need to be rendered differently if they are nested (eg with/without margin/padding; I'm not using groups/covers/containers etc here so the client only has to insert the content-blocks and styling is all done via css/html).
So until WP 5.4 I hook into pre_render_block on the last priority and if it didn't return null I registered the currentRenderedBlock in an internal array. On render_block (again last priorty) I deregistered the block again. In between a block could check if it's nested if the currentRenderedBlocks is an array with more than one item.

I may have to adapt this to the new order of hooks being called, but that should be possible - or maybe it's even possible via block context to check if it's nested. Gotta figure that out.

I don't really understand why these filters are in the constructor and not inside render? They are render specific, and putting them in the constructor of the Block class introduces side-effects and limits the utility of WP_Block as a model object.

I don't really understand why these filters are in the constructor and not inside render? They are render specific, and putting them in the constructor of the Block class introduces side-effects and limits the utility of WP_Block as a model object.

By "model object" do you mean that you'd expect $parsed_object === ( new WP_Block( $parsed_object ) )->parsed_object to always be true?

WP_Block::render concatenates and returns whatever is already set in WP_Block::$inner_blocks, WP_Block::$inner_html, and WP_Block::$inner_content. These properties are eagerly set in WP_Block::__construct.

We could lazily instantiate this data in WP_Block::render instead but then that would break backwards compatibility because WP_Block::$inner_blocks, WP_Block::$inner_html, and WP_Block::$inner_content are all public properties.

So, I'd like to move these filters to WP_Block::render, but am unsure how to do so given this predicament. Any ideas?

By "model object" do you mean that you'd expect $parsed_object === ( new WP_Block( $parsed_object ) )->parsed_object to always be true?

Exactly.

So, I'd like to move these filters to WP_Block::render, but am unsure how to do so given this predicament. Any ideas?

Can we initialize the property in the constructor, but then in render backup the current prop value, apply the filters, and then restore the prop value at the end of render?

Can we initialize the property in the constructor, but then in render backup the current prop value, apply the filters, and then restore the prop value at the end of render?

I dislike that this would mean we're doing unnecessary work in the constructor. render_block is a fairly warm path, so performance is important.

I'll have a play. It might be possible to move all of the work into render() and turn the public properties into lazy getters.

OK, I've moved the two filters to WP_Block::render() in cc5c894.

I did this by making it so that most of the properties that are derived from $parsed_block and $available_context are computed lazily.

We can then, in WP_Block::render(), set $parsed_block and $available_context to the filtered values and reset the cache. The properties will be re-derived from the filtered values.

In real usage, this means we compute the properties at most once per render.

This looks great to me!

[gaambo]

I see, that this is not an issue easy to fix, but I still wanted to ask: Any possibility that this goes into 5.6?
Since the original commits which removed the filters for innerBlocks were part of 5.5 means that I'm stuck von 5.4 on a few clients sites and have to wait for 5.7.

[gziolo]

I personally think that it's a bug fix that should still land in 5.6. @TimothyBlynJacobs accepted the latest revision to the patch so I don't see any blockers. What do you all think?

[TimothyBlynJacobs]

I definitely agree it is a bug fix, but it is frighteningly close to RC.

[noisysocks]

Agree that it would be nice to fix this 5.5 regression in 5.6.

I'll commit this for 5.6. We can revert this if there are any hiccups found in the RC period.

[noisysocks]

In 49608:

Editor: Move pre_render_block, render_block_data, render_block_context

Move the pre_render_block, render_block_data, and render_block_context
filters from render_block() to WP_Block. This ensures that they are
called for all blocks, including nested blocks, not just top-level
blocks.

Fixes #51612.
Props gaambo, gziolo, TimothyBlynJacobs.

[SergeyBiryukov]

In 49609:

Docs: Drop src/ from pre_render_block duplicate hook reference.

Follow-up to [49608].

See #51612.

[azaozz]

May be overthinking this but [49608] seems to introduce two "states" of WP_Block. When it is instantiated it has unfiltered (wrong?) data, then after render() is run the data is reset to the filtered values. As far as I understand WP_Block was modeled after WP_Post that is (mostly) static in nature.

This seems to bring a lot of complexity/possible confusion when using it, and some back-compat concerns. Before [49608] WP_Block was instantiated with the filtered data that was (more or less) immutable while the instance is used. Now the data is mutated after render() (hence the need for WP_Block::reset(), __get(), etc.).

Not sure if this is a good change going forward. It's true that WP_Block::$parsed_block is public and plugins can "hack it" at any time (that btw doesn't seem like a good idea too), but having an instance with unfiltered data doesn't seem useful/helpful.

A possible solution may be to run the existing filters as soon as WP_Block is instantiated (in the constructor, as mentioned above) and perhaps introduce new filter(s) in render() that only affect the output HTML, if needed.

[TimothyBlynJacobs]

I think this should probably be reverted and tried again in 5.7-early. This has more ramifications than was previously thought, that should be resolved in Beta not RC.

When it is instantiated it has unfiltered (wrong?) data

IMO this isn't incorrect data. I think it would be incorrect if the render filters were used when the block wasn't in a render context. I think this is analogous to WP_Post::filter and WP_Term::filter. Maybe we should make the WP_Block follow that filter API pattern instead to be more clear.

[azaozz]

Replying to TimothyBlynJacobs:

I think this should probably be reverted and tried again in 5.7-early. This has more ramifications than was previously thought, that should be resolved in Beta not RC.

Yeah, starting to look that way.

When it is instantiated it has unfiltered (wrong?) data

IMO this isn't incorrect data. I think it would be incorrect if the render filters were used when the block wasn't in a render context.

Yeah, the render filters would ideally run while the block is being rendered (judging by their names). However before this change they were running before WP_Block was even instantiated. Running them "late" seems to bring unneeded complexity and problems.

One possible solution could be to deprecate these filters and leave them in place, then introduce new "render" filters that run in render() and do not mutate the block properties. On the other hand deprecating something and leaving it in place means plugins that already use these filters would likely never change...

Another (preferable?) solution would be to remove (and shim) WP_Block::render(), i.e. do the rendering as soon as WP_Block is instantiated. Seems this will bring WP_Block more inline with WP_Post, WP_Term, etc.

I think this is analogous to WP_Post::filter and WP_Term::filter. Maybe we should make the WP_Block follow that filter API pattern instead to be more clear.

WP_Post::filter seems to hold the level of sanitization of the post, doesn't seem to affect specifically the hooks/filters.

[TimothyBlynJacobs]

However before this change they were running before WP_Block was even instantiated. Running them "late" seems to bring unneeded complexity and problems.

But only if render_block is the only consumer of WP_Block, which I don't think we want to constrain in that way.

Another (preferable?) solution would be to remove (and shim) WP_Block::render(), i.e. do the rendering as soon as WP_Block is instantiated. Seems this will bring WP_Block more inline with WP_Post, WP_Term, etc.

That would bring a performance penalty if someone is trying to use WP_Block as a data structure, and they don't need the rendered content. Developers do weird things during rendering sometimes that can be quite slow. It's why we needed to allow for pulling just the raw content via the REST API for instance.

WP_Post::filter seems to hold the level of sanitization of the post, doesn't seem to affect specifically the hooks/filters.

Right but the sanitization is a series of filters on the individual post fields. Including a display filter.

[azaozz]

Replying to TimothyBlynJacobs:

But only if render_block is the only consumer of WP_Block, which I don't think we want to constrain in that way.
...
if someone is trying to use WP_Block as a data structure, and they don't need the rendered content...

Right. In this case guessing plugins and themes will have to learn to live with the fact that WP_Block is different than WP_Post and is "mutable". Still thinking this should be described in more details, perhaps in the top docblock.

However thinking that resetting the properties after rendering (i.e. the class properties are different before running render(), while running render(), and after running render()) makes things more complex, and hard to understand and work with. Same for the magic __get().

Probably missing something but what purpose/use case does this solve? Seems these properties can be instantiated with null or the corresponding values from $parsed_block, then set/changed during render(). Why do they need to be unset/reset after that? render_block() calls $block->render() immediately after instantiating WP_Block, so these will be (should be) correct there.

Right but the sanitization is a series of filters on the individual post fields. Including a display filter.

Right, the difference is not to misunderstand that "filter" in WP_Post::filter means filtering/sanitization of the post object values, nothing to do (directly) with hooks.

[noisysocks]

In 49694:

Editor: Remove render_block hooks from WP_Block

Reverts the move of pre_render_block, render_block_data, and
render_block_context to WP_Block.

This change has more implications than first thought so will be revisted later
in 5.7.

Reverts [49609,49608].
See #51612.

[noisysocks]

In 49695:

Editor: Remove render_block hooks from WP_Block

Reverts the move of pre_render_block, render_block_data, and
render_block_context to WP_Block.

This change has more implications than first thought so will be revisted later
in 5.7.

Reverts [49609,49608].
See #51612.

[noisysocks]

I reverted the changes from trunk and 5.6 🙂

Move the pre_render_block, render_block_data, and render_block_context filters from render_block() to WP_Block. This ensures that they are called for all blocks, including nested blocks, not just top-level blocks.

So that there is no performance penalty incurred from calculating block properties twice, WP_Block can now be made to not set any derived block properties until render() is called.

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

@azaozz @TimothyBJacobs: How about something like this? Properties can still be lazily initialised, but it requires explicitly passing an option to new WP_Block(). This way we don't need __get().

Neither works for me.

On reread it goes in to an options property so best as is.

Thinking now is the best time to look a bit at the "bigger picture" :)

As far as I see WP_Block is --not-- a "standard OOP" class intended to be sub-classed, extended, instantiates in different ways, etc. It is, in a way, closer to a theme's template part than to a low-level data structure, same as WP_Post, WP_Comment, etc.

• What is the purpose of WP_Block? Looking at https://core.trac.wordpress.org/ticket/49926 it is to "to serve the purpose of providing a first-class interface for plugins to interact with an instance of a block from within their own render callbacks." So plugins are expected to replace core's render_block()? Is that the way it works now? Is that the way plugins use it? Are there other (non-intended/non-supported) ways plugins may use it?
• Should WP_Block be final like WP_Post, WP_Comment, etc.? If not, what would be the expected/supported use in plugins? Should it be a "wrapper" for the parsed and filtered block data (as used in core), or should there be other expected uses?
• What happens with the low-level pre_render_block, render_block_data, etc. filters when five plugins instantiate and/or extend WP_Block in five different ways? What about other plugins that depend on these filters? Should plugins be able to overload $block->render()?
• As @TimothyBJacobs mentioned on the trac ticket, WP_Block may be possible to use as data structure in plugins, without needing to cal render(). Are there plugins using it that way? What are the advantages to use multiple, instances of WP_Block (with different data depending on the level of "rendering") to using the pasted blocks data directly?
• Perhaps revisit the need of having WP_Block::render() and maybe deprecate it. I know, that mimics similar structures in React in js, but it doesn't work the same way in PHP. (Actually even in React "plugins" are not supposed to cal render() on different instances manually, afaik. That would break the app).

Uh, sorry this is getting too long. As far as I see there's no clarity about howWP_Block should, and should not be used by plugins, and how plugins will interact with one another when using it.

I don't really have much to add that I haven't mentioned on the ticket yet. I think this is a fine approach, though I personally would prefer it followed a pattern closer to our existing patterns using $filter like I mentioned on the ticket because I think it is more explicit about what type of mode the object is operating in.

[johnbillion]

@peterwilsoncc @TimothyBlynJacobs @azaozz What's left here?

[noisysocks]

This needs a new owner and possibly a new approach to address the points raised by @azaozz.

[gaambo]

This is still (WP 5.7) a very big problem for nearly every project I build with the block editor. As soon as you put a block into any "wrapper" (eg Media & Text, Group, etc.) pre_render_block and render_block_data do not get called.
A few use cases I see:

• I often use pre_render_block to call a function which adds an action/a filter into a more generatel hook but only needed for this specific block (e.g. if I want to filter the Latest Posts Blocks query with pre_get_posts)
• Use pre_render_block to short-circuit rendering of a nested block for non-loggedin-users (eg. on a member site)
• Use render_block_data to pass additional data to the block (maybe this could be done with context, but often times I'm using these hooks to add to third party blocks).

I don't see any reason for these hooks not to be called for nested blocks, especially with FSE on the way there will be more complex layouts which need nesting.

I read through all the comments here and it seems the biggest problem is that WP_Block was introduced without a real reason and it is not clearly defined how WP_Block should be used. In this case I think it's important to define the usecase and document it clearly very soon. Because when more and more theme and plugin developers will build blocks and there are no clear guidelines for its usage, we'll see more different usages and it will be harder to have a well defined interface.

My findings so far:

• WP_Block_Type represents a block type (similar to WP_Post_Type) - WP_Post_Type is a final class and not intended to be subclassed for registration etc.
• A WP_Block instance is only created when a parsed block should be rendered (from render_block). For each parsed innerBlock a new instance is created as well.
• WP_Block_List is an array interface for all the innerBlocks of a WP_Block instance

To keep the interface similar to posts, I would suggest making WP_Block_Type a final class - there is one plugin (and no theme) in the repository right now which extends this class (see ​wpdirectory search for `extends WP_Block_Type`). Therefore I assume there are little to none premium plugins/themes which extend it, a dev-note should be enough imho.
I've ran a few searches: extends WP_Block_Type, extends \WP_Block_Type, new WP_BlockType (there are a few cases of that, but I don't see any problem with that).

I'm not sure about WP_Block. It's kind of similar to WP_Post in a sense that it has data from the database set as properties. But it has one method which generates dynamic output (render) and a constructor which initialized other objects (WP_Block_List with WP_Block instances). I think WP_Block should be final as well and all the render logic should be put back to the render_block function as in before #49926 . Therefore the WP_Block constructor doesn't have to do some complex juggling of filtered/parsed/non-filtered data.

There were thoughts about just implementing WP_Block as an ArrayAcces data object in the ​original Gutenberg PR.
The issue of filters only running on top-level-blocks was even mentioned in ​a related PR.
I've got to admin I'm a little confused to what was changed and reverted when 😅
I've also searched
on wpdirectory.net for extends WP_Block and only Blocksy extends WP_Block_Parser.

I think extracting the render method back to render_block should be an easier fix, but someone with more experience and knowledge of block context/globals etc would have to check for that.

[jakeparis]

I just want to thumbs up this request (not exactly sure what the trac-approved way to add support to a request is). This issue stopped me in my tracks when writing something to replace a specific block upon render.

If the original block is in a column or a group, my filter doesn't run at all. Are there any known workarounds for this issue?

[gaambo]

Replying to jakeparis:

I just want to thumbs up this request (not exactly sure what the trac-approved way to add support to a request is). This issue stopped me in my tracks when writing something to replace a specific block upon render.

If the original block is in a column or a group, my filter doesn't run at all. Are there any known workarounds for this issue?

I've coded a workaround which uses render_block_data to render all child blocks recursively with render_block: ​https://gist.github.com/gaambo/16c7fb6fd758e4eb72e0f5db3888dd40

Works up to WordPress 5.7, haven't tested with 5.8 - if you notice any errors/bugs feel free to comment under the gist.

[azaozz]

Replying to gaambo:

Thanks for the excellent break-down!

I don't see any reason for these hooks not to be called for nested blocks, especially with FSE on the way there will be more complex layouts which need nesting.

Right, this needs to be fixed, one way or the other.

I read through all the comments here and it seems the biggest problem is that WP_Block was introduced without a real reason and it is not clearly defined how WP_Block should be used.

Yes, seems that way especially around "top level block" vs. "nested block".

My findings so far:

• WP_Block_Type represents a block type (similar to WP_Post_Type) - WP_Post_Type is a final class and not intended to be subclassed for registration etc.
• A WP_Block instance is only created when a parsed block should be rendered (from render_block). For each parsed innerBlock a new instance is created as well.
• WP_Block_List is an array interface for all the innerBlocks of a WP_Block instance

As far as I see that's correct.

To keep the interface similar to posts, I would suggest making WP_Block_Type a final class

That's a good idea but not really sure it can be "pulled off". As there is only one plugin (and the author(s) can probably be contacted) perhaps this can be done. Still, the possibility of a fatal error in custom themes/plugins is pretty "scary" :)

I'm not sure about WP_Block. It's kind of similar to WP_Post in a sense that it has data from the database set as properties. But it has one method which generates dynamic output (render) and a constructor which initialized other objects (WP_Block_List with WP_Block instances). I think WP_Block should be final as well and all the render logic should be put back to the render_block function as in before #49926 . Therefore the WP_Block constructor doesn't have to do some complex juggling of filtered/parsed/non-filtered data.

Yes, my thoughts too. Unfortunately don't think it can be made final now. I would have really liked if that render() method wasn't in there and the class was implemented as "data store" similarly to WP_Post.

On the other hand an instance of WP_Block can also be a "container" for other instances of WP_Block which makes it more complicated to work with. Thinking that needs to be documented well.

I think extracting the render method back to render_block should be an easier fix, but someone with more experience and knowledge of block context/globals etc would have to check for that.

Yeah, thinking that can be done.

[azaozz]

Replying to gaambo:

I've coded a workaround which uses render_block_data to render all child blocks recursively with render_block: ​https://gist.github.com/gaambo/16c7fb6fd758e4eb72e0f5db3888dd40

Yes, was thinking something like that may work. As far as I see a solution can be based on replacing

$block_content .= is_string( $chunk ) ?
    $chunk :
    $this->inner_blocks[ $index++ ]->render();

with something like

if ( is_string( $chunk ) ) {
    $block_content .= $chunk;
} else {
    $block_content .= render_block( $this->inner_blocks[ $index ]->parsed_block );
    $index++;
}

Can you see if that change fixes the filter problem and if there are any unwanted side effects, and perhaps make an "official" PR or patch as the change is mostly like your gist anyway :)

Changes WP_Block::render to use render_block for inner blocks instead of WP_Block::render directly.
Added a test to ensure all related block filters now run for inner blocks as well.
Breaks some block tests ❗

Trac ticket: #51612

[gaambo]

I've added a new PR on github to change the WP_Block::render method to use render_block for inner blocks as proposed by @azaozz. This fixes the bug in my test cases.
It can be tested by creating a post/page with a group block and an inner block (eg paragraph) and adding the code from ticket:51612#comment:4 somewhere (plugin, functions.php) and ensuring it outputs the data for the group and paragraph block.

I've also written an unit test test_calls_block_filters_for_inner_blocks to prevent future regressions.

The problem is it breaks two other tests: Tests_Blocks_wpBlock::test_passes_content_to_render_callback and Tests_Blocks_wpBlock::test_render_static_block_type_returns_own_content. I've debugged them and the problem is the following:
In these tests a custom block registry instance is passed to the WP_Block constructor. The WP_Block constructor get's the WP_Block_Type instance from this registry and saves it in a property of the block instance. With the proposed changed it now creates a WP_Block instance for each inner block but calls render_block in render, which creates a new WP_Block instance for the parsed block again. Since render_block has no way of passing a block registry instance it passes none to (re-)creating the new block instance and then WP_Block just assumes the global block registry. Which lead's to empty( $this->block_type->skip_inner_blocks ) (class-wp-block.php:206) not evaluating to true and therefore not calling the render code in this if.

I'm not sure how to go about this. I tried changing all tests to use render_block instead of creating new WP_Block instances and rendering them. But that brings the same problem and I don't think it's a test-problem but rather an implementation problem.

Another way could be to allow passing a block registry instance to render_block but I don't know if that's good? Seems we're back to talk about the use-cases of render_block and WP_Block::render 🤷‍♂️

Add pre_render_inner_block, render_inner_block_data, and render_inner_block_context filters that run before an inner block is rendered.

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

This patch is similar to the original idea by @noisysocks ​https://github.com/WordPress/wordpress-develop/pull/691/commits/b04425a5d557ada359a1a7768cf1ba80e2d486bf only the filters are in render() and run before the inner WP_Block is instantiated (same as how they run for the top/parent WP_Block).

This is actually a cool idea! 🚀

This is actually a cool idea! 🚀

This patch is similar to the original idea by @noisysocks ​b04425a only the filters are in render() and run before the inner WP_Block is instantiated (same as how they run for the top/parent WP_Block).

Clever! That addresses ​the main criticism of my original approach which was that the filters were being called in WP_Block::__construct and not WP_Block::render. Admittedly it breaks encapsulation to have one object mutate another, but I don't see why it should matter in practice, and both objects are of the same type.

The filters have _inner in the name to distinguish them from the top block's filters. The same context could also be provided in a different way, perhaps. Ideas welcome :)

I do not like this. It creates extra work for plugin developers as they'll have to define two filters instead of one. Blocks should work the same way regardless of whether they’re nested inside another block or not.

Hmm, tests seem to fail globally at the moment, something about mismatched cert. ​https://github.com/WordPress/wordpress-develop/pull/1684/commits/80695cfd4f0a439e931755fa12d5f5cee8797b43 should be fine, tests pass locally.

it breaks encapsulation to have one object mutate another, but I don't see why it should matter in practice

Right, to get to the inner blocks you'll need to render the top/parent block. However there's a pre_render_block filter meant to short-circuit the rendering including any inner/nested blocks, so the results can be very different if that filter is not applied. Not sure we can do anything to avoid/daly running that filter...

I do not like this. It creates extra work for plugin developers as they'll have to define two filters instead of one. Blocks should work the same way regardless

Yeah, devs will have to use both "top" and "inner" filters. Having a different name makes it very clear the currently filtered block is inner. Blocks should work the same way bit they don't, and most look very differently when nested. Also was thinking it may be useful to pass some more context to the inner filters, but may be redundant.

Updated the PR and changed the filter names to match. Context is provided by adding another param: $parent_block which is null for top level blocks. That should be enough for inner blocks imho.

Blocks should work the same way but they don't, and most look very differently when nested.

I think this point is valid, maybe "some" not "most", but it's a little inconvenience that I think adds more control IMO.

Yes but often the reason a block looks different when nested is only due to styling. The HTML will be the same. The most common use case for these filters is to slightly change the HTML. I think that plugin developers and theme developers will expect that the filters do this regardless of whether the block is nested or not.

Updated the PR and changed the filter names to match. Context is provided by adding another param: $parent_block which is null for top level blocks. That should be enough for inner blocks imho.

I agree. Perfect 🙂

[noisysocks]

​https://github.com/WordPress/wordpress-develop/pull/1684 looks good to me and it tests well locally 👍

Might want to consider adding a unit test to tests/phpunit/tests/blocks/wpBlock.php.

[gaambo]

​https://github.com/WordPress/wordpress-develop/pull/1684 is a great solution - I tested it locally and it worked for my manual test cases.
The unit tests I added in my PR work as well - You can use them in this PR if you want (any feedback welcome, these are some of my first unit tests 😅).
​https://github.com/WordPress/wordpress-develop/pull/1617/files#diff-9892dfa3fee8002bf9e3b901c50243a72e2e9509700d2cbd857ccc757348106e

[azaozz]

Great! Thanks for testing and reviewing @noisysocks and @gaambo. Sure, going to include the tests from ​https://github.com/WordPress/wordpress-develop/pull/1617.

[azaozz]

In 51894:

Apply the pre_render_block, render_block_data, and render_block_context filters when rendering inner/nested blocks. Introdices another param to these filters: $parent_block that is the "parent" WP_Block instance for nested blocks and null for top level blocks. Adds unit tests for the filters.

Props noisysocks, gaambo, azaozz.
Fixes #51612.

Fixed in https://core.trac.wordpress.org/changeset/51894.

[SergeyBiryukov]

In 51895:

Docs: Add a @since note for the new $parent_block parameter of several filters:

• pre_render_block
• render_block_data
• render_block_context

Follow-up to [51894].

See #51612.

Closing as alternative implementation was merged and ticket is closed.

[scruffian]

#51708 was marked as a duplicate.