Editor: Move pre_render_block, render_block_data, render_block_context

[noisysocks]

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

---

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.

[gziolo]

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

[gziolo]

Good changes applied. The solution proposed should be fine in terms of the backward compatibility.

[TimothyBJacobs]

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.

[noisysocks]

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?

[TimothyBJacobs]

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?

[noisysocks]

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.

[noisysocks]

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.

[TimothyBJacobs]

This looks great to me!