-
Notifications
You must be signed in to change notification settings - Fork 2.4k
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
Global styles: cache post-processed style variations #6857
base: trunk
Are you sure you want to change the base?
Conversation
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 Core Committers: Use this line as a base for the props when committing in SVN:
To understand the WordPress project's expectations around crediting contributors, please review the Contributor Attribution page in the Core Handbook. |
Test using WordPress PlaygroundThe 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
For more details about these limitations and more, check out the Limitations page in the WordPress Playground documentation. |
@joemcgill what are your thoughts on doing this? I was looking at the performance results for #6843 (cache reading the contents from the filesystem), and I'm a bit confused. I'm not familiar with what we are testing for the "Admin" test, so I don't know if it should have been impacted. However, for the "Front End › Theme: twentytwentyfour" I would have expected to be more impacted. Instead, I see timid or conflicting results:
![]() The results for this PR are similarly confusing:
![]() This is my current understanding of the new feature:
Unless the tests are not triggering the file reads from the filesystem for some reason, I'd have expected the numbers to have improved with the cache (previous PR & this one). |
In theory, adding a cache here should be more performant. However, it seem that the refactor of It seems odd to me that in trunk, none of the This seems like a likely bug. |
As far as I tested, the feature is working correctly in I haven't had the time to look at this using xdebug today, as I was investigating a proof of concept to optimize the data flow. |
Thanks, I'll do some more testing. I'm just surprised that currently no variations are being processed while running TT4 in trunk, but perhaps it's because those code paths aren't being triggered without adding some variations manually? |
Trac ticket https://core.trac.wordpress.org/ticket/61451
Follow-up to #6843
Backports WordPress/gutenberg#62610
What
This PR caches the post-processed style variation files (theme.json within the
styles/
folder).Why
To improve performance, so we don't have to process them more than once.
How
Introduces a static
$style_variations_cache
variable.Test
How to test
Go to "Site Editor > Styles" and apply one of them. Verify the changes are reflected in the front end. Do the same from the global styles sidebar in the site editor.
Create a
partial.json
file within thestyles/
folder with the following contents:Go to any editor, add a group block, and verify there is a "Partial" style variation (Block Settings > Styles). Apply the variation and save the changes. Verify the contents are the expected (background color is aliceblue) — also in the front-end.
Commit
Proposal for commit message, to make it easier for committers: