Pass content to block render callback

[mattheu]

Description

See #5760

Support for passing of a blocks content to the render block was removed in #4591

How has this been tested?

• Locally I used the docker dev environment provided by the Gutenberg project.
• I created a custom block to test this.
• I tested single and multiple instances of the same block, as well as with other blocks on the page.
• I updated the custom block to support inner content.
• I wrote PHP unit tests for the changes.

Types of changes

This is not a breaking change.
It simply passes the block content to the render callback as the second param, in addition to the attributes which are passed currently.
For self closing blocks, this will always be an empty string.

Checklist:

• My code is tested.
• My code follows the WordPress code style.
• My code follows the accessibility standards.
• My code has proper inline documentation.

[danielbachhuber]

It might be useful to pass the $block_type instance too.

One aspect to consider (although not a blocker): if someone is using an existing shortcode render callback for their dynamic block, this could have unintended consequences.

[johnbillion]

It might be useful to pass the $block_type instance too.

Definitely. See #4673. Needs updating since the changes made in #4591.

[dmsnell]

If we removed them can we share a justification to bring them back as well as an explanation at a high-level on how this PR addresses the needs that provoked taking the content out in the first place?

[mattheu]

I've updated this code to also pass the block name to the render callback, taking code from #4673

The justification for bringing this back?

• Why would dynamic blocks not have access to the block content?
• Requirement for dynamic blocks to be only partially dynamic. Maybe setting a class dynamically, or appending content to a block.
• Dynamic blocks with nested content.
• Some other use cases are explored in the ticket: Pass content to render callback #5760

Why was this removed in the first place? It's not completely clear to me from the pull request in which it was removed (#4591).

Is the concern is that for a dynamic block with nested content, the nested content is not parsed into blocks? It would be up to your code to handle calling do_blocks. In my opinion, this would be OK.

[mattheu]

Passing the block name would also be useful if you are trying to create a generic render callback that handles a few block types. My use case here is that the render_callback simply includes a template file for that block.

This functionality is achievable with a closure.

RE @danielbachhuber's concerns:

If someone is using an existing shortcode render callback for their dynamic block, this could have unintended consequences.

What are your concerns? That the value of $tag will refer to a block and not a shortcode? I think it would only cause an issue if the code was checking the value of $tag and expecting it to be empty - which seems a bit of an edge case.

[dmsnell]

@aduth maybe you can provide some historical context here. I'd like to avoid making slinky-commits to undo and redo changes different people are making.

@mattheu to note, I think dynamic blocks should have their content, but I want to make sure we don't skip over something we are unaware of because we didn't ask.

[aduth]

The behavior prior to #4591 was such that displaying a post on the front-end consisted of reconstructing all of block content from the parsed object. This is why WP_Block_Type::render had more logic to handle both static and dynamic blocks, the former being the only one where $content had been used. With #4591 turning the rendering into more of a "seek and replace dynamic", we weren't manipulating static blocks at all (aside from stripping comments, which was a later addition), and thus the responsibility (and the $content parameter) was removed from WP_Block_Type::render.

I don't have an issue with making more available to the render callback. What I might propose is:

• If WP_Block_Type::render is receiving $content and also considering whether the block is dynamic, should we return the $content rather than an empty string, as existed prior to Framework: Skip server grammar parse, seek and replace dynamic blocks #4591 ? In practice, we're not using this behavior, but in case someone calls render on a static block, it seems reasonable to return the content.
• As mentioned at Pass the block name as a parameter to the dynamic block render callback #4673 (comment) and Pass content to block render callback #6239 (comment), rather than cherry-picking properties to pass as arguments, why not just pass $this in its entirety as the argument.

[dmsnell]

Also tangentially related is #6170

[mattheu]

Updated following suggestions by @aduth

• Pass $this to render_callback as 3rd param.
• Return $content instead of empty string, just in case render is ever called on a static block.

[danielbachhuber]

If someone is using an existing shortcode render callback for their dynamic block, this could have unintended consequences.

What are your concerns? That the value of $tag will refer to a block and not a shortcode? I think it would only cause an issue if the code was checking the value of $tag and expecting it to be empty - which seems a bit of an edge case.

I meant to include an example, but I was lazy. I'll include one now:

function render_callback( $atts, $content = '' ) {
	$output = '';
	// Do some behavior if $content isn't empty
	if ( $content ) {
		// Render something
		return $output;
	}
	// Otherwise, parse $atts and do something else
	return $output;
}

add_action( 'init', array(
	add_shortcode( 'foo-bar', 'render_callback' );
	register_block_type( 'foo/foobar', array(
		'render_callback' => 'render_callback',
	) );
) );

Based on the current pull request, render_callback() would exhibit different behavior with this change. However, I don't think this is necessarily a bad thing — just something to be aware of and communicate properly.

[gschoppe]

Rather than provide content as a string, it should be an array of child blocks and boundary html, so that dynamic blocks can potentially contain children. This would be crucial to fully match the functionality currently available with shortcodes.

This would be needed for functions like a "members only" block, which would render child blocks only if the current user is a specific role, or a "conditional content" block, where different child blocks would be shown depending on custom conditionals.

[dmsnell]

Somewhat random, but if and as we incorporate attribute parsing on the server side as well the $content could become less important. That is, the JS edit and save functions don't need the HTML string so neither does the PHP intrinsically. We can think about having the server fully parse the attributes and pass them into the callback.

[gschoppe]

@dmsnell This would be a great idea if the data-structure didn't encourage storing parts of a block's editable features in a flat HTML blob. The structure would need to be more like the MobileDoc specification to truly make it render-agnostic.

[danielbachhuber]

Pass content to block render callback

This particular implementation detail was handled in 68519eb (#8077)

The remaining details are being discussed in #8352