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.

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

Backport from WordPress Core: improve performance of WP_Theme_JSON::merge when merging background styles #66002

Merged
merged 3 commits into from
Oct 10, 2024

Conversation

ramonjd
Copy link
Member

@ramonjd ramonjd commented Oct 10, 2024

What?

Follow up to #64128

Backporting from WordPress/wordpress-develop#7486

See: https://core.trac.wordpress.org/ticket/61858

Props to @mukeshpanchal27 @felixarntz @joemcgill @andrewserong

Why?

The original implementation uses WP_Theme_JSON::get_block_nodes to fetch the array of style node paths.

WP_Theme_JSON::get_block_nodes calls WP_Theme_JSON::get_blocks_metadata`, which caused a significant performance regression. See: WordPress/performance#1572 (comment)

Testing Instructions

See testing instructions in #64128

Also, run the tests!

npm run test:unit:php:base -- --filter WP_Theme_JSON_Gutenberg_Test

@ramonjd ramonjd added [Type] Regression Related to a regression in the latest release Backport from WordPress Core Pull request that needs to be backported to a Gutenberg release from WordPress Core labels Oct 10, 2024
@ramonjd ramonjd self-assigned this Oct 10, 2024
Copy link

github-actions bot commented Oct 10, 2024

Warning: Type of PR label mismatch

To merge this PR, it requires exactly 1 label indicating the type of PR. Other labels are optional and not being checked here.

  • Type-related labels to choose from: [Type] Automated Testing, [Type] Breaking Change, [Type] Bug, [Type] Build Tooling, [Type] Code Quality, [Type] Copy, [Type] Developer Documentation, [Type] Enhancement, [Type] Experimental, [Type] Feature, [Type] New API, [Type] Task, [Type] Technical Prototype, [Type] Performance, [Type] Project Management, [Type] Regression, [Type] Security, [Type] WP Core Ticket, Backport from WordPress Core, Gutenberg Plugin.
  • Labels found: [Type] Regression, Backport from WordPress Core.

Read more about Type labels in Gutenberg. Don't worry if you don't have the required permissions to add labels; the PR reviewer should be able to help with the task.

1 similar comment
Copy link

Warning: Type of PR label mismatch

To merge this PR, it requires exactly 1 label indicating the type of PR. Other labels are optional and not being checked here.

  • Type-related labels to choose from: [Type] Automated Testing, [Type] Breaking Change, [Type] Bug, [Type] Build Tooling, [Type] Code Quality, [Type] Copy, [Type] Developer Documentation, [Type] Enhancement, [Type] Experimental, [Type] Feature, [Type] New API, [Type] Task, [Type] Technical Prototype, [Type] Performance, [Type] Project Management, [Type] Regression, [Type] Security, [Type] WP Core Ticket, Backport from WordPress Core, Gutenberg Plugin.
  • Labels found: [Type] Regression, Backport from WordPress Core.

Read more about Type labels in Gutenberg. Don't worry if you don't have the required permissions to add labels; the PR reviewer should be able to help with the task.

Copy link

github-actions bot commented Oct 10, 2024

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.

If you're merging code through a pull request on GitHub, copy and paste the following into the bottom of the merge commit message.

Co-authored-by: ramonjd <[email protected]>
Co-authored-by: mukeshpanchal27 <[email protected]>
Co-authored-by: felixarntz <[email protected]>

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

@ramonjd ramonjd added [Status] In Progress Tracking issues with work in progress [Type] Regression Related to a regression in the latest release and removed [Type] Regression Related to a regression in the latest release labels Oct 10, 2024
Copy link
Member

@felixarntz felixarntz left a comment

Choose a reason for hiding this comment

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

Thanks @ramonjd!

@ramonjd ramonjd removed the [Status] In Progress Tracking issues with work in progress label Oct 10, 2024
@ramonjd ramonjd enabled auto-merge (squash) October 10, 2024 22:39
@ramonjd ramonjd force-pushed the backport/merge-theme-json-performance-6.7 branch from 251e889 to 7c59449 Compare October 10, 2024 23:02
@ramonjd ramonjd merged commit 2cd6929 into trunk Oct 10, 2024
63 checks passed
@ramonjd ramonjd deleted the backport/merge-theme-json-performance-6.7 branch October 10, 2024 23:39
@github-actions github-actions bot added this to the Gutenberg 19.5 milestone Oct 10, 2024
ciampo pushed a commit that referenced this pull request Oct 14, 2024
…erge when merging background styles (#66002)

Backporting the current state of WordPress/wordpress-develop#7486 and adding tests.

Co-authored-by: ramonjd <[email protected]>
Co-authored-by: mukeshpanchal27 <[email protected]>
Co-authored-by: felixarntz <[email protected]>
Co-authored-by: andrewserong < [email protected]>
Co-authored-by: joemcgill < [email protected]>
ramonjd added a commit that referenced this pull request Nov 13, 2024
Including variations in the nodes array when 'include_node_paths_only' => true
karthick-murugan pushed a commit to karthick-murugan/gutenberg that referenced this pull request Nov 13, 2024
…erge when merging background styles (WordPress#66002)

Backporting the current state of WordPress/wordpress-develop#7486 and adding tests.

Co-authored-by: ramonjd <[email protected]>
Co-authored-by: mukeshpanchal27 <[email protected]>
Co-authored-by: felixarntz <[email protected]>
Co-authored-by: andrewserong < [email protected]>
Co-authored-by: joemcgill < [email protected]>
ramonjd added a commit that referenced this pull request Nov 13, 2024
…_block_nodes (#66948)

Follow up to #66002
Including variations in the nodes array when 'include_node_paths_only' => true

Co-authored-by: ramonjd <[email protected]>
Co-authored-by: aaronrobertshaw <[email protected]>
peterwilsoncc pushed a commit to peterwilsoncc/gutenberg-build that referenced this pull request Nov 22, 2024
…_block_nodes (#66948)

Follow up to WordPress/gutenberg#66002
Including variations in the nodes array when 'include_node_paths_only' => true

Co-authored-by: ramonjd <[email protected]>
Co-authored-by: aaronrobertshaw <[email protected]>

Source: WordPress/gutenberg@d0a190b
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Backport from WordPress Core Pull request that needs to be backported to a Gutenberg release from WordPress Core [Type] Regression Related to a regression in the latest release
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants