🚧 WIP: Use the style engine in global-styles 🚧 🧪

[aristath]

What?

Why?

How?

Testing Instructions

Testing Instructions for Keyboard

Screenshots or screencast

[github-actions]

Flaky tests detected in 11536da.
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/4445506352
📝 Reported issues:

• [Flaky Test] doesn't throw a block error when clicking on a link #47747 in /test/e2e/specs/site-editor/template-part.spec.js

[aristath]

@ramonjd @andrewserong Continuing the work we started last year with the Style Engine, we should try to implement it in global-styles, block-styles etc.
This PR is a proof of concept so we can continue the discussion and see how we can make things work and complete the migration to the Style Engine 👍

[andrewserong]

Thanks for picking up this work again @aristath! I'll start having a look at this over the next week.

[andrewserong]

Very cool @aristath! A few first impressions:

• I really like seeing rules constructed via the style engine rather than concatenating strings
• In that respect this demonstrates a good relationship between one part of the codebase and where to iteratively start leveraging style engine functions 👍
• Being able to define a store when instantiating the Theme JSON class looks really powerful, and I can see that being useful
• There's a fair bit of additional code added, and for example in add_styles_for_block_to_rules_store we still have quite a long function (I understand this is mostly re-arranging the existing long function). Is there an opportunity to move any of the logic from the Theme JSON class either to existing style engine classes, or add an additional style engine class or function where we can? For example, the WP_Style_Engine class can currently deal with a style object, so could we potentially make compute_style_properties calls obsolete by having the style engine handle styles for each block?
• If we can find opportunities to split off parts of the Theme JSON class from including the logic for generating styles, then hopefully we might be able to make the Theme JSON class easier to read / having clearer points of delineation between the Theme JSON logic and the style engine itself
• For some of the folks following along with the style engine project, one of the hopes was that we could either simplify how global styles are generated, or at least carve the logic out a bit, to help reduce the complexity of the Theme JSON class. So hopefully we'll be able to consolidate a bit further, there.

Great work so far! Thanks for diving in the deep end with all this exploration 🙂

[ramonjd]

Sorry for the loooong delay in getting back to you on this. I'm still adjusting to being back behind a keyboard.

Thanks a lot for diving in here. I'm loving the direction.

Adding the global styles to a store will open up a lot of possibilities, e.g., it is very pleasing to see prettified and consolidated output in the global-styles-inline-css style tag compared with trunk. 😄

Also I'm excited to see that Chrome will support CSS nesting, having global styles available in the store will (I hope) allow us to better control specificity and more easily avoid CSS override issues. That's a while off yet, but it's something I'm really keen on. Can't you tell? 😆

Is there an opportunity to move any of the logic from the Theme JSON class either to existing style engine classes, or add an additional style engine class or function where we can?

I agree in principle here — the Theme JSON class is massive and adding replacement classes would increase the already high cognitive load — but am wondering how it would work in practice.

Marking the replaced methods as deprecated, which is what this PR does, would help even though it is a longer term strategy.

If we abstract, I think the style engine should be ignorant of its implementation in WP_Theme_JSON as much as possible, so maybe there's scope for a new Gutenberg class (not part of style engine) to which we can migrate specific global styles functionality and integrate the style engine, e.g., WP_Global_Styles or whatever.

That class could act as an adapter/facade and eventually take over all global styles responsibility, e.g., subsume the logic in gutenberg_enqueue_global_styles.

I'm just rambling here by the way.

For example, the WP_Style_Engine class can currently deal with a style object, so could we potentially make compute_style_properties calls obsolete by having the style engine handle styles for each block?

I noted this on the relevant issue. #46563

There's also the potential to remove the need for to_ruleset as well - it looks like this PR replaces the get_styles_for_block, get_css_variables, get_layout_styles, compute_preset_classes and compute_preset_classes methods in WP_Theme_Json, which use it so that point might be moot.

[youknowriad]

It's very hard to read the code to try to understand what's happening here. I would love if we could write some documentation/schemas to explain the goal and the architecture here.