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

Compute presets from theme.json: skip those without classes or variables #53574

Merged
merged 3 commits into from
Aug 14, 2023

Conversation

oandregal
Copy link
Member

Part of #45171
Extracted from #53351

What?

When processing presets to generate its classes or CSS Custom Properties, skip those who don't declare any classes or variables in their metadata.

Why?

To reduce unnecessary computations.

How?

Checking whether the preset has a class before processing it within compute_preset_classes and checking whether the preset has a variable before processing it within compute_preset_vars.

Testing Instructions

Verify tests pass.

@oandregal oandregal self-assigned this Aug 11, 2023
@oandregal oandregal added the [Type] Performance Related to performance efforts label Aug 11, 2023
@github-actions
Copy link

This pull request has changed or added PHP files. Please confirm whether these changes need to be synced to WordPress Core, and therefore featured in the next release of WordPress.

If so, it is recommended to create a new Trac ticket and submit a pull request to the WordPress Core Github repository soon after this pull request is merged.

If you're unsure, you can always ask for help in the #core-editor channel in WordPress Slack.

Thank you! ❤️

View changed files
❔ lib/class-wp-theme-json-gutenberg.php

@oandregal
Copy link
Member Author

The performance tests reports this PR to be 1.5ms faster for TTFB. It's very minimal impact, given the uncertainty and standard deviation of the tests. It is still useful not computing the items that don't have classes/variables – it's a linear evolution: the more of them there are, the more performance is impacted if we compute them all unnecessarily.

Captura de ecrã 2023-08-14, às 12 08 51

@@ -1768,6 +1772,10 @@ protected static function replace_slug_in_string( $input, $slug ) {
protected static function compute_preset_vars( $settings, $origins ) {
$declarations = array();
foreach ( static::PRESETS_METADATA as $preset_metadata ) {
if ( ! isset( $preset_metadata['css_vars'] ) ) {
Copy link
Member

Choose a reason for hiding this comment

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

Just for consistency could we use empty and in the other condition?

Copy link
Member

@jorgefilipecosta jorgefilipecosta left a comment

Choose a reason for hiding this comment

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

The change looks good to me 👍

@github-actions
Copy link

Flaky tests detected in 034f49c.
Some tests passed with failed attempts. The failures may not be related to this commit but are still reported for visibility. See the documentation for more information.

🔍 Workflow run URL: https://github.com/WordPress/gutenberg/actions/runs/5854698995
📝 Reported issues:

@oandregal oandregal merged commit e1e0545 into trunk Aug 14, 2023
50 checks passed
@oandregal oandregal deleted the update/check-for-metadata-before-looping branch August 14, 2023 11:07
@github-actions github-actions bot added this to the Gutenberg 16.5 milestone Aug 14, 2023
@mikachan mikachan added the Needs PHP backport Needs PHP backport to Core label Sep 5, 2023
@SiobhyB SiobhyB removed the Needs PHP backport Needs PHP backport to Core label Sep 25, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
[Type] Performance Related to performance efforts
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants