Styles: Move global styles data logic to a dedicated package #72464
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
youknowriad
requested review from
ellatrix and
tellthemachines
as code owners
youknowriad
added
the
[Type] Code Quality
|
Size Change: +9.58 kB (+0.44%) Total Size: 2.18 MB
ℹ️ View Unchanged
|
|
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 If you're merging code through a pull request on GitHub, copy and paste the following into the bottom of the merge commit message. To understand the WordPress project's expectations around crediting contributors, please review the Contributor Attribution page in the Core Handbook. |
youknowriad
requested review from
ajitbohra,
fabiankaegy and
juanmaguitar
as code owners
|
Flaky tests detected in fa87ef6. 🔍 Workflow run URL: https://github.com/WordPress/gutenberg/actions/runs/18689711522
|
ramonjd
approved these changes
Oct 19, 2025
Member
ramonjd
left a comment
ramonjd
left a comment
There was a problem hiding this comment.
Thanks for getting this started, and for the time taken to convert to TS.
I tested block/top-level styles. Style book/revisions also work as expected as far as I can see.
Stuff like addressing TS ignores and adding types for theme.json settings can always be done incrementally.
LGTM, though I think it'd be helpful to get a a confidence test from other folks.
| } from './core/render'; | ||
| export { getBlockSelector } from './core/selectors'; | ||
|
|
||
| // Utilities (Ideally these shouldn't be exposed) |
Member
There was a problem hiding this comment.
What's the alternative?
I think a set of consistent functions to deal with theme.json's many special values is useful.
Just taking getValueFromVariable as an example, a presets control needs that to interpret var: strings and resolve them. Not saying the controls have to work this way.
I'm wondering, if a more controller API is desired, whether we could also move GlobalStylesContext to this package?
Then hooks like useGlobalStyle and others would follow.
Contributor
Author
There was a problem hiding this comment.
The alternative for me is that this is all absorbed by getStyle setStyle, getSetting setSetting and things like that and UI elements shouldn't have to deal with these low level stuff.
Contributor
Author
There was a problem hiding this comment.
I'm wondering, if a more controller API is desired, whether we could also move GlobalStylesContext to this package?
My idea is to remove the context entirely but I don't know how feasible is that yet. I think the context just adds confusion.
Contributor
There was a problem hiding this comment.
At least some of these (getValueFromVariable, getPresetVariableFromValue for instance) could live in the style engine package, alongside things like getCSSValueFromRawStyle. They have similar generic converting-thing-to-other-thing functionality and can be useful for any styles, not just global ones.
Contributor
Author
There was a problem hiding this comment.
the style engine package, alongside things like getCSSValueFromRawStyle. They have similar generic converting-thing-to-other-thing functionality and can be useful for any styles, not just global ones.
I think the style engine package should just merge with this new higher level package, there's no reason for it existing separately, it's the same purpose. Now, I don't want to do that merge work here, it can be done later, it's low impact.
Contributor
Author
There was a problem hiding this comment.
style-engine is used in core though, if we removed it we'd still have to support it for older WP versions.
global-style-engine is not and is a bundled package, so it can be removed at any moment.
Contributor
Author
There was a problem hiding this comment.
@andrewserong I don't think we should move forward with this yet. The global-style-engine API is private and by moving it to style-engine it becomes public. It is very early to expose it as a public api (global variable) and I'm not sure if we should.
Contributor
There was a problem hiding this comment.
Fair point @youknowriad! I've closed out #72559 — I found it a useful exercise at least to get a feel for what it might look like to combine them. It wasn't very much work to create that PR, so if and when we are ready to make things public, I don't think it'll be too much work to try it again 🙂
Contributor
There was a problem hiding this comment.
There's also the option of exporting anything we don't want public as a private API from the style engine package, that should ensure it doesn't end up in the global var, right?
Contributor
Author
There was a problem hiding this comment.
Yes, that's possible too but I'm preferring more and more regular packages for a few reasons:
- privateApis usage have proven challenging in DataViews. Basically every time a "bundled" plugin uses a private API, that bundled plugin can't be used twice in two different plugins/scripts anymore. So this is forcing us to remove private APIs usage from DataViews. (not done yet but we'll have to do it soon).
- bundled packages are nice because they need to be stateless (no side effects) in general so they force folks to write the right APIs
- There's a DevX aspect to it: no ugly unlock and privateApis imports all around our code base.
- Bundled packages are tree shakable. You can bundle some of it in edit-post and another part in edit-site without necessarily needing to load the full package in both.
| getComputedFluidTypographyValue, | ||
| } from './fluid'; | ||
|
|
||
| function isFluidTypographyEnabled( typographySettings?: { fluid?: any } ) { |
Member
There was a problem hiding this comment.
I think we'd already documented the typography type in JSDOC. Can be used here, or I'm happy to take care of it in a follow up
Contributor
tellthemachines
left a comment
tellthemachines
left a comment
There was a problem hiding this comment.
I did a quick round of testing on this by changing some global styles and checking them on site and post editors, and all seems to be in order!
I see the style book is erroring when opening a block style variation from the global styles sidebar, but that's also happening on trunk so not related to these changes 😅
| return getValueFromVariable( features, blockName, result as string ); | ||
| } | ||
|
|
||
| export function getValueFromVariable( |
Contributor
There was a problem hiding this comment.
It would be good to copy across the JSDoc comments for these functions where they exist, makes it easier to tell at a glance what the function does.
Contributor
Author
There was a problem hiding this comment.
Good call on the comments, I added the ones that I found back.
| } from './core/render'; | ||
| export { getBlockSelector } from './core/selectors'; | ||
|
|
||
| // Utilities (Ideally these shouldn't be exposed) |
Contributor
There was a problem hiding this comment.
At least some of these (getValueFromVariable, getPresetVariableFromValue for instance) could live in the style engine package, alongside things like getCSSValueFromRawStyle. They have similar generic converting-thing-to-other-thing functionality and can be useful for any styles, not just global ones.
| * | ||
| * @return {Array} Array of stylesheets and settings. | ||
| */ | ||
| export function useGlobalStylesOutputWithConfig( |
Contributor
There was a problem hiding this comment.
Won't this ever be needed outside of edit-site? I'm thinking that strictly speaking things like the style book shouldn't really be a part of the site editor, but sit at the admin level instead.
Contributor
Author
There was a problem hiding this comment.
Won't this ever be needed outside of edit-site? I'm thinking that strictly speaking things like the style book shouldn't really be a part of the site editor, but sit at the admin level instead.
It will in the post editor but I don't like how it works today (update settings that are not supposed to be edited), so let's leave it as is for now and see what makes sense when adding the global styles sidebar to the post editor.
youknowriad
force-pushed
the
add/global-styles-engine-package
branch
from
4627fbb to
fa87ef6
Compare
This was referenced Nov 25, 2025
Merged
ramonjd
reviewed
Nov 25, 2025
| hasFallbackGapSupport, | ||
| disableLayoutStyles, | ||
| disableRootPadding, | ||
| getBlockStyles, |
Member
There was a problem hiding this comment.
Note to self: missed the object that tells the function to also generate block variation styles.
Fix here: #73535
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
First step to achieve #72317
Summary
The global styles code has long been neglected and lived within unrelated packages (block-editor, edit-site), this PR starts the work to organize it better. It moves all the logic to generate CSS out of a global styles object to a dedicated package, all the global styles object manipulation as well. It's the "global-styles-engine".
The code is also full typescript now and the PR also maintains same functionality without any change.
The following step is going to be a UI package that relies on this engine (extracted from edit-site) which would allow us to use the global styles UI in a more portable way (include in the post editor)
Testing instructions
Check global styles behavior / rendering in the site editor (it should work just like trunk)