Block Hooks API: Consolidate approach to get all hooked blocks

[tjcafferkey]

Our current approach to getting hooked blocks has two parts to it:

We use get_hooked_blocks() to get all of the hooked blocks which are hooked from the block.json file. In function insert_hooked_blocks we apply the filter callbacks (passing a derived value from step 1 as the default value) to get hooked blocks that are applied by the hooked_block_types filter.

Currently it's a bit confusing and without knowledge you'd assume get_hooked_blocks() would retrieve all of the hooked blocks. Additionally it feels like you have to run a few different pieces of code separately and combine their resulting values to get a complete picture which could result in some bugs or unnecessary complexities.

This PR aims to fix that by creating a single function to get hooked blocks based on anchor block and position.

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

---

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.

[github-actions[bot]]

Test using WordPress Playground

The changes in this pull request can previewed and tested using a WordPress Playground instance.

WordPress Playground is an experimental project that creates a full WordPress instance entirely within the browser.

Some things to be aware of

• The Plugin and Theme Directories cannot be accessed within Playground.
• All changes will be lost when closing a tab with a Playground instance.
• All changes will be lost when refreshing the page.
• A fresh instance is created each time the link below is clicked.
• Every time this pull request is updated, a new ZIP file containing all changes is created. If changes are not reflected in the Playground instance, it's possible that the most recent build failed, or has not completed. Check the list of workflow runs to be sure.

For more details about these limitations and more, check out the Limitations page in the WordPress Playground documentation.

Test this pull request with WordPress Playground.

[ockham]

I think this approach (i.e. the newly introduced function) makes sense 👍

[github-actions[bot]]

The following accounts have interacted with this PR and/or linked issues. I will continue to update these lists as activity occurs. You can also manually ask me to refresh this list by adding the props-bot label.

Core Committers: Use this line as a base for the props when committing in SVN:

Props bernhard-reiter, tomjcafferkey.

To understand the WordPress project's expectations around crediting contributors, please review the Contributor Attribution page in the Core Handbook.

[tjcafferkey]

@ockham I believe this is ready for rereview 🙏🏻

[ockham]

I'll rebase.

[ockham]

The major problem to solve here is performance. FWIW, make_before_block_visitor and make_before_after_visitor didn't always accept $hooked_blocks as an argument; we added it later (in https://github.com/WordPress/wordpress-develop/pull/5399) as an optimization, since we were concerned that get_hooked_blocks() would be called too often (from those visitors).

At the time, we decided against caching the result of get_hooked_blocks(), since it was deemed too error-prone. I seem to remember one specific scenario where caching would actually cause issues: get_hooked_blocks() iterates over all known (registered) blocks to look for blockHooks fields. The problem was that get_hooked_blocks() ended up getting called before all blocks were registered (I think via the Template Part block's PHP code somehow), which meant that any blocks that were registered afterwards wouldn't be taken into account.

(Generally, people were skeptical about caching things that are depending on registered blocks, as that had turned out tricky in the past.)

For the above reasons, I'm a bit skeptical about caching inside of get_hooked_blocks, as that would be affected by the "called before all blocks are registered"-problem. Maybe we can cache at a different level? 🤔 It's a tricky problem, since the whole point of this PR is to bundle functionality in get_hooked_blocks_by_anchor_block() (which by definition is called for each anchor block), which is at odds with having something called once (at most) per tree traversal 😕

[tjcafferkey]

At the time, we decided against caching the result of get_hooked_blocks(), since it was deemed too error-prone. I seem to remember one specific scenario where caching would actually cause issues: get_hooked_blocks() iterates over all known (registered) blocks to look for blockHooks fields. The problem was that get_hooked_blocks() ended up getting called before all blocks were registered (I think via the Template Part block's PHP code somehow), which meant that any blocks that were registered afterwards wouldn't be taken into account.

Interesting, I wasn't aware of the background here so appreciate you providing that additional context for me. I didn't experience this in my testing but that's not to say it's not an issue at all, just an issue that didn't surface itself to me yet.

For the above reasons, I'm a bit skeptical about caching inside of get_hooked_blocks, as that would be affected by the "called before all blocks are registered"-problem. Maybe we can cache at a different level? 🤔 It's a tricky problem, since the whole point of this PR is to bundle functionality in get_hooked_blocks_by_anchor_block() (which by definition is called for each anchor block), which is at odds with having something called once (at most) per tree traversal 😕

Hmm, interesting. I'll have a further think and potentially revisit this improvement but I can see how you're thinking about this solution.

[github-actions[bot]]

A commit was made that fixes the Trac ticket referenced in the description of this pull request.

SVN changeset: 59715 GitHub commit: https://github.com/WordPress/wordpress-develop/commit/7091fcd2f4e2eb285551bae10ae6a015818fb4f4

This PR will be closed, but please confirm the accuracy of this and reopen if there is more work to be done.