WordPress.org

Make WordPress Core

Opened 4 months ago

Last modified 3 weeks ago

#48104 new enhancement

Add the gutenberg_provide_render_callback_with_block_object shim to core

Reported by: andraganescu Owned by:
Milestone: 5.4 Priority: normal
Severity: normal Version:
Component: Editor Keywords: has-patch needs-testing needs-dev-note needs-docs
Focuses: Cc:
PR Number:

Description (last modified by 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

Attachments (1)

48104.diff (1.5 KB) - added by andraganescu 4 months ago.

Download all attachments as: .zip

Change History (9)

#1 @andraganescu
4 months ago

  • Description modified (diff)

#2 @andraganescu
4 months ago

  • Description modified (diff)

@andraganescu
4 months ago

#3 @andraganescu
4 months ago

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.

#4 @andraganescu
4 months ago

  • Keywords has-patch needs-testing needs-dev-note needs-docs added; needs-patch removed

#5 @youknowriad
4 months ago

  • Milestone changed from 5.3 to 5.4

#6 @noisysocks
4 months ago

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.

#7 @aduth
6 weeks ago

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.

#8 @andraganescu
3 weeks ago

@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.

Note: See TracTickets for help on using tickets.