Conversation
This skips the caching that happens in get_blocks_metadata when core/template-part needs to construct a WP_Theme_JSON to get the template parts which don't rely on the blocks metadata.
andrewserong
left a comment
There was a problem hiding this comment.
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! ✨
|
Thank you @ajlende! I thought that for 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 Otherwise, we add metadata -- for that subset of registered blocks only that doesn't have them yet. (It was |
|
(FWIW, I'm in favor of merging this PR -- I agree with the "minor performance or code quality improvement" rationale 👍 ) |
|
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. |
What?
Gets the block names directly from the
WP_Block_Type_Registryin theWP_Theme_JSONconstructor.This avoids caching
static::$blocks_metadatawhen a newWP_Theme_JSONis 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 inWP_Theme_JSON_Resolverthat 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 theWP_Theme_JSONconstructor.The only reason
get_blocks_metadatawas being called was to get the block names which doesn't require all of the additional processing thatget_blocks_metadatadoes.How?
Diff below for the actual change with core.
Testing Instructions
Ensure that themes load the same as they did before.