Add the gutenberg_provide_render_callback_with_block_object shim to core

[andraganescu]

Part of making the navigation block dynamic was adding a new filter at pre_render_block which enhances the way blocks are passed to the server render callback by passing render_callback the block object as the argument.

We should to update WP Core with these changes and remove them from the Gutenberg compat lib ​https://github.com/WordPress/gutenberg/blob/95e769df1f82f6b0ef587d81af65dd2f48cd1c38/lib/compat.php#L33

[andraganescu]

So the shim in Gutenberg compat.php only sends $block to the render method of WP_Block_Type. 48104.diff adds this to render_block and an argument to the render method of WP_Block_Type.

[noisysocks]

48104.diff​ looks good to me. I wonder, though, if we should move away from giving third parties the raw block array and instead create a WP_Block class to encapsulate that data. This would make the API nicer (e.g. $block->attributes instead of $block['attrs']), and would help developers avoid nasty surprises like default block attributes not being set. We could consider updating parse_blocks() to also return WP_Block.

What do you think?

At any rate, since this isn't needed for 5.3, let's punt on this for now and focus on landing it when trunk opens for 5.4.

[aduth]

Part of making the navigation block dynamic was adding a new filter at pre_render_block which enhances the way blocks are passed to the server render callback by passing render_callback the block object as the argument.

Can you elaborate on why these changes were needed? At a glance, the $block parameter seems highly redundant with the previous parameters (which are effectively $block['attrs'] and a derivation of $block['innerContent'] respectively). Just seems like we're adding more, without having a good sense of what we're expecting people to be using this function for. Or it's a scenario where, in retrospect, we should have just given the $block object as the sole parameter and left it to the block to deal with.

I wonder if there's also another option for these blocks where, instead of adding this through the block interface render callback, a block can choose to implement their own advanced rendering by applying their own hooks to render_block filter, etc.

[andraganescu]

@aduth I tried and tried by can't remember all the details, other than the main issue being that innerBlocks were not passed into the render callback so we could not loop through them to make the navigation nested lists.

I believe you're right at this point:

it's a scenario where, in retrospect, we should have just given the $block object as the sole parameter and left it to the block to deal with.

Indeed, allowing blocks to set their own hooks to render_block filter might be a great idea, but more usecases could let us know of how this will be used.

[jorgefilipecosta]

It seems currently on wp-trunk the navigation block does not work on the front end. The block is not rendered(on the plugin it works). I think the problem affecting the block _render parameter issue. The navigation block will land on WordPress 5.4, so we will need to land a fix, even if temporary.

@andraganescu what are your thoughts on the option @aduth suggested:

I wonder if there's also another option for these blocks where, instead of adding this through the block interface render callback, a block can choose to implement their own advanced rendering by applying their own hooks to render_block filter, etc.

It seems this option suggested by @aduth does not involve a public API change. Is that right @aduth? @andraganescu, what are your thoughts on this option?

[aduth]

It seems this option suggested by @aduth does not involve a public API change. Is that right @aduth?

Correct, it was my hope that it wouldn't require any public API changes.

[andraganescu]

Well, @aduth @jorgefilipecosta the way to find out is to test and see if there is any issue. I will open a PR and see if the Navigation block can instead work instead by hooking into render_block filter as indeed that filter passes the whole $block so the rest of the logic should be unchanged. I will post the PR here as well asap!

[andraganescu]

@aduth @jorgefilipecosta ​here is a PR implementing the hook into the render_block filter. It renders fine without the shim, so that is an option - forgot how PR's link to Trac!

Not sure about the fact that we will call one function for every block render, cost should be minimal since we bail early if the block is not the one we need.

[andraganescu]

This ticket is not needed anymore so I closed it. We don't need any API updates on the block rendering callback execution because of the better way for advanced rendering which is to "hook" into the render_block filter and use the whole block which exists there.

[desrosj]

This ticket was closed without requiring changes. Removing needs-dev-note.