Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Improve performance of WP_Theme_JSON::merge when merging background styles #7486

Closed
wants to merge 14 commits into from
Closed

Improve performance of WP_Theme_JSON::merge when merging background styles #7486

Changes from 5 commits
Commits
Show all changes
14 commits
Select commit Hold shift + click to select a range
c13a398
Improve performance
mukeshpanchal27 Oct 3, 2024
2b7be60
Merge branch 'WordPress:trunk' into fix/61858-performance-regression
mukeshpanchal27 Oct 7, 2024
46640a7
Introduce get_block_node_paths()
mukeshpanchal27 Oct 7, 2024
76e0140
Add options that include node path and block elements
mukeshpanchal27 Oct 8, 2024
82dc69a
Apply suggestions from code review
mukeshpanchal27 Oct 8, 2024
867ae4a
Remove "include_block_elements" parameter
mukeshpanchal27 Oct 9, 2024
dbbd010
Address review feedbacks
mukeshpanchal27 Oct 9, 2024
bdf769c
Merge branch 'WordPress:trunk' into fix/61858-performance-regression
mukeshpanchal27 Oct 10, 2024
2adb26a
Remove variable
mukeshpanchal27 Oct 10, 2024
58dfa83
Update new variable description
mukeshpanchal27 Oct 10, 2024
35e4ed4
Unit tests for new changes
mukeshpanchal27 Oct 10, 2024
f1237f8
Merge branch 'trunk' into fix/61858-performance-regression
felixarntz Oct 10, 2024
115dc4e
Add since annotation for the method change.
felixarntz Oct 10, 2024
0eb736b
Move option variable assignments before the loops.
felixarntz Oct 10, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
107 changes: 64 additions & 43 deletions src/wp-includes/class-wp-theme-json.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2696,7 +2696,9 @@ private static function update_separator_declarations( $declarations ) {
* @param array $options {
* Optional. An array of options for now used for internal purposes only (may change without notice).
*
* @type bool $include_block_style_variations Includes nodes for block style variations. Default false.
* @type bool $include_block_style_variations Includes nodes for block style variations. Default false.
* @type bool $include_node_paths_only Includes node path for block nodes. Default false.
mukeshpanchal27 marked this conversation as resolved.
Show resolved Hide resolved
* @type bool $include_block_elements Includes block elements for block nodes. Default true.
mukeshpanchal27 marked this conversation as resolved.
Show resolved Hide resolved
* }
* @return array The block nodes in theme.json.
*/
Expand All @@ -2713,58 +2715,77 @@ private static function get_block_nodes( $theme_json, $selectors = array(), $opt
}

foreach ( $theme_json['styles']['blocks'] as $name => $node ) {
$selector = null;
if ( isset( $selectors[ $name ]['selector'] ) ) {
$selector = $selectors[ $name ]['selector'];
}

$duotone_selector = null;
if ( isset( $selectors[ $name ]['duotone'] ) ) {
$duotone_selector = $selectors[ $name ]['duotone'];
}
$include_node_paths_only = $options['include_node_paths_only'] ?? false;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This could be moved outside of the foreach loop since that option doesn't change

$node_path = array( 'styles', 'blocks', $name );
if ( $include_node_paths_only ) {
mukeshpanchal27 marked this conversation as resolved.
Show resolved Hide resolved
$nodes[] = $node_path;
mukeshpanchal27 marked this conversation as resolved.
Show resolved Hide resolved
} else {
felixarntz marked this conversation as resolved.
Show resolved Hide resolved
$selector = null;
if ( isset( $selectors[ $name ]['selector'] ) ) {
$selector = $selectors[ $name ]['selector'];
}

$feature_selectors = null;
if ( isset( $selectors[ $name ]['selectors'] ) ) {
$feature_selectors = $selectors[ $name ]['selectors'];
}
$duotone_selector = null;
if ( isset( $selectors[ $name ]['duotone'] ) ) {
$duotone_selector = $selectors[ $name ]['duotone'];
}

$variation_selectors = array();
$include_variations = $options['include_block_style_variations'] ?? false;
if ( $include_variations && isset( $node['variations'] ) ) {
foreach ( $node['variations'] as $variation => $node ) {
$variation_selectors[] = array(
'path' => array( 'styles', 'blocks', $name, 'variations', $variation ),
'selector' => $selectors[ $name ]['styleVariations'][ $variation ],
);
$feature_selectors = null;
if ( isset( $selectors[ $name ]['selectors'] ) ) {
$feature_selectors = $selectors[ $name ]['selectors'];
}

$variation_selectors = array();
$include_variations = $options['include_block_style_variations'] ?? false;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Similarly, this could also be set outside the loop

if ( $include_variations && isset( $node['variations'] ) ) {
foreach ( $node['variations'] as $variation => $node ) {
$variation_selectors[] = array(
'path' => array( 'styles', 'blocks', $name, 'variations', $variation ),
'selector' => $selectors[ $name ]['styleVariations'][ $variation ],
);
}
}

$nodes[] = array(
'name' => $name,
'path' => $node_path,
'selector' => $selector,
'selectors' => $feature_selectors,
'duotone' => $duotone_selector,
'features' => $feature_selectors,
'variations' => $variation_selectors,
'css' => $selector,
);
}

$nodes[] = array(
'name' => $name,
'path' => array( 'styles', 'blocks', $name ),
'selector' => $selector,
'selectors' => $feature_selectors,
'duotone' => $duotone_selector,
'features' => $feature_selectors,
'variations' => $variation_selectors,
'css' => $selector,
);
$include_block_elements = $options['include_block_elements'] ?? true;

if ( isset( $theme_json['styles']['blocks'][ $name ]['elements'] ) ) {
if ( $include_block_elements && isset( $theme_json['styles']['blocks'][ $name ]['elements'] ) ) {
foreach ( $theme_json['styles']['blocks'][ $name ]['elements'] as $element => $node ) {
$nodes[] = array(
'path' => array( 'styles', 'blocks', $name, 'elements', $element ),
'selector' => $selectors[ $name ]['elements'][ $element ],
);
$node_path = array( 'styles', 'blocks', $name, 'elements', $element );
if ( $include_node_paths_only ) {
$nodes[] = $node_path;
mukeshpanchal27 marked this conversation as resolved.
Show resolved Hide resolved
} else {
mukeshpanchal27 marked this conversation as resolved.
Show resolved Hide resolved
$nodes[] = array(
'path' => $node_path,
'selector' => $selectors[ $name ]['elements'][ $element ],
);
}

// Handle any pseudo selectors for the element.
if ( isset( static::VALID_ELEMENT_PSEUDO_SELECTORS[ $element ] ) ) {
foreach ( static::VALID_ELEMENT_PSEUDO_SELECTORS[ $element ] as $pseudo_selector ) {
if ( isset( $theme_json['styles']['blocks'][ $name ]['elements'][ $element ][ $pseudo_selector ] ) ) {
$nodes[] = array(
'path' => array( 'styles', 'blocks', $name, 'elements', $element ),
'selector' => static::append_to_selector( $selectors[ $name ]['elements'][ $element ], $pseudo_selector ),
);
$node_path = array( 'styles', 'blocks', $name, 'elements', $element );
if ( $include_node_paths_only ) {
$nodes[] = $node_path;
} else {
$nodes[] = array(
'path' => $node_path,
'selector' => static::append_to_selector( $selectors[ $name ]['elements'][ $element ], $pseudo_selector ),
);
}
}
}
}
Expand Down Expand Up @@ -3236,14 +3257,14 @@ public function merge( $incoming ) {
* some values provide exceptions, namely style values that are
* objects and represent unique definitions for the style.
*/
$style_nodes = static::get_styles_block_nodes();
$style_options = array( 'include_node_paths_only' => true );
$style_nodes = static::get_block_nodes( $this->theme_json, array(), $style_options );
mukeshpanchal27 marked this conversation as resolved.
Show resolved Hide resolved
foreach ( $style_nodes as $style_node ) {
$path = $style_node['path'];
mukeshpanchal27 marked this conversation as resolved.
Show resolved Hide resolved
/*
* Background image styles should be replaced, not merged,
* as they themselves are specific object definitions for the style.
*/
$background_image_path = array_merge( $path, static::PROPERTIES_METADATA['background-image'] );
$background_image_path = array_merge( $style_node, static::PROPERTIES_METADATA['background-image'] );
$content = _wp_array_get( $incoming_data, $background_image_path, null );
if ( isset( $content ) ) {
_wp_array_set( $this->theme_json, $background_image_path, $content );
Expand Down
Loading