Add registered_block_type action, register clearing Theme JSON cache to fix global styles issue

[andrewserong]

This is (yet another) alternative to #3352, #3359, #3373 and #3384, as an attempt to resolve the issue described in: WordPress/gutenberg#44434 — where block-level global styles are not output and cannot be saved in 6.1.

Changes proposed:

• Add registered_block_type and unregistered_block_type actions that fires after a block is registered or unregistered in the WP_Block_Type_Registry class.
• Register a callback against the registered_block_type action that then flushes the Theme JSON and Theme JSON Resolver caches.

This should fix the issue where the list of registered blocks becomes cached, preventing block-level global styles from working. The approach here ensures that any time a block is registered, the cache will be cleared after the block is registered, so the Theme JSON and Resolver caches should not contain any stale block data.

In #3359 and #3373 it was proposed to invalidate the cache based on the number of registered blocks. However, as @ajlende and @ramonjd flagged, using the number of blocks as the determining factor for the cache key may not be robust enough.

The idea in this PR is that if we add an action once a block is registered, then we don't need to perform a look-up in the Theme JSON and Resolver classes, as a side effect of registering a block will be to clear out the existing cache.

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

---

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.

[ajlende]

I think we could move the hook into WP_Block_Type_Registry's register and unregister rather than in blocks.php, but otherwise this seems to me like a good fix to me to mitigate the problem for 6.1. 👍

[andrewserong]

I think we could move the hook into WP_Block_Type_Registry's register and unregister rather than in blocks.php

Good call — that ensures that we don't miss any registrations that happen using the class directly, and means we can remove the duplication in this PR 👍. I'll update!

[andrewserong]

I've updated this PR to move the actions to the class, and that seems to fix up the issue of making sure that we capture all registration events. I've left the callback targeting just the registered event, though — since the action occurs after registration completes, if a plugin unregisters blocks for any reason, the cache will still be clear. I think it's probably pretty unlikely that a plugin would call the Theme JSON classes doing an unregister task, so possibly not worth the extra processing to target unregister, too. I've included the action, though, for consistency and so that it's easy to add in if we need to in the future.

[talldan]

I like this solution. It feels fairly scalable in that I can imagine in the future, if there are improvements to the caching system, this could only cause a partial invalidation rather than a full invalidation.

[talldan]

It might be worth adding more documentation if this is a public function. I'm thinking along the lines of a brief detailing of the side effects calling this function will have.

[andrewserong]

Thanks, I've added an extra line here to indicate what the function does. I found it hard to capture in a single sentence without falling into the trap of describing the particular use case with registered_block_type. Happy to expand on that if it's suitable, though.

[talldan]

From what I can see this won't be called on failure as the function returns early. It seems fine to only call the action on success though.

[talldan]

Something else that came to mind is that 'Fires after a block type is registered.' might not be specific enough. It might be worth mentioning that it's specifically when a block is registered with the WP_Block_Type_Registry class.

The reason I'm thinking this is that there's the JavaScript registerBlockType function, and so documentation readers might get confused about the timing of this action.

Same would apply for the unregister action too.

[andrewserong]

I've updated both of the hooks to remove |false and the reference to failure, and updated the description to flag with the WP_Block_Type_Registry class.

[talldan]

I'm not completely sure of what the best naming convention is, but I see some actions use past tense and others use something like 'after_register_block_type' or 'post_register_block_type'.

It would be interesting to know if there's a preference!

[andrewserong]

It would be interesting to know if there's a preference!

Same, I'd be curious to hear what folks think! In this case, I borrowed the naming convention from registered_post_type and registered_taxonomy.

[talldan]

Same comment here about this being incorrect, it looks like it only triggers the action on successful unregistering.

[andrewserong]

Same with the other hook, I've updated this to remove reference to |false

[andrewserong]

Thanks for reviewing @talldan! I've made a few small updates to the comments 👍

[andrewserong]

I'll close out this PR now as folks decided to go in a different direction to avoid adding new actions, as flagged in WordPress/gutenberg#44434 (comment).