From 8c2f6039c6ca8203bc0ee2420e9c992404b33eea Mon Sep 17 00:00:00 2001
From: Dominic Gannaway <trueadm@users.noreply.github.com>
Date: Thu, 21 Mar 2024 13:39:29 +0000
Subject: [PATCH] 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 <5968653+dummdidumm@users.noreply.github.com>

---------

Co-authored-by: Simon H <5968653+dummdidumm@users.noreply.github.com>
---
 .changeset/selfish-spies-help.md              |  5 ++
 .../3-transform/client/visitors/template.js   | 13 +++--
 .../src/internal/client/dom/elements/class.js | 49 +++++++++++++++++--
 .../element-attribute-removed/_before.html    |  2 +-
 .../element-attribute-removed/main.svelte     |  4 +-
 .../component-binding-infinite-loop/C.svelte  |  4 +-
 6 files changed, 66 insertions(+), 11 deletions(-)
 create mode 100644 .changeset/selfish-spies-help.md

diff --git a/.changeset/selfish-spies-help.md b/.changeset/selfish-spies-help.md
new file mode 100644
index 0000000000..2a06251e1a
--- /dev/null
+++ b/.changeset/selfish-spies-help.md
@@ -0,0 +1,5 @@
+---
+"svelte": patch
+---
+
+fix: improve element class attribute behaviour
diff --git a/packages/svelte/src/compiler/phases/3-transform/client/visitors/template.js b/packages/svelte/src/compiler/phases/3-transform/client/visitors/template.js
index 0d6f13aa18..b2ccd60251 100644
--- a/packages/svelte/src/compiler/phases/3-transform/client/visitors/template.js
+++ b/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)) {
diff --git a/packages/svelte/src/internal/client/dom/elements/class.js b/packages/svelte/src/internal/client/dom/elements/class.js
index 84466e8058..ffb4f5bea1 100644
--- a/packages/svelte/src/internal/client/dom/elements/class.js
+++ b/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);
diff --git a/packages/svelte/tests/hydration/samples/element-attribute-removed/_before.html b/packages/svelte/tests/hydration/samples/element-attribute-removed/_before.html
index fec75ee427..4bb2d47ff3 100644
--- a/packages/svelte/tests/hydration/samples/element-attribute-removed/_before.html
+++ b/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-->
diff --git a/packages/svelte/tests/hydration/samples/element-attribute-removed/main.svelte b/packages/svelte/tests/hydration/samples/element-attribute-removed/main.svelte
index c33bc0981c..959352e727 100644
--- a/packages/svelte/tests/hydration/samples/element-attribute-removed/main.svelte
+++ b/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>
\ No newline at end of file
+<div id={id}></div>
diff --git a/packages/svelte/tests/runtime-legacy/samples/component-binding-infinite-loop/C.svelte b/packages/svelte/tests/runtime-legacy/samples/component-binding-infinite-loop/C.svelte
index 0bfc2f1d4e..8226de6d71 100644
--- a/packages/svelte/tests/runtime-legacy/samples/component-binding-infinite-loop/C.svelte
+++ b/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>
\ No newline at end of file
+</span>