Block Hooks: `"multiple": false` not respected

[Bernhard Reiter]

Block Hooks currently do not respect the "multiple": false setting: If this is set in a prospective hooked block's block.json, and there's already an instance of that block present in a given context (i.e. template, template part, pattern, or navigation menu), then Block Hooks should not insert another instance of that block.

In a similar vein, if no other instance of that block is present in the given context, then Block Hooks should insert only one instance of that block -- i.e. the first time a matching anchor block is encountered, but not for any other matching anchor blocks.

This will help extenders avoid checking for the presence of another instance of their hooked block inside of the hooked_block_types filter, which is called for every block found in a given context -- thus introducing an unnecessary performance overhead.

[Bernhard Reiter]

In order to find whether a given block is already present in the context, we might use the new helpers proposed in ​https://github.com/WordPress/wordpress-develop/pull/6760.

[Bernhard Reiter]

Generally, this check is probably best added to apply_block_hooks_to_content; however, we should make sure that that function is used pervasively to inject hooked blocks. (It isn't yet, e.g. for Patterns and the Navigation block.)

In the Patterns registry, use apply_block_hooks_to_content instead of the WP_Block_Patterns_Registry class's private get_content method. (The latter is removed at part of this PR.)

To that end, apply_block_hooks_to_content is amended to inject the theme attribute into Template Part blocks, even if no hooked blocks are present.

This kind of centralization is required as a preparation for https://core.trac.wordpress.org/ticket/61902.

Trac ticket: https://core.trac.wordpress.org/ticket/61902.

WIP. Details to follow.

Trac ticket: https://core.trac.wordpress.org/ticket/61902

I have opened this for review, as it's going to be a prerequisite for ​https://github.com/WordPress/wordpress-develop/pull/7296 (to which I'll push some commits shortly that will implement the required change inside of apply_block_hooks_to_content, as that's the most suitable location for it).

I haven't yet added dedicated unit test coverage for apply_block_hooks_to_content in this PR, but note that it's covered indirectly by unit tests for the functions where it is called from, e.g.:

• _build_block_template_result_from_file: covered in buildBlockTemplateResultFromFile.php
• _build_block_template_result_from_post: covered in buildBlockTemplateResultFromPost.php
• inject_ignored_hooked_blocks_metadata_attributes: covered in injectIgnoredHookedBlocksMetadataAttributes.php
• update_ignored_hooked_blocks_postmeta: covered in updateIgnoredHookedBlocksPostMeta.php
• WP_Block_Patterns_Registry::get_registered: covered in wpBlockPatternsRegistry.php
• WP_Block_Patterns_Registry::get_all_registered: covered in wpBlockPatternsRegistry.php

Alternative to ​https://github.com/WordPress/wordpress-develop/pull/7296.

TODO:

• [ ] Cache
• [ ] Cover case where block isn't present at first, but multiple anchor blocks are (and so it would be inserted more than once).

Closing in favor of #7443.

These changes make sense, it seems to be mostly shifting some code around so that apply_glock_hooks_to_content is used more consistently.

Thank you very much for reviewing and approving!

I'm not especially well versed in templates or block hooks, so if there's additional testing I can do I'd be happy to.

I just did a bunch of smoke-testing and I'm confident enough to merge this, but thank you for offering! FWIW, I typically use my own ​Like Button block plugin to test (which has code to use Block Hooks to automatically insert it after the Post Content block), or something like the following to inject the Login/out block into the Navigation menu. I then check if hooked blocks are inserted both on the frontend and in the editor, and move them around in the editor to see if those changes are respected on the frontend.

function register_logout_block_as_navigation_last_child( $hooked_blocks, $position, $anchor_block, $context ) {
        if ( $anchor_block === 'core/navigation' && $position === 'last_child' ) {
                $hooked_blocks[] = 'core/loginout';
        }

        return $hooked_blocks;
}
add_filter( 'hooked_block_types', 'register_logout_block_as_navigation_last_child', 10, 4 );

[Bernhard Reiter]

In 59101:

Block Hooks: apply_block_hooks_to_content in Patterns, Templates.

In the Patterns registry, use apply_block_hooks_to_content (introduced in [58291]) instead of the WP_Block_Patterns_Registry class's private get_content method. (The latter is removed as part of this changeset.)

In a similar vein, use apply_block_hooks_to_content in the _build_block_template_result_from_file and _build_block_template_result_from_post functions, respectively.

For that to work, apply_block_hooks_to_content is amended to inject the theme attribute into Template Part blocks, even if no hooked blocks are present.

This kind of centralization is required as a preparation for #61902.

Props bernhard-reiter, jonsurrell.
See #61902.

Committed to Core in https://core.trac.wordpress.org/changeset/59101.

There's one more instance where we should call apply_block_hooks_to_content (rather than invoke more low-level Block Hooks related functions): The Navigation block in Gutenberg. I've filed a PR: ​https://github.com/WordPress/gutenberg/pull/65703.

Note to self, this is going to need a Dev Note.

Tested with the Like Button and the following changes:

I can confirm that the block gets only inserted once in the related area of the template or pattern:

Well, it all works as expected on the templating level. In the final page rendered on the site, it's difficult to change some of the behavios like the Comments block using the template from Comment template that works as a repeater field, so that's why we still see multiple instances of the Like block.

Thank you very much for reviewing and approving!

Well, it all works as expected on the templating level. In the final page rendered on the site, it's difficult to change some of the behaviors like the Comments block using the template from Comment template that works as a repeater field, so that's why we still see multiple instances of the Like block.

Right, that's a good point that hadn't occurred to me. The same template part is used multiple times — but since we only enforce single-instance insertion on a per-context (here: per-template-part) basis, there is still one Like Button per template part — and thus, multiple Like buttons in the Comments section, as you said 😅

I'm not sure there's an easy way to change this so there would be only one instance in the entire rendered output, as that would have to work across context boundaries. We might look into that later (for 6.8?) -- assuming we can even easily define the desired behavior in that case -- but it's clearly out of scope for 6.7.

I'll go and commit this change 😄

I'm not sure there's an easy way to change this so there would be only one instance in the entire rendered output, as that would have to work across context boundaries. We might look into that later (for 6.8?) -- assuming we can even easily define the desired behavior in that case -- but it's clearly out of scope for 6.7.

To be fair, it's exactly the same problem that exists in the editor. It's even more difficult to resolve there as there are multiple factors involved, like you can use a zoom out mode to edit only a pattern, or template part, etc. However, the more challenging factor is that some aspects of the theme might change through external updates, and the same applies to the content. At the end of the day, it all has to be assembled into a single page on the site when rendering for the visitor. It's rather impossible to find a way to represent that in the UI that such block might or might not display in the future.

[Bernhard Reiter]

In 59124:

Block Hooks: Respect "multiple": false in hooked blocks.

If a prospective hooked block has its multiple block-supports field set to false (thus allowing only one instance of the block to be present), ensure that:

1. Only one instance of the block will be inserted if it's not yet present in the current context.
2. The block will not be inserted at all if an instance of it is already present in the current context.

As always in Block Hooks parlance, "context" denotes the containing template, template part, pattern, or navigation post that a hooked block is supposed to be inserted into.

The markup of a webpage that uses a Block Theme typically comprises a number of such contexts -- one template and any number of template parts, patterns, and navigation posts. Note that the limitation imposed by this changeset only applies on a per-context basis, so it's still possible that the resulting page contains more than one instance of a hooked block with "multiple": false set, as each context could contribute up to one such instance.

Props bernhard-reiter, jonsurrell, gziolo.
Fixes #61902.

Committed to Core in https://core.trac.wordpress.org/changeset/59124.

[Bernhard Reiter]

Dev Note draft available ​here.

[fabiankaegy]

@bernhard-reiter the dev note looks good to me :) From my POV this is ready to be published 🚀

[Bernhard Reiter]

Thank you! It's now ​published :)