WP_Block: Inner blocks not being set

[noisysocks]

From ​https://wordpress.slack.com/archives/C02QB2JS7/p1606038913306200:

Gutenberg 9.4 expects $block→inner_blocks to be set. WordPress's version of the WP_Block class is passing $block->parsed_block['innerBlocks']. Blocks such as Navigation return '' when inner_blocks isn't set. ie My FSE navigation menus don't work anymore. It looks like some extra shimming is needed.

[noisysocks]

​https://github.com/WordPress/wordpress-develop/pull/756 contains a fix.

[bobbingwide]

I tested this patch in my local development environment. My navigation menus are working again.

[azaozz]

This looks like a "logical companion" to WP_Block::__get(). However in the PHP manual there are some pretty strong, valid objections to using these magic methods: ​https://www.php.net/manual/en/language.oop5.overloading.php#allnotes.

Looks like in WP 5.5 WP_Block::__get() was used only to retrieve the block attributes: https://core.trac.wordpress.org/browser/tags/5.5/src/wp-includes/class-wp-block.php#L153, however in 5.6 it changed to get everything.

I'm probably missing something but why is that public __get() needed? The $parsed_block is passed in the constructor, so the get_attributes(), get_inner_blocks(), get_inner_content(), etc. would work without additional loading, lazy or not. What is the problem with setting the properties in the constructor at the same time as $parsed_block? (Looks like they will have to be set in the new WP_Block::reset().)

[noisysocks]

See #51612 for some background context here. (Apologies, I should have linked to this ticket.)

We want to call pre_render_block, render_block_data, and render_block_context in WP_Block::render() as it is not intuitive for these "render" filters to be called in the constructor. WP_Block::$inner_blocks, WP_Block::$inner_html, and WP_Block::$inner_content are all derived from the result of calling these filters.

This means we either have to:

1) Eagerly set the properties in the constructor and then re-set them in render(); or
2) Make the properties lazy.

I opted for (2) because (1) involves doing unnecessary work in the constructor and WP_Block::render() is a fairly warm path that is called when viewing a post.

Note that the dynamic properties are documented using @property PHPDoc attributes and so auto completion will still work for these properties in most IDEs.

[azaozz]

Replying to noisysocks:

We want to call pre_render_block, render_block_data, and render_block_context in WP_Block::render() as it is not intuitive for these "render" filters to be called in the constructor. WP_Block::$inner_blocks, WP_Block::$inner_html, and WP_Block::$inner_content are all derived from the result of calling these filters.

Thanks for the explanation :)

That seems to introduce two states of WP_Block, one with data from before the filters are run, and another after the filters (after render()). This seems to bring some back-compat problems: filters were run on the block data before WP_Block was instantiated, so the filtered, "correct" data was in the instance from the beginning. Now the "correct" data is available only after WP_Block::render() has been run.

Then as far as I see the magic __get() and __isset() are needed to support getting pre-filtered (wrong?) data?

Going to reopen #51612 with the above questions.

[noisysocks]

This is fixed by reverting the changes made in #51612 in [49694,49695].

This is fixed by reverting the changes made in #51612 in changesets 49694 and 49695.