Avoid calling get_blocks_metadata in WP_Theme_JSON constructor

[ajlende]

What?

Gets the block names directly from the WP_Block_Type_Registry in the WP_Theme_JSON constructor.

This avoids caching static::$blocks_metadata when a new WP_Theme_JSON is constructed which was causing some issues with #44434 and #44619. This change by itself doesn't fix those issues because there's another layer of caching in WP_Theme_JSON_Resolver that can be seen in in WordPress/wordpress-develop#3359.

Why?

At the very least this is a minor performance improvement or code quality improvement where you need to construct a WP_Theme_JSON, but don't need to use $blocks_metadata. And it would fix bugs related to the caching side-effect in the WP_Theme_JSON constructor.

The only reason get_blocks_metadata was being called was to get the block names which doesn't require all of the additional processing that get_blocks_metadata does.

How?

Diff below for the actual change with core.

diff --git a/src/wp-includes/class-wp-theme-json.php b/src/wp-includes/class-wp-theme-json.php
index ae214b361a..6c1d9e1062 100644
--- a/src/wp-includes/class-wp-theme-json.php
+++ b/src/wp-includes/class-wp-theme-json.php
@@ -501,7 +501,8 @@ class WP_Theme_JSON {
 		}
 
 		$this->theme_json    = WP_Theme_JSON_Schema::migrate( $theme_json );
-		$valid_block_names   = array_keys( static::get_blocks_metadata() );
+		$registry            = WP_Block_Type_Registry::get_instance();
+		$valid_block_names   = array_keys( $registry->get_all_registered() );
 		$valid_element_names = array_keys( static::ELEMENTS );
 		$theme_json          = static::sanitize( $this->theme_json, $valid_block_names, $valid_element_names );
 		$this->theme_json    = static::maybe_opt_in_into_settings( $theme_json );

Testing Instructions

Ensure that themes load the same as they did before.

[andrewserong]

Thanks @ajlende, this looks like a good change to me!

✅ Confirmed that the diff here between core and this PR is just the swapping out of static::get_blocks_metadata() for $registry->get_all_registered()
✅ Double-checked that get_blocks_metadata() does not add or remove blocks from the list of get_all_registered() so these lists should be the same 👍
✅ Smoke tested changes in theme.json and global styles, and it looks like this change doesn't adversely affect anything as far as I can tell!

This LGTM. We'll also need one of the core PRs to land to fix the other caching issues (I think WordPress/wordpress-develop#3392 might be viable now?), but I think this is a good step toward improving the caching problem in the Theme JSON class.

Thanks again! ✨

[ockham]

Thank you @ajlende!

I thought that for WP_Theme_JSON, we could actually fix the caching issue with a small change to get_blocks_metadata that I proposed here:

diff --git a/src/wp-includes/class-wp-theme-json.php b/src/wp-includes/class-wp-theme-json.php
index 77b77388af2..594ad4c61c7 100644
--- a/src/wp-includes/class-wp-theme-json.php
+++ b/src/wp-includes/class-wp-theme-json.php
@@ -729,14 +729,19 @@ protected static function append_to_selector( $selector, $to_append, $position =
 	 * @return array Block metadata.
 	 */
 	protected static function get_blocks_metadata() {
-		if ( null !== static::$blocks_metadata ) {
-			return static::$blocks_metadata;
-		}
-
-		static::$blocks_metadata = array();
-
 		$registry = WP_Block_Type_Registry::get_instance();
 		$blocks   = $registry->get_all_registered();
+
+		if ( null === static::$blocks_metadata ) {
+			static::$blocks_metadata = array();
+		} else {
+			// Do we have metadata for all currently registered blocks?
+			$blocks = array_diff_key( $blocks, static::$blocks_metadata );
+			if ( count( $blocks ) === 0 ) {
+				return static::$blocks_metadata;
+			}
+		}
+
 		foreach ( $blocks as $block_name => $block_type ) {
 			if (
 				isset( $block_type->supports['__experimentalSelector'] ) &&

This mostly leverages PHP's array_diff_key: It looks at the list of currently registered $blocks and compares its keys (which are block names, e.g. core/heading) to those of the list of blocks metadata -- which are also keyed by block name. It then returns the subset of registered $blocks whose keys aren't in static::$blocks_metadata. If that subset is empty, we return the cached blocks metadata, as we can assume that we have metadata for all registered blocks already.

Otherwise, we add metadata -- for that subset of registered blocks only that doesn't have them yet.

---

(It was WP_Theme_JSON_Resolver that I've found harder to fix, see the related discussion over at WordPress/wordpress-develop#3359.)

[ockham]

(FWIW, I'm in favor of merging this PR -- I agree with the "minor performance or code quality improvement" rationale 👍 )

[oandregal]

Hey, while working on #46579 I realized this PR was not backported to WordPress 6.1. When we land that PR and then backport to WordPress 6.2 everything will be fine, but I wanted to raise in case you think this should be ported for a potential WordPress 6.1.X release.