Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Block supports: refactor how the block to render is tracked to improve core/plugin compatibility #26356

Merged
merged 10 commits into from
Oct 22, 2020
Merged

Block supports: refactor how the block to render is tracked to improve core/plugin compatibility #26356

Show file tree
Hide file tree
Changes from 9 commits
Commits
Show all changes
10 commits
Select commit Hold shift + click to select a range
a08d133
Convert the global to a class property
oandregal Oct 21, 2020
886be87
Rename and make it static
oandregal Oct 21, 2020
fad54c0
Remove use of $current_parsed_block
oandregal Oct 21, 2020
12067f6
Remove use of $current_parsed_block
oandregal Oct 21, 2020
9b4c70f
Make linter happy
oandregal Oct 21, 2020
d7bc1ce
Move track code out of the class
oandregal Oct 21, 2020
07b9214
Make linter happy
oandregal Oct 22, 2020
00f7e56
Fix test
oandregal Oct 22, 2020
7dffcdd
Make linter happy
oandregal Oct 22, 2020
ad18bb4
Remove tests, these are already migrated to core
oandregal Oct 22, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
42 changes: 36 additions & 6 deletions lib/class-wp-block-supports.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,13 @@ class WP_Block_Supports {
*/
private $block_supports = array();

/**
* Tracks the current block to be rendered.
*
* @var array
*/
public static $block_to_render = null;

/**
* Container for the main instance of the class.
*
Expand Down Expand Up @@ -67,13 +74,12 @@ public function register( $block_support_name, $block_support_config ) {
* Generates an array of HTML attributes, such as classes, by applying to
* the given block all of the features that the block supports.
*
* @param array $parsed_block Block as parsed from content.
* @return array Array of HTML attributes.
*/
public function apply_block_supports( $parsed_block ) {
$block_attributes = $parsed_block['attrs'];
public function apply_block_supports() {
$block_attributes = self::$block_to_render['attrs'];
$block_type = WP_Block_Type_Registry::get_instance()->get_registered(
$parsed_block['blockName']
self::$block_to_render['blockName']
);

// If no render_callback, assume styles have been previously handled.
Expand Down Expand Up @@ -144,8 +150,7 @@ private function register_attributes() {
* @return string String of HTML classes.
*/
function get_block_wrapper_attributes( $extra_attributes = array() ) {
global $current_parsed_block;
$new_attributes = WP_Block_Supports::get_instance()->apply_block_supports( $current_parsed_block );
$new_attributes = WP_Block_Supports::get_instance()->apply_block_supports();

if ( empty( $new_attributes ) && empty( $extra_attributes ) ) {
return '';
Expand Down Expand Up @@ -191,4 +196,29 @@ function get_block_wrapper_attributes( $extra_attributes = array() ) {
return implode( ' ', $normalized_attributes );
}

/**
* Callback hooked to the register_block_type_args filter.
*
* This hooks into block registration to wrap the render_callback
* of dynamic blocks with a closure that keeps track of the
* current block to be rendered.
*
* @param array $args Block attributes.
* @return array Block attributes.
*/
function wp_block_supports_track_block_to_render( $args ) {
if ( null !== $args['render_callback'] ) {
$block_render_callback = $args['render_callback'];
$args['render_callback'] = function( $attributes, $content, $block ) use ( $block_render_callback ) {
$parent_block = WP_Block_Supports::$block_to_render;
WP_Block_Supports::$block_to_render = $block->parsed_block;
$result = $block_render_callback( $attributes, $content, $block );
WP_Block_Supports::$block_to_render = $parent_block;
return $result;
};
}
return $args;
}

add_action( 'init', array( 'WP_Block_Supports', 'init' ), 22 );
add_filter( 'register_block_type_args', 'wp_block_supports_track_block_to_render' );
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do you think that filter is better where it was in compat.php because it's specifically about that right? compatibility with WP versions that don't support that variable yet.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yup:

  • if the plugin is active on a WP version that doesn't have this class => this will be loaded.
  • if the plugin is active on a WP version that does have the class => this won't be loaded.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

oh, but do we need to signal anywhere that this class shouldn't be loaded when the plugin requires WP 5.6? I'm not sure how that works.

Copy link
Member Author

@oandregal oandregal Oct 22, 2020 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

one thing we could do would be to move this code from load.php:

if ( ! class_exists( 'WP_Block_Supports' ) ) {
	require_once dirname( __FILE__ ) . '/class-wp-block-supports.php';
}

to compat.php (and add the comment about WP 5.6).

What do you think?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

One of the goals is to only set up the register_block_type_args filter when the WP_Block_Supports class loaded in the runtime is the one from the plugin and not core, correct? If so, what if the plugin's version of the class provided its own wp_block_supports_track_block_to_render as a static method? That way, we would have:

  • Conditional class loading in load.php, as is the case for all other classes
  • In compat.php, something like:
if ( class_exists( 'WP_Block_Supports' ) && method_exists( 'WP_Block_Supports', 'track_block_to_render' ) ) {
    add_filter( 'register_block_type_args', array( 'WP_Block_Support', 'track_block_to_render' ) );
}

Or am I missing something?

The advantages are that we preserve normal loading, and avoid any arbitrary signals to instead use duck-typing (i.e. merely requiring the method to exist in order to register it as a filter).

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

One of the goals is to only set up the register_block_type_args filter when the WP_Block_Supports class loaded in the runtime is the one from the plugin and not core, correct?

I would think this PR should also be ported to core for 5.6 as it is. Following that rationale my answer would be no.

Sure, there's the edge case of WP 5.6 beta 1, which already has a class different to this one. I'd argue that's a short-lived version for testing purposes, though. Anyway, the issue with inner blocks will be fixed anyway (because it's already fixed in core beta 1) and we don't need the current code in compat.php.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fair enough. :)

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should we go ahead and merge this, then? I've already ported the changes to WordPress/wordpress-develop#640 Would benefit from some orientation as to how to land that one.

12 changes: 3 additions & 9 deletions lib/class-wp-block.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -203,7 +203,6 @@ public function __get( $name ) {
*/
public function render( $options = array() ) {
global $post;
global $current_parsed_block;
$options = array_replace(
array(
'dynamic' => true,
Expand All @@ -217,14 +216,9 @@ public function render( $options = array() ) {
if ( ! $options['dynamic'] || empty( $this->block_type->skip_inner_blocks ) ) {
$index = 0;
foreach ( $this->inner_content as $chunk ) {
if ( is_string( $chunk ) ) {
$block_content .= $chunk;
} else {
$parent_parsed_block = $current_parsed_block;
$current_parsed_block = $this->inner_blocks[ $index ]->parsed_block;
$block_content .= $this->inner_blocks[ $index++ ]->render();
$current_parsed_block = $parent_parsed_block;
}
$block_content .= is_string( $chunk ) ?
$chunk :
$this->inner_blocks[ $index++ ]->render();
}
}

Expand Down
37 changes: 0 additions & 37 deletions lib/compat.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -508,40 +508,3 @@ function gutenberg_override_reusable_block_post_type_labels() {
);
}
add_filter( 'post_type_labels_wp_block', 'gutenberg_override_reusable_block_post_type_labels', 10, 0 );

global $current_parsed_block;
$current_parsed_block = array(
'blockName' => null,
'attributes' => null,
);

/**
* Wraps the render_callback of dynamic blocks to keep track
* of the current block being rendered via a global variable
* called $current_parsed_block.
*
* This is for get_block_wrapper_attributes to get access
* to the runtime data of the block being rendered.
*
* This shim can be removed when the plugin requires WordPress 5.6.
*
* @since 9.2.1
*
* @param array $args Block attributes.
* @return array Block attributes.
*/
function gutenberg_current_parsed_block_tracking( $args ) {
if ( null !== $args['render_callback'] ) {
$block_render_callback = $args['render_callback'];
$args['render_callback'] = function( $attributes, $content, $block ) use ( $block_render_callback ) {
global $current_parsed_block;
$parent_parsed_block = $current_parsed_block;
$current_parsed_block = $block->parsed_block;
$result = $block_render_callback( $attributes, $content, $block );
$current_parsed_block = $parent_parsed_block;
return $result;
};
}
return $args;
}
add_filter( 'register_block_type_args', 'gutenberg_current_parsed_block_tracking' );
10 changes: 6 additions & 4 deletions phpunit/class-block-supported-styles-test.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -94,12 +94,14 @@ private function get_content_from_block( $block ) {
/**
* Returns the rendered output for the current block.
*
* @param array $block Block to render.
* @param WP_Block $block Block to render.
*
* @return string Rendered output for the current block.
*/
private function render_example_block( $block ) {
global $current_parsed_block;
$current_parsed_block = $block;
$wrapper_attributes = get_block_wrapper_attributes(
WP_Block_Supports::init();
WP_Block_Supports::$block_to_render = $block;
$wrapper_attributes = get_block_wrapper_attributes(
array(
'class' => 'foo-bar-class',
'style' => 'test: style;',
Expand Down