Block Hooks: Persist information about ignoredHookedBlocks in anchor block metadata

[ockham]

The biggest tradeoff we made in the implementation of Block Hooks was that we had to limit them to templates, template parts, and patterns that didn't have any user modifications (see #59313 for the reason). We would like to remove this limitation, so they’ll also work with user-modified templates, parts, and patterns.

The crucial problem we need to solve is to acknowledge if a user has opted to remove or persist a hooked block, so that the auto-insertion mechanism won't run again and inject an extraneous hooked block on the frontend when none is solicited.

This is achieved by storing all known blocks hooked to a given anchor block in the metadata attribute on that anchor block; specifically in a field called ignoredHookedBlocks inside of metadata. Hooked blocks are only rendered on the frontend when they're absent from that field; OTOH, they're injected into that field (via the REST API) when first loaded in the editor.

This simple logic guarantees that once a user modifies a given template or template part, those changes are respected on the frontend; yet if a plugin that includes a hooked block is activated after those modifications have taken place, the hooked block will be rendered on the frontend. This new technique supplants the one previously used (i.e. rendering hooked blocks on the frontend only if a template or template part doesn't have any modifications) in a rather direct way.

Note that this PR only introduces the new metadata attribute; it does not yet enable hooked block insertion into modified layouts (see #5712 (comment) for the rationale).

Alternative approach to #5523. See #5609 (comment) for the rationale.

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

---

This Pull Request is for code review only. Please keep all other discussion in the Trac ticket. Do not merge this Pull Request. See GitHub Pull Requests for Code Review in the Core Handbook for more details.

[gziolo]

Alternative approach to #5523. See #5609 (comment) for the rationale.

The proposed alternative looks very appealing. I need to think more about it, but on the first look I would go with this approach.

[gziolo]

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?

[ockham]

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 👍

[gziolo]

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 );

[gziolo]

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

[ockham]

Done in f4379e1.

[gziolo]

It's less important, but the Ci job reports some coding style violations.

The concept introduced around ignoredHookedBlocks on the anchor block works great, and I'm unable to find any drawbacks of this approach on the server side. If you still favor the new helper proposed get_hooked_block_markup (most likely annotated as private), feel free to commit it together with related unit tests.

I'm a bit hesitant to open modified templates todat to fully integrate with Block Hooks because of a single edge discovered. Everything works like charm with the special ignoredHookedBlocks metadata value for existing and modified blocks. The only blocker is the case where a new block gets inserted in the editor that is also an anchor block. We discussed with Bernie possible solutions and it isn't that trivial. However, I'm leaning towards trying to avoid auto-inserting hooked blocks for the newly inserted anchor blocks and finding a way to annotate it with the list of hooked blocks to make it as close as possible to what happens on the server. The only limitation on the client is no ability to apply WP filter that can modify the list of hooked blocks for a certain target position.

[ockham]

I'm a bit hesitant to open modified templates todat to fully integrate with Block Hooks because of a single edge discovered. Everything works like charm with the special ignoredHookedBlocks metadata value for existing and modified blocks. The only blocker is the case where a new block gets inserted in the editor that is also an anchor block. We discussed with Bernie possible solutions and it isn't that trivial. However, I'm leaning towards trying to avoid auto-inserting hooked blocks for the newly inserted anchor blocks and finding a way to annotate it with the list of hooked blocks to make it as close as possible to what happens on the server. The only limitation on the client is no ability to apply WP filter that can modify the list of hooked blocks for a certain target position.

Here's a short screencast to illustrate the issue:

[ockham]

As stated in this comment,

I've filed #60008 to have a dedicated ticket for introducing the ignoredBlockHooks metadata, without actually enabling the new technique for modified layouts. For the latter, we'll continue to use this ticket.

I'll link this PR to the new ticket, will update the title and description, and remove the changes to _build_block_template_result_from_post.

[ockham]

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