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.

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 Hooks: Persist information about ignoredHookedBlocks in anchor block metadata #5712

Closed
Show file tree
Hide file tree
Changes from 14 commits
Commits
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
8 changes: 8 additions & 0 deletions src/wp-includes/block-template-utils.php
Original file line number Diff line number Diff line change
Expand Up @@ -894,6 +894,14 @@ function _build_block_template_result_from_post( $post ) {
}
}

$hooked_blocks = get_hooked_blocks();
Copy link
Member

Choose a reason for hiding this comment

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

If we leave out the change in this file, we could land the rest at any point. So the question is whether we can think of any potential issue with making the edited templates hookable?

Do we have user-created patterns covered?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

If we leave out the change in this file, we could land the rest at any point. So the question is whether we can think of any potential issue with making the edited templates hookable?

We can give it some smoke testing, plus maybe some more unit test coverage (e.g. in tests/phpunit/tests/block-templates/buildBlockTemplateResultFromPost.php) for increased confidence.

Do we have user-created patterns covered?

I haven't checked yet! I'm assuming that yes (since AFAIK their only "entry point" is the Patterns Registry class), but it'll be good to verify 👍

Copy link
Member

Choose a reason for hiding this comment

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

I think that leaves only Synced Blocks (aka Reusable Blocks), which are loaded through code similar to:

$reusable_block = get_post( $attributes['ref'] );
$content = do_blocks( $reusable_block->post_content );

if ( ! empty( $hooked_blocks ) || has_filter( 'hooked_block_types' ) ) {
$before_block_visitor = make_before_block_visitor( $hooked_blocks, $template );
$after_block_visitor = make_after_block_visitor( $hooked_blocks, $template );
$blocks = parse_blocks( $template->content );
$template->content = traverse_and_serialize_blocks( $blocks, $before_block_visitor, $after_block_visitor );
}

return $template;
}

Expand Down
37 changes: 33 additions & 4 deletions src/wp-includes/blocks.php
Original file line number Diff line number Diff line change
Expand Up @@ -757,6 +757,35 @@ function get_hooked_blocks() {
return $hooked_blocks;
}

/**
* Conditionally returns the markup for a given hooked block type.
*
* Accepts two arguments: A reference to an anchor block, and the name of a hooked block type.
* If the anchor block has already been processed, and the given hooked block type is in the list
* of ignored hooked blocks, an empty string is returned.
*
Copy link
Member

Choose a reason for hiding this comment

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

For consistency with other similar utilities, should it be marked as private, too?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done in f4379e1.

* @since 6.5.0
*
* @param array $anchor_block The anchor block. Passed by reference.
* @param string $hooked_block_type The name of the hooked block type.
* @return string The markup for the given hooked block type, or an empty string if the block is ignored.
*/
function get_hooked_block_markup( &$anchor_block, $hooked_block_type ) {
if ( ! isset( $anchor_block['attrs']['metadata']['ignoredHookedBlocks'] ) ) {
$anchor_block['attrs']['metadata']['ignoredHookedBlocks'] = array();
}

if ( in_array( $hooked_block_type, $anchor_block['attrs']['metadata']['ignoredHookedBlocks'] ) ) {
return '';
}

// The following is only needed for the REST API endpoint.
// However, its presence does not affect the frontend.
$anchor_block['attrs']['metadata']['ignoredHookedBlocks'][] = $hooked_block_type;

return get_comment_delimited_block_content( $hooked_block_type, array(), '' );
}

/**
* Returns a function that injects the theme attribute into, and hooked blocks before, a given block.
*
Expand Down Expand Up @@ -813,7 +842,7 @@ function make_before_block_visitor( $hooked_blocks, $context ) {
*/
$hooked_block_types = apply_filters( 'hooked_block_types', $hooked_block_types, $relative_position, $anchor_block_type, $context );
foreach ( $hooked_block_types as $hooked_block_type ) {
$markup .= get_comment_delimited_block_content( $hooked_block_type, array(), '' );
$markup .= get_hooked_block_markup( $parent_block, $hooked_block_type );
}
}

Expand All @@ -826,7 +855,7 @@ function make_before_block_visitor( $hooked_blocks, $context ) {
/** This filter is documented in wp-includes/blocks.php */
$hooked_block_types = apply_filters( 'hooked_block_types', $hooked_block_types, $relative_position, $anchor_block_type, $context );
foreach ( $hooked_block_types as $hooked_block_type ) {
$markup .= get_comment_delimited_block_content( $hooked_block_type, array(), '' );
$markup .= get_hooked_block_markup( $block, $hooked_block_type );
}

return $markup;
Expand Down Expand Up @@ -874,7 +903,7 @@ function make_after_block_visitor( $hooked_blocks, $context ) {
/** This filter is documented in wp-includes/blocks.php */
$hooked_block_types = apply_filters( 'hooked_block_types', $hooked_block_types, $relative_position, $anchor_block_type, $context );
foreach ( $hooked_block_types as $hooked_block_type ) {
$markup .= get_comment_delimited_block_content( $hooked_block_type, array(), '' );
$markup .= get_hooked_block_markup( $block, $hooked_block_type );
}

if ( $parent_block && ! $next ) {
Expand All @@ -888,7 +917,7 @@ function make_after_block_visitor( $hooked_blocks, $context ) {
/** This filter is documented in wp-includes/blocks.php */
$hooked_block_types = apply_filters( 'hooked_block_types', $hooked_block_types, $relative_position, $anchor_block_type, $context );
foreach ( $hooked_block_types as $hooked_block_type ) {
$markup .= get_comment_delimited_block_content( $hooked_block_type, array(), '' );
$markup .= get_hooked_block_markup( $parent_block, $hooked_block_type );
}
}

Expand Down
49 changes: 49 additions & 0 deletions tests/phpunit/tests/blocks/getHookedBlockMarkup
Original file line number Diff line number Diff line change
@@ -0,0 +1,49 @@
<?php
/**
* Tests for the get_hooked_block_markup function.
*
* @package WordPress
* @subpackage Blocks
*
* @since 6.5.0
*
* @group blocks
* @group block-hooks
*/
class Tests_Blocks_GetHookedBlockMarkup extends WP_UnitTestCase
{
/**
* @ticket 59646
*
* @covers ::get_hooked_block_markup
*/
public function test_get_hooked_block_markup_adds_metadata() {
$anchor_block = array(
'blockName' => 'tests/anchor-block',
);

$actual = get_hooked_block_markup( $anchor_block, 'tests/hooked-block' );
$this->assertSame( array( 'tests/hooked-block' ), $anchor_block['attrs']['metadata']['ignoredHookedBlocks'] );
$this->assertSame( '<!-- wp:tests/hooked-block /-->', $actual );
}

/**
* @ticket 59646
*
* @covers ::get_hooked_block_markup
*/
public function test_get_hooked_block_markup_if_block_is_already_hooked() {
$anchor_block = array(
'blockName' => 'tests/anchor-block',
'attrs' => array(
'metadata' => array(
'ignoredHookedBlocks' => array( 'tests/hooked-block' ),
),
)
);

$actual = get_hooked_block_markup( $anchor_block, 'tests/hooked-block' );
$this->assertSame( array( 'tests/hooked-block' ), $anchor_block['attrs']['metadata']['ignoredHookedBlocks'] );
$this->assertSame( '', $actual );
}
gziolo marked this conversation as resolved.
Show resolved Hide resolved
}
6 changes: 3 additions & 3 deletions tests/phpunit/tests/blocks/getHookedBlocks.php
Original file line number Diff line number Diff line change
Expand Up @@ -149,7 +149,7 @@ public function test_loading_template_with_hooked_blocks() {
$template->content
);
$this->assertStringContainsString(
'<!-- wp:post-content {"layout":{"type":"constrained"}} /-->'
'<!-- wp:post-content {"layout":{"type":"constrained"},"metadata":{"ignoredHookedBlocks":["tests/hooked-after"]}} /-->'
. '<!-- wp:tests/hooked-after /-->',
$template->content
);
Expand All @@ -176,7 +176,7 @@ public function test_loading_template_part_with_hooked_blocks() {

$this->assertStringContainsString(
'<!-- wp:tests/hooked-before /-->'
. '<!-- wp:navigation {"layout":{"type":"flex","setCascadingProperties":true,"justifyContent":"right"}} /-->',
. '<!-- wp:navigation {"layout":{"type":"flex","setCascadingProperties":true,"justifyContent":"right"},"metadata":{"ignoredHookedBlocks":["tests/hooked-before"]}} /-->',
$template->content
);
$this->assertStringNotContainsString(
Expand Down Expand Up @@ -215,7 +215,7 @@ public function test_loading_pattern_with_hooked_blocks() {
$pattern['content']
);
$this->assertStringContainsString(
'<!-- wp:comments -->'
'<!-- wp:comments {"metadata":{"ignoredHookedBlocks":["tests/hooked-first-child"]}} -->'
. '<div class="wp-block-comments">'
. '<!-- wp:tests/hooked-first-child /-->',
str_replace( array( "\n", "\t" ), '', $pattern['content'] )
Expand Down
18 changes: 7 additions & 11 deletions tests/phpunit/tests/blocks/wpBlockPatternsRegistry.php
Original file line number Diff line number Diff line change
Expand Up @@ -381,14 +381,12 @@ public function test_get_all_registered_includes_hooked_blocks() {
$pattern_three['name'] = 'test/three';
$pattern_three['content'] .= '<!-- wp:tests/my-block /-->';

$expected = array(
$pattern_one,
$pattern_two,
$pattern_three,
);

$registered = $this->registry->get_all_registered();
$this->assertSame( $expected, $registered );
$this->assertCount( 3, $registered );
$this->assertStringEndsWith( '<!-- wp:tests/my-block /-->', $registered[1]['content'] );
$this->assertStringContainsString( '"metadata":{"ignoredHookedBlocks":["tests/my-block"]}', $registered[1]['content'] );
$this->assertStringEndsWith( '<!-- wp:tests/my-block /-->', $registered[2]['content'] );
$this->assertStringContainsString( '"metadata":{"ignoredHookedBlocks":["tests/my-block"]}', $registered[2]['content'] );
}

/**
Expand Down Expand Up @@ -444,11 +442,9 @@ public function test_get_registered_includes_hooked_blocks() {
);
$this->registry->register( 'test/two', $pattern_two );

$pattern_one['name'] = 'test/one';
$pattern_one['content'] = '<!-- wp:tests/my-block /-->' . $pattern_one['content'];

$pattern = $this->registry->get_registered( 'test/one' );
$this->assertSame( $pattern_one, $pattern );
$this->assertStringStartsWith( '<!-- wp:tests/my-block /-->', $pattern['content'] );
$this->assertStringContainsString( '"metadata":{"ignoredHookedBlocks":["tests/my-block"]}', $pattern['content'] );
}

/**
Expand Down
Loading