fix: improve element class attribute behaviour (#10856)

* fix: improve element class attribute behaviour

* Update packages/svelte/src/internal/client/dom/elements/class.js

Co-authored-by: Simon H <[email protected]>

---------

Co-authored-by: Simon H <[email protected]>

Author: trueadm

.changeset/selfish-spies-help.md

@@ -0,0 +1,5 @@
+---
+"svelte": patch
+---
+
+fix: improve element class attribute behaviour

packages/svelte/src/compiler/phases/3-transform/client/visitors/template.js

@@ -473,6 +473,7 @@ function serialize_dynamic_element_attributes(attributes, context, element_id) {
 function serialize_element_attribute_update_assignment(element, node_id, attribute, context) {
 	const state = context.state;
 	const name = get_attribute_name(element, attribute, context);
+	const is_svg = context.state.metadata.namespace === 'svg';
 	let [contains_call_expression, value] = serialize_attribute_value(attribute.value, context);
 
 	// The foreign namespace doesn't have any special handling, everything goes through the attr function
@@ -507,13 +508,19 @@ function serialize_element_attribute_update_assignment(element, node_id, attribu
 		if (name === 'class') {
 			if (singular) {
 				return {
-					singular: b.stmt(b.call('$.class_name_effect', node_id, b.thunk(singular))),
-					grouped: b.stmt(b.call('$.class_name', node_id, singular)),
+					singular: b.stmt(
+						b.call(
+							is_svg ? '$.svg_class_name_effect' : '$.class_name_effect',
+							node_id,
+							b.thunk(singular)
+						)
+					),
+					grouped: b.stmt(b.call(is_svg ? '$.svg_class_name' : '$.class_name', node_id, singular)),
 					skip_condition: true
 				};
 			}
 			return {
-				grouped: b.stmt(b.call('$.class_name', node_id, value)),
+				grouped: b.stmt(b.call(is_svg ? '$.svg_class_name' : '$.class_name', node_id, value)),
 				skip_condition: true
 			};
 		} else if (!DOMProperties.includes(name)) {

packages/svelte/src/internal/client/dom/elements/class.js

@@ -3,7 +3,7 @@ import { set_class_name } from '../operations.js';
 import { render_effect } from '../../reactivity/effects.js';
 
 /**
- * @param {Element} dom
+ * @param {HTMLElement} dom
  * @param {() => string} value
  * @returns {void}
  */
@@ -14,7 +14,47 @@ export function class_name_effect(dom, value) {
 }
 
 /**
- * @param {Element} dom
+ * @param {SVGElement} dom
+ * @param {() => string} value
+ * @returns {void}
+ */
+export function svg_class_name_effect(dom, value) {
+	render_effect(() => {
+		svg_class_name(dom, value());
+	});
+}
+
+/**
+ * @param {SVGElement} dom
+ * @param {string} value
+ * @returns {void}
+ */
+export function svg_class_name(dom, value) {
+	// @ts-expect-error need to add __className to patched prototype
+	var prev_class_name = dom.__className;
+	var next_class_name = to_class(value);
+
+	if (hydrating && dom.getAttribute('class') === next_class_name) {
+		// In case of hydration don't reset the class as it's already correct.
+		// @ts-expect-error need to add __className to patched prototype
+		dom.__className = next_class_name;
+	} else if (
+		prev_class_name !== next_class_name ||
+		(hydrating && dom.getAttribute('class') !== next_class_name)
+	) {
+		if (next_class_name === '') {
+			dom.removeAttribute('class');
+		} else {
+			dom.setAttribute('class', next_class_name);
+		}
+
+		// @ts-expect-error need to add __className to patched prototype
+		dom.__className = next_class_name;
+	}
+}
+
+/**
+ * @param {HTMLElement} dom
  * @param {string} value
  * @returns {void}
  */
@@ -31,7 +71,10 @@ export function class_name(dom, value) {
 		prev_class_name !== next_class_name ||
 		(hydrating && dom.className !== next_class_name)
 	) {
-		if (next_class_name === '') {
+		// Removing the attribute when the value is only an empty string causes
+		// peformance issues vs simply making the className an empty string. So
+		// we should only remove the class if the the value is nullish.
+		if (value == null) {
 			dom.removeAttribute('class');
 		} else {
 			set_class_name(dom, next_class_name);

packages/svelte/tests/hydration/samples/element-attribute-removed/_before.html

@@ -1 +1 @@
-<!--ssr:0--><div class="foo"></div><!--ssr:0-->
+<!--ssr:0--><div id="foo"></div><!--ssr:0-->

packages/svelte/tests/hydration/samples/element-attribute-removed/main.svelte

@@ -1,5 +1,5 @@
 <script>
-	export let className;
+	export let id;
 </script>
 
-<div class={className}></div>
+<div id={id}></div>

packages/svelte/tests/runtime-legacy/samples/component-binding-infinite-loop/C.svelte

@@ -13,7 +13,7 @@
 
 <span
 	on:click="{toggle}"
-	class="{isCurrentlySelected ? 'selected' : ''}"
+	class="{isCurrentlySelected ? 'selected' : null}"
 >
 	<slot></slot>
-</span>
+</span>