Style Engine: Try approach for consolidating styles into a single style tag and de-dupe styles

[andrewserong]

Description

🚧 🚧 🚧 🚧 Note: this is an exploratory PR just to try out some ideas for server-rendering styles 🚧 🚧 🚧 🚧

At the moment, the idea of this PR is to take a look at a couple of areas that we hope to improve via server-rendering using the style engine:

1. Consolidating style tags for layout support
2. Explore whether we can de-dupe class names by generating class names based on values used in constructing the styles instead of using a random class name. E.g. wp-container-1 becomes wp-layout-default wp-layout-default-content-size-650px wp-layout-default-wide-size-1000px wp-layout-default-global-gap

Related work:

There is another similar / alternate exploration in #39086

Related:

• Tracking: Add a Style Engine to manage rendering block styles #38167

Testing Instructions

Screenshots

Currently this PR looks at consolidating style generation for the layout support into a single style tag, and generating class names based on style values instead of a randomly assigned number.

Before	After

Types of changes

Exploratory

Checklist:

• My code is tested.
• My code follows the WordPress code style.
• My code follows the accessibility standards.
• I've tested my changes with keyboard and screen readers.
• My code has proper inline documentation.
• I've included developer documentation if appropriate.
• I've updated all React Native files affected by any refactorings/renamings in this PR (please manually search all *.native.js files for terms that need renaming or removal).
• I've updated related schemas if appropriate.

[ramonjd]

Thanks for getting this started!

Just looking at deduping/storying common values, such as the following, and wondering if we can do it as we add them or whether it can be done on output.

There are quite a few of them generated by layout:

.wp-container-14 > :where(:not(.alignleft):not(.alignright)) {max-width: 650px;margin-left: auto !important;margin-right: auto !important;}.wp-container-14 > .alignwide { max-width: 1000px;}.wp-container-14 .alignfull { max-width: none; }.wp-container-14 .alignleft { float: left; margin-right: 2em; margin-left: 0; }.wp-container-14 .alignright { float: right; margin-left: 2em; margin-right: 0; }.wp-container-14 > * { margin-top: 0; margin-bottom: 0; }.wp-container-14 > * + * { margin-top: var( --wp--style--block-gap );  margin-bottom: 0; }



.wp-container-15 > :where(:not(.alignleft):not(.alignright)) {max-width: 650px;margin-left: auto !important;margin-right: auto !important;}.wp-container-15 > .alignwide { max-width: 1000px;}.wp-container-15 .alignfull { max-width: none; }.wp-container-15 .alignleft { float: left; margin-right: 2em; margin-left: 0; }.wp-container-15 .alignright { float: right; margin-left: 2em; margin-right: 0; }.wp-container-15 > * { margin-top: 0; margin-bottom: 0; }.wp-container-15 > * + * { margin-top: var( --wp--style--block-gap );  margin-bottom: 0; }

I might have a play around if that's okay.

[andrewserong]

Thanks for taking a look @ramonjd! It's a very small step in digging in and seeing what we might be able to do 😀

and wondering if we can do it as we add them or whether it can be done on output.

Yes, that'd be a great next step, I haven't quite started looking at that kind of de-duping yet, but feel free to jump in, the more the merrier! I was wondering if we can construct more semantic-like (or at least non-random) classnames, then perhaps using the class name as a key might be a way to naturally de-dupe... I'm sure there could be lots of issues with attempting that, but might be one way to explore it 🙂

[ramonjd]

I was wondering if we can construct more semantic-like (or at least non-random) classnames, then perhaps using the class name as a key might be a way to naturally de-dupe... I'm sure there could be lots of issues with attempting that, but might be one way to explore

For sure!

I'm starting with exploring different registry models, which might make it easier to parse/dedupe registered styles, e.g.,

			WP_Style_Engine_Gutenberg::get_instance()->add_styles(
				array(
					"$selector > :where(:not(.alignleft):not(.alignright))" =>
					array(
						'max-width'    => esc_html( $all_max_width_value ),
						'margin-left'  => 'auto !important',
						'margin-right' => 'auto !important',
					),
				)
			);

Not sure where it's going yet, but starting like that 😄

[andrewserong]

Oh, nice! The array based approach looks pretty cool. I started having a play with returning the class name from add_style so that we can start building out an array of class names to add, so thought I might try something like the following:

			$class_names[] = $style_engine->add_style(
				"wp-container-all-max-width-$all_max_width_value",
				".wp-container-all-max-width-$all_max_width_value > :where(:not(.alignleft):not(.alignright)) {" .
				'max-width: ' . esc_html( $all_max_width_value ) . ';' .
				'margin-left: auto !important;' .
				'margin-right: auto !important;' .
				'}'
			);

It was starting to look pretty messy, though! So I quite like your idea of an array to split it out to make it much more readable 👍. Looking forward to comparing notes! 😀

[andrewserong]

Inspired by your array idea @ramonjd, I've had a play at writing an add_style function that looks a bit like the following:

	public function add_style( $key, $options ) {
		$class    = ! empty( $options['suffix'] ) ? $key . '-' . sanitize_title( $options['suffix'] ) : $key;
		$selector = ! empty( $options['selector'] ) ? ' ' . trim( $options['selector'] ) : '';
		$rules    = ! empty( $options['rules'] ) ? $options['rules'] : array();
		$prefix   = ! empty( $options['prefix'] ) ? $options['prefix'] : '.';

		if ( ! $class ) {
			return;
		}

		$style = "{$prefix}{$class}{$selector} {\n";

		if ( is_string( $rules ) ) {
			$style .= '  ';
			$style .= $rules;
		} else {
			foreach( $rules as $rule => $value ) {
				$style .= "  {$rule}: {$value};\n";
			}
		}
		$style .= "}\n";

		$this->registered_styles[ $class . $selector ] = $style;

		return $class;
	}

This then allows us to call it like:

			// Add value specific content size.
			$class_names[] = $style_engine->add_style(
				'wp-layout-default-content-size',
				array(
					'suffix'   => $all_max_width_value,
					'selector' => '> :where(:not(.alignleft):not(.alignright))',
					'rules' => array(
						'max-width'    => esc_html( $all_max_width_value ),
						'margin-left'  => 'auto !important',
						'margin-right' => 'auto !important',
					)
				)
			);

To then output something like the following:

.wp-layout-default-content-size-650px > :where(:not(.alignleft):not(.alignright)) {
  max-width: 650px;
  margin-left: auto !important;
  margin-right: auto !important;
}

In view-source, for the default layout, in my local environment it's currently looking like this:

I'll continue hacking around with layout.php to see how this might look (it could get a bit more complex with the flex layout), but I just thought I'd share where I'm up to in case I run out of time before the end of the week 😀

I think by combining the class name + a slugified unique value + selector for the key for the style, we should be able to de-dupe the styles somewhat reliably in the style array?

[ramonjd]

Thanks @andrewserong

Very interesting! I think including the extra metadata in options is an excellent idea for extensibility/readability.

I'm wondering if we should store the styles in array format up until the time we want to use them in case we need (or allow via hooks) any preprocessing. I suppose this sort of stuff will become clear later.

I'm also getting similar results doing the following:

	public function add_styles( $styles ) {
		if ( ! is_array( $styles ) ) {
			return array();
		}

		foreach ( $styles as $key => $value ) {
			$value = is_array( $value ) ? $value : array();
			if ( isset( $this->registered_styles[ $key ] ) ) {
				$this->registered_styles[ $key ] = array_merge( $this->registered_styles[ $key ], $value );
			} else {
				$this->registered_styles[ $key ] = $value;
			}
		}
	}

	public function output_styles() {
		$callback = function( $css_selector, $css_ruleset ) {
			$style = "{$css_selector} {\n";
			foreach ( $css_ruleset as $css_property => $css_value ) {
				$style .= "    {$css_property}: {$css_value};\n";
			}
			$style .= "}\n";
			return $style;
		};

		$output = array_map( $callback, array_keys( $this->registered_styles ), array_values( $this->registered_styles ) );
		$output = implode( "\n", $output );

		echo "<style>\n$output</style>\n";
	}

Adding styles:

$styles = array();
$styles[ "$selector > :where(:not(.alignleft):not(.alignright))" ] = array(
    'max-width'    => esc_html( $all_max_width_value ),
    'margin-left'  => 'auto !important',
    'margin-right' => 'auto !important',
);
WP_Style_Engine_Gutenberg::get_instance()->add_styles( $styles );

Output example

.wp-container-5 > :where(:not(.alignleft):not(.alignright)) {
    max-width: 650px;
    margin-left: auto !important;
    margin-right: auto !important;
}

Just got bogged down trying to dedupe, so there's probably a better model or pattern.

[andrewserong]

Ah, nice! Yes, it could be worth keeping the array around so that the method of output can be customised (or possibly filtered, even?) — It'll be interesting to explore further.

I've pushed my exploration in c1b24c2 which also removes the randomly generated class name, but the trade-off there is that we then have more verbose class names added to the markup, which may or may not be a good or a bad thing 😄🤷

[andrewserong]

Of course, the good thing about removing the randomly generated class name is that it's then fairly trivial to de-dupe as we never wind up with duplicates in the registered_styles array.

[ramonjd]

removes the randomly generated class name, but the trade-off there is that we then have more verbose class names added to the markup

Nice!!

I like the approach, but I think we'd have to be careful about the classnames we generate. Folks could start relying on wp-layout-default-wide-size-1000px!

But if used in conjunction with standardized naming schemes it would be ace.

[ramonjd]

Too early to think about BEM? E.g., wp-layout__flex-orientation-vertical--justify-right 😄

[andrewserong]

I think a BEM-like naming convention would be a good way to go if we wind up using this highly-specific class based approach 👍

[ramonjd]

I know it's implied – and also that this is all highly experimental – but I guess we'll have to replicate all this in the editor(s) as well.

[andrewserong]

Yeah, totally. I figured a good place to start is with support for the front-end of the site, and if we settle on an approach, then we'd want to roll it out everywhere. I'm imagining this PR more as a discussion point rather than necessarily the right path forward, but happy to keep hacking away here, too, of course!

[ramonjd]

❤️

This is a creative way of getting around the unique id. I wonder how we could marry it with some overarching and semi-permanent list of layout tokens (and their values, assuming they exist in theme.json)

[andrewserong]

It would be good to centralise somewhere the different parts of the class name instead of having them added in an ad hoc way? 🤔

[ramonjd]

It would be good to centralise somewhere the different parts of the class name instead of having them added in an ad hoc way?

Yeah, I wonder how much we can get from a set of defaults + theme.json overrides.

Taking wp-layout-default-content-size-${ suffix } as an example, I'm seeing a few morphological units.

Assuming I knew nothing about the layout implementation, but am familiar with Gutenberg blocks:

1. wp-layout. This is familiar. I suppose it belongs to the block that uses this support feature. A reliable hook that will always be there.
2. default. This doesn't reveal much information. Is the default layout relate to the CSS rules that will be applied if I do nothing? Could this mean "fluid", or "flow"?
3. content. Stringing the morphemes together, I think we're talking about the layout of a piece of content, and because it's block supports, the content of the block.
4. size - I guess we're talking about "width", the value of which could be derived from a set of design rules, support by the wider system, or random (?)

For example, I'd find it something like the following reasonably readable:

<div class="wp-layout wp-layout__flex wp-layout__width--large" />

Might be too BEM.

Still, there are probably some things we could borrow from BEM, tailwind or cube, which uses data attributes as selectors for exceptions. Very interesting!

[markhowellsmead]

Please don't use data attributes. They're for holding data; class names are for this specific purpose.

[ramonjd]

Thinking way ahead here. Is it premature to stringify the values? I wonder if there's any preprocessing, whether via hooks or otherwise that we'd need to do before rendering.

Just thinking out loud.

[andrewserong]

Great point — it might be better to do the stringification (if that's a word? 😄) on output instead so that there's more flexibility 👍

[andrewserong]

I've just given this a rebase and have had a play in 842b83d at splitting out the generation of each Layout type into separate functions. There might be a better way we'd like to do it longer-term, but at least in the shorter-term it helps reduce the length of the functions a bit, and gutenberg_get_layout_style is just responsible for calling out to those other functions.

[ramonjd]

I wonder if there's a neat way to identify classes/CSS that are for backwards compatibility (and don't correspond to any HTML element on the page)?

I know these classes have to be there to support previous versions, but it creates a bit of noise on the frontend.

I'm just throwing out wild ideas, but even if they were batched and printed out in a separate section.

It's not a huge deal, mainly for organization and code digestion purposes 😄

[andrewserong]

It’s a good point! Anything we can do to tidy up and make these functions more readable / comprehensible and easy to change sounds good to me 😀

[scruffian]

Thanks for your work on this. I have some thoughts, but I'm not very familiar with this work so I might be missing some context. Please forgive me if I'm missing things!

The concept of the "default layout" is a whole collection of different CSS rules which all work together to produce a particular type of layout. Given that all of these rules combine to create a particular effect we could use the same class name for all of the rules which makes the intention clear and helps to keep the rules scoped. In the same way I don't think we need the specific wp-layout-default-content-size-650px class, as we know the content size, so we can output that in the CSS wthout needing a specific class.

The way I see this working conceptually is that we'll have many different layout types in the future. Switching the layout type on the parent would result in a totally different flow/layout for the children. If we can keep each layout scoped to one class name then as we add more it will be much easier to reason about.

There is also a risk in creating many class names - theme developers will start to rely on them which can lead to backcompt issues to juggle.

[ramonjd]

Thanks for the notes @scruffian

Given that all of these rules combine to create a particular effect we could use the same class name for all of the rules which makes the intention clear and helps to keep the rules scoped.... If we can keep each layout scoped to one class name then as we add more it will be much easier to reason about.

I was playing around with some more scoped CSS classnames yesterday and had intended to take it further either today or tomorrow.

The gist is

<div class="wp-layout wp-layout__flow [ is-content-size-900px is-wide-size-1240px is-gap-24px ]" />

// Example CSS:

.wp-layout__flow.is-content-size-900px {
    max-width: 900px;
    margin-left: auto !important;
    margin-right: auto !important;
}
.wp-layout__flow.is-wide-size-1240px {
    max-width: 1240px;
}

@andrewserong also had the idea of

the [BEM] modifier -- is a better fit for splitting out values within the class name? I guess a bit like the custom properties in Gutenberg documented here --wp--custom--{key}--{nested-key}

So we could incorporate that syntax.

In the same way I don't think we need the specific wp-layout-default-content-size-650px class, as we know the content size, so we can output that in the CSS wthout needing a specific class.

Is that for the default content size coming from the theme? Some blocks allow custom content sizes and widths so we might have to create some transient modifiers. At least that's the way I've understood things so far. I might be way off there.

There is also a risk in creating many class names - theme developers will start to rely on them which can lead to backcompt issues to juggle.

Good point! Totally agree we should be avoiding creating too many. That's why I think we should definitely pursue the idea of scoping things according to the layout.

[scruffian]

Some blocks allow custom content sizes and widths so we might have to create some transient modifiers.

Would it be possible to do these using inline styles?

[andrewserong]

Thanks for taking a look @scruffian, lots of great questions! This is all very much experimental / exploratory at this stage, as we're mostly trying things out to see how it could all potentially fit together.

Given that all of these rules combine to create a particular effect we could use the same class name for all of the rules which makes the intention clear and helps to keep the rules scoped

A tricky thing here is that we want to try to de-dupe common styles as much as possible, so if we group too many of the unique settings together, it's easy for the styling rules to balloon out. While this PR possibly adds too many class names at the moment, we'd like to drastically reduce the number of unique styling rules that get created (currently on trunk it's one per block even when styles are shared). So far, de-duping by unique selector name seems like a fairly low cost way to do it, and that sort of nudged us in the direction of unique class names, for better or worse! 😅

If we can keep each layout scoped to one class name then as we add more it will be much easier to reason about.

I couldn't quite think of a way of doing that without still having the class name with the randomly generated numbers (e.g. wp-container-12345) when the layout involves any unique settings — and we'd then still have the duplication of styling rules. Having separate class names also helps us avoid having to have a nested data structure for storing style rules, which would move the complexity around a bit — the class names might be easier to reason about, but then the styling rules in the style engine would no longer be a flat structure? 🤔

Would it be possible to do these using inline styles?

I think part of the goal is to move away from relying on inline styles, particularly when we get to complex blocks where we might need to target a child selector instead of using inline styles on the wrapper element. Between content size, gap, and alignments, there's quite a few different modifiers we'll wind up needing, so having separate class names gives us a bit more flexibility to have a separation between unique modifiers and common styles.

There is also a risk in creating many class names - theme developers will start to rely on them which can lead to backcompt issues to juggle.

That's a great point, too. If we do use a common naming convention without random numbers, it does mean we're sort of committing ourselves long-term to those as an API. One of the things I've had in the back of my mind with this work is that we need to (in the long-term) come up with a fix for rendering block supports styles in async contexts like the post content block within the editor (see: #35376) — so we do need a way of exposing the generated styling rules in some capacity in a public-ish API (either via inferring by classnames, or being able to fetch those styles separately) for some needs anyway. Not sure how that'll impact decisions here, but just something for us to keep in mind!

[youknowriad]

Thanks for starting this exploration, there's a lot to learn. Here are some quick thoughts on the subject.

A tricky thing here is that we want to try to de-dupe common styles as much as possible

I think shipping the minimum amount of CSS is a good goal but as mentioned above the approach taken in this PR give seemingly meaningful classnames that theme authors could be tempted to use to override stuff, but since these are auto-generated based on values, they are not meant for that. In other words, maybe here we should create random obfuscated classnames instead like most CSS-in-JS solutions do.

Consolidating style tags for layout support

This PR seem go solve the default layout duplicated styles as an indirect consequence, I think we should probably solve it directly by making it a preset (because that's what it is conceptually). #36944

[scruffian]

I couldn't quite think of a way of doing that without still having the class name with the randomly generated numbers (e.g. wp-container-12345) when the layout involves any unique settings — and we'd then still have the duplication of styling rules.

It seems like there are some rules that will be applied to all block that use this layout, and some that are unique per block. We should separate these so that one class contains all the generic rules that apply to all blocks that use this layout and then apply the unique settings though inline styles or a unique class.

I think part of the goal is to move away from relying on inline styles, particularly when we get to complex blocks where we might need to target a child selector instead of using inline styles on the wrapper element.

Yeah that is where things get complicated. The alternative would be to specify a selector for each support so that the style engine knows which element to apply each style to.

[andrewserong]

Thanks for the extra feedback! This discussion is super helpful for us.

This PR seem go solve the default layout duplicated styles as an indirect consequence, I think we should probably solve it directly by making it a preset (because that's what it is conceptually).

That's a great point — at the moment we're still generating the styles each time, but as you mention the de-duping is happening as a side-effect rather than as a preset. I'll take a look next week at seeing if we can split that out in a more logical / separated way, more like a preset, so it's only the unique values that have to be generated for each block instance 👍

but since these are auto-generated based on values, they are not meant for that. In other words, maybe here we should create random obfuscated classnames instead like most CSS-in-JS solutions do.

That's a good point, too — but I'm a bit ambivalent about the obfuscated classnames. If the only benefit is obfuscation, adding in a hashing approach to link together a block with its styles feels like a bit more complexity than we need, if we can instead use the value as the linking identifier. But I suppose I've also been thinking about storing the value in the classname so we can usefully reverse engineer the values to solve issues like the async rendering in #35376 — so my thinking about this side of things is probably a bit biased toward wanting to solve that issue! Whether or not folks go to use the class name directly, I kinda prefer class names that point to their usage, so would lean toward seeing what we can solve with clearer naming conventions that it's a custom value over obfuscation. But I think I see your point, even trying to flag that a value is custom, like in the following, it still sort of looks like it's a stable rule: 🤔

.is-custom-content-size--650px
.is-custom-wide-size--1000px

It seems like there are some rules that will be applied to all block that use this layout, and some that are unique per block. We should separate these so that one class contains all the generic rules that apply to all blocks that use this layout and then apply the unique settings though inline styles or a unique class.

I think this is already happening to an extent in this PR, most of the common styles are linked to the root wp-layout-flex or wp-layout-flow class, it's just that there's then quite a few unique values. Perhaps it'll become clearer what the separation is when we look at the presets next week 🤔 I'll have a play! 😃

[andrewserong]

I've made a start on a very naive variant of this PR in #39374 that looks at hooking into the WP_Theme_JSON_Gutenberg class to call out to the Layout block support to generate preset-like common styling rules. It's very early and doesn't do anything useful yet (it just proves that we can pass styles up to the global styles stylesheet), but I'll continue hacking away at it to explore some of the ideas next week.

(It's not quite ready for another pair of eyes yet, either, but just thought I'd share where I got up to at the end of this week!) 😁

[ramonjd]

That's a good point, too — but I'm a bit ambivalent about the obfuscated classnames. If the only benefit is obfuscation, adding in a hashing approach to link together a block with its styles feels like a bit more complexity than we need, if we can instead use the value as the linking identifier. But I suppose I've also been thinking about storing the value in the classname so we can usefully reverse engineer the values to solve issues like the async rendering

I've toyed around with a very simple obfuscation feature over in https://github.com/WordPress/gutenberg/pull/39446/files#diff-df6c4ae64dee8330db2c09f5c60668cea91dfcbbad4e5143055c5c98e29ccbf5R148

It outputs something like:

.wp-my-key__002d308c {
    color: pink;
}

As for reverse engineer, do you mean being able to search and retrieve registered CSS rules from the styles engine store?

If the arguments are the same, the hash value should also be the same, so I think things could be still retrievable?

Or did you see this working another way?

Edit: Just saw the comment over on the 👍 #39446 (comment)

combines our two approaches by using the unique / semantic classname as the hash

Let's hope!! Thanks!

[andrewserong]

As for reverse engineer, do you mean being able to search and retrieve registered CSS rules from the styles engine store?

Not exactly. The problem in the back of my mind (that we totally don't have to solve just yet) is this:

• In a rendered view of a page, there's a certain amount of styles already loaded (e.g. in our layout presets exploration we'll have some default / base / common CSS styles for layouts available)
• When we async fetch some more post content (either in a post content preview in the editor, or via an AJAX-ey interface like infinite scroll), how do we ensure we render styles for that newly fetched post content?
• For those blocks rendered with common class names that correspond to common / base styling, we'll have covered that via presets, as the corresponding CSS is already loaded
• For anything with unique values (or unique class names) how do we render corresponding CSS for that post content we've just fetched? (Which does not include the rendered style tags)

One (potentially very) hacky way to do it, if we have semantic class names that include the values in them, is to process the fetched post content, and extract the classnames, and then re-generate the unique styles based on those class names. I don't love that idea, because it's quite a lot of processing to do with some content that's just been fetched — so it'd be great if we didn't need to do that. But I've also been struggling to come up with a better way to do it, too! 😅

I guess the bigger question for us (and again, I don't think this should block or slow down our exploration), is — what's the simplest way we could retrieve the unique styles that are generated by the style engine, for an arbitrary piece of post content. Hopefully, via presets, we limit the volume of these styles that we need to deal with in this way, but there'll still be some 🙂

Edit: Just saw the comment over on the 👍 #39446 (comment)

For the moment, one of the reasons I like that approach, is that if in our calling code, we're still passing all the values for more semantic-style class names, but then switching them out for an obfuscated class name in the style engine, it'll be easy to change after the fact if we want to go in another direction.

The async problem is a bit of a hard one to solve, so for now, I'm just really happy if we can implement things where we feel pretty confident we can make further changes easily enough if we'd like to explore different ways of doing it. And so far, I think we're heading in that direction with the explorations 😀

[ramonjd]

process the fetched post content, and extract the classnames, and then re-generate the unique styles based on those class names

Ah, you mean reverse engineering!

Ha, seriously, thanks a lot for the explainer.

Yeah that does sound tricky. Unless we come up with a Gutenberg enigma code 🤣

class="__fsX140ScXfffStdX0SbtX20" > font-size: 14px; color: #fff; text-decoration: none; border-top: 2px;

[andrewserong]

Closing out this experiment now that #40875 has landed which progresses a consolidated class-based approach to common layout styles (and the style engine is progressing nicely toward style optimisation, too).