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

[Proof of Concept] Optimize how shared block style variations are processed #6868

Conversation

oandregal
Copy link
Member

@oandregal oandregal commented Jun 20, 2024

Trac ticket https://core.trac.wordpress.org/ticket/61451
Follow-up work for https://core.trac.wordpress.org/ticket/61312

What

This PR tries to optimize the data flow for operating with section styles.

Why

It has proven less optimized than we wanted.

How

At some point, the code transforms the following:

{
  "styles": {
    "blocks": {
      "variations": {
        "myVariation": {
          "title": "My variation",
          "blockTypes": [ "core/paragraph", "core/group" ],
          "color": {
            "background": "yellow"
          }
        }
      }
    }
  }
}

into:

{
  "styles": {
    "blocks": {
      "core/paragraph": {
        "color": {
          "background": "yellow"
        }
      },
      "core/group": {
        "color": {
          "background": "yellow"
        }
      }
    }
  }
}

The existing code executes that transformation using the wp_theme_json_data_* filters – having to recreate the WP_Theme_JSON structures a few times.

The optimization proposed in this PR explores moving that transform into the WP_Theme_JSON constructor directly, following what we do with appearanceTools settings.

Test

TBD.

@oandregal oandregal self-assigned this Jun 20, 2024
@oandregal oandregal changed the title Update block style variations processing Optimize how shared block style variations are processed Jun 20, 2024
@oandregal
Copy link
Member Author

oandregal commented Jun 20, 2024

This is just a proof of concept, but I wanted to share the approach as soon as possible to gather feedback and gauge the impact on performance (not much so far, as far as I see).

What works:

  • Variations defined in theme.json are working without having to be processed in the wp_theme_json_data_theme filter.

What doesn't:

  • For some reason, the block style variations defined in a theme style variation (e.g.: in Ember.json) and that become user data (see function wp_resolve_block_style_variations_from_theme_style_variation) are not working now.

Viability of the PoC:

  • My current thinking is that this approach should help absorb what trunk does in wp_resolve_block_style_variations_from_primary_theme_json, wp_resolve_block_style_variations_from_theme_json_partials, and wp_resolve_block_style_variations_from_theme_style_variation.
  • We could also probably absorb wp_resolve_block_style_variations_from_styles_registry, but differently. Potential idea: in the WP_Theme_JSON_Resolver::get_theme_data, we take the data from the block registry and merge it there.

Copy link

Test using WordPress Playground

The changes in this pull request can previewed and tested using a WordPress Playground instance.

WordPress Playground is an experimental project that creates a full WordPress instance entirely within the browser.

Some things to be aware of

  • The Plugin and Theme Directories cannot be accessed within Playground.
  • All changes will be lost when closing a tab with a Playground instance.
  • All changes will be lost when refreshing the page.
  • A fresh instance is created each time the link below is clicked.
  • Every time this pull request is updated, a new ZIP file containing all changes is created. If changes are not reflected in the Playground instance,
    it's possible that the most recent build failed, or has not completed. Check the list of workflow runs to be sure.

For more details about these limitations and more, check out the Limitations page in the WordPress Playground documentation.

Test this pull request with WordPress Playground.

@oandregal oandregal changed the title Optimize how shared block style variations are processed [Proof of Concept] Optimize how shared block style variations are processed Jun 20, 2024
@joemcgill
Copy link
Member

I like the idea of trying to move this processing directly into the parsing of a WP_Theme_JSON object, rather than needing to normalize the JSON before passing it to the constructor of a WP_Theme_JSON object. If we are expecting styles.blocks.variations.{variationKey} to be a valid part of the schema, passing that shape to the WP_Theme_JSON constructor should result in a correctly parsed object, IMO.

I ran some initial profiles on this PR compared with the parent commit (c1c8d30) and can confirm that it currently has very little impact, but that is mostly because the processing being skipped, wp_resolve_block_style_variations_from_primary_theme_json() is currently not doing much in trunk:

image

Looking further into the profiling data, it looks like when wp_merge_block_style_variations_data is being called, it's being passed an empty array, so no new WP_Theme_JSON_Data is created, nor is anything merged. I'm guessing this is due to the bug I observed in #6857 (comment).

image

@oandregal
Copy link
Member Author

#6873 takes the ideas from this PR and does a few other things.

@oandregal
Copy link
Member Author

#6873 implements this Proof of concept so this can be closed.

@oandregal oandregal closed this Jun 24, 2024
@oandregal oandregal deleted the update/block-style-variations-processing branch June 24, 2024 08:52
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants