bug: The class: directive don't remove existing className on SSR

[adiguba]

Describe the bug

Hi,

This dummy code :

<div class="red" class:red={false}></div>

Should produce something like <div class=""></div> (or <div></div>), because the directive takes precedence over the class attribute.
But that not the case on SSR where the generated code is <div class="red"></div>, which can cause a flash on hydration.

Reproduction

Demo on Sveltelabs (for SSR).
Hit the [RELOAD] button to refresh the page :
https://www.sveltelab.dev/je2q9zussa08osn

Logs

System Info

last version

Severity

annoyance

[adiguba]

In fact, beyond this bug, I think that the class attribute and the class: directive should be handheld by an unique function.

=> Currently a code like <div class={className} class:red class:important></div> is translated into something like this :

	$.template_effect(() => {
		$.set_class(div, $.clsx($.get(className)), 'svelte-12n9c8');
		$.toggle_class(div, 'red', $.get(red));
		$.toggle_class(div, 'important', $.get(important));
	});

With the following states: className: 'hello-world', red: true, important: true :

1. SSR will generate this : <div class="hello-world svelte-12n9c8 red important"></div>.

2. On hydration the class attribute will be modified up to 3 times :

   • set_class() will set the class to the value of className (+hash) (class="hello-world svelte-12n9c8")
   • The first toggle_class() will add "red" if truely (so class="hello-world svelte-12n9c8 red")
   • The second toggle_class() will add "important" if truely, (so class="hello-world svelte-12n9c8 red important")

3. Same thing on any update of className, where set_class() will reset the class attribute, and each toggle_class() will update it again.

So, I think that the toggle_class() should be integrated into set_class() using a local variable for previous values (like set_attributes() for spreading).

Something like this :

	let classes;

	$.template_effect(() => {
		classes = $.set_class(div, $.clsx($.get(className)), 'svelte-12n9c8', classes, {
			red: $.get(red), important: $.get(important)
		});
	});

By grouping all the processing, this would allow the class to be updated only once when needed.

This should also apply to style and style: for the same reasons...

If needed, I think I can develop a POC of this.

[paoloricciuti]

A single method would also solve the fact that that part of the codebase is a nightmare to touch since it relies on the order of things to be precise sometimes pushing updates in the init phase. But a single method would also mean that for every class toggle we would rerun the logic for all the classes no?

[adiguba]

But a single method would also mean that for every class toggle we would rerun the logic for all the classes no?

I think it can handle the two cases.
In the example : <div class={className} class:red class:important></div>

• If the className is modified, we need to rebuild the full class in one step.
• Otherwise if red or important is modified, we can just add/remove the value.

Will try to make a POC...