chore: rewrite set_style() to handle directives

[adiguba]

This is the #15352 counterpart for style:directive

Currently the style:xxx directives are managed separately from the style attribute.
This is a PoC for rewriting set_style() in order to includes class: directives.

• Closes Spread is not consistent with attribute for style #15309
• Closes bug: SSR merge_styles() don't handle correctly some style attributes #15416
• Avoid unnecessary mutations during hydration

Note that I edited a test, for removing an empty attribute
Basically :

-	<p style=""></p>
+	<p></p>

I haven't had time to do a test for this yet.
I'll do it this week...

Before submitting the PR, please make sure you do the following

• It's really useful if your PR references an issue where it is discussed ahead of time. In many cases, features are absent for a reason. For large changes, please create an RFC: https://github.com/sveltejs/rfcs
• Prefix your PR title with feat:, fix:, chore:, or docs:.
• This message body should clearly illustrate what problems it solves.
• Ideally, include a test that fails without this PR but passes with it.
• If this PR changes code within packages/svelte/src, add a changeset (npx changeset).

Tests and linting

• Run the tests with pnpm test and lint the project with pnpm lint

[changeset-bot]

🦋 Changeset detected

Latest commit: 4894e3d

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 1 package

Name	Type
svelte	Patch

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

[svelte-docs-bot]

Preview: https://svelte-dev-git-preview-svelte-15418-svelte.vercel.app/

[github-actions]

Playground

pnpm add https://pkg.pr.new/svelte@15418

[adiguba]

Reverted a last-minute change thaht break tests...

Using dom.style.cssText seem have better performance than dom.setAttribute() : https://www.measurethat.net/Benchmarks/Show/15479/0/setattribute-vs-stylecsstext#latest_results_block
But with cssText the browser interprets the value which alters the result for the tests.

Note sure what we should do...

[adiguba]

I just adjusted some tests to allow the use of style.cssText, typically by adding a trailing ; (because style.cssText add it automatically when missing)

[dummdidumm]

What is the reason behind separating important from non-important styles and setting them separately in DOM?
Also, is it possible to avoid the to_style call (which adds some perf and bundle size overhead) in the DOM (and maybe even the server) by just concatenating styles and letting the browser figure out the duplicates?

[adiguba]

What is the reason behind separating important from non-important styles and setting them separately in DOM?

I haven't found a better way to handle the |important modifier...

Also, is it possible to avoid the to_style call (which adds some perf and bundle size overhead) in the DOM (and maybe even the server) by just concatenating styles and letting the browser figure out the duplicates?

This can be a (small) breaking change if the initial style use an !important on the same property.
Ex:

<div style="color: red !important" style:color={'blue'}>Hello World</div>

=> "color: red !important; color: blue" is different from a "color: blue"

But yes that would simplify a lot of things...

Also the set_style() in DOM helps to avoid multiple mutations during hydration...
But maybe it's not that bad.

[dummdidumm]

This can be a (small) breaking change if the initial style use an !important on the same property.

Honestly I would call this a bugfix, because !important means ... well, it's more important.

Also the set_style() in DOM helps to avoid multiple mutations during hydration

How exactly? Aren't all styles collected and then applied to cssText a single time? So it would only be one mutation.

[adiguba]

Honestly I would call this a bugfix, because !important means ... well, it's more important.

In this case the to_style() function will be greatly simplified :

export function to_style(value, styles) {
	if (styles) {
		var new_style = value ? String(value).trim() + ';' : '';

		if (Array.isArray(styles)) {
			new_style += append_styles(styles[0]);
			new_style += append_styles(styles[1], true);
		} else {
			new_style += append_styles(styles);
		}
		return new_style === '' ? null : new_style;
	} else if (value == null) {
		return null;
	}
	return String(value);
}

How exactly? Aren't all styles collected and then applied to cssText a single time? So it would only be one mutation.

Yes in this PR with to_style(), but not in current version (or if we remove to_style())

Take the following code (example in REPL)

	<div {style} style:color style:border>Hello World</div>

On server-side it will render something like that : "background: black; color: white; border: 1px solid red;". OK

But on client-site it's currently handheld by 3 differents function call :

	$.set_attribute(div, 'style', style());
	$.set_style(div, 'color', color());
	$.set_style(div, 'border', border());

On hydration there will be mismatch and the style is modified 3 times, even though the server generated the correct code :

• set_attribute() will found a mismatch and set the style to "background: black;"
• The first set_style() will add the "color: white"
• The second set_style will add the "border: 1px solid red"

[adiguba]

Using the "simplified" version of to_style() will break some tests which precisely checks that style:directives remove existing properties, like :

runtime-browser/inline-style-directive-precedence
runtime-legacy/inline-style-directive-and-style-attr-merged
runtime-legacy/inline-style-directive-and-style-attr-merged
runtime-legacy/inline-style-directive-spread-and-attr
runtime-legacy/inline-style-directive-spread-and-attr-empty

An alternative would be to use to_style() only on the server side.
The multiple mutations are not really problematic...

[Rich-Harris]

This is great! In addition to fixing the bugs, getting rid of is_attributes_reactive allowed for a chain reaction of simplifying code