chore: perf tweaks for actions/styles/classes (#12654)

* chore: perf tweaks for actions/styles/classes

- check if we really need to add/remove the class (calling `includes` first is cheaper than always setting/removing it)
- check if we really need to update a style (calling `getPropertyValue/setProperty` is expensive)
- check if we should call the action's update function (this is not only a perf tweak but also a correctness fix)

closes #12652

* changeset

---------

Co-authored-by: Rich Harris <[email protected]>

Author: dummdidumm

.changeset/sharp-fishes-serve.md

@@ -0,0 +1,5 @@
+---
+"svelte": patch
+---
+
+feat: perf tweaks for actions/styles/classes

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

@@ -83,8 +83,15 @@ function get_attribute_name(element, attribute, context) {
  * @param {Identifier} element_id
  * @param {ComponentContext} context
  * @param {boolean} is_attributes_reactive
+ * @param {boolean} force_check Should be `true` if we can't rely on our cached value, because for example there's also a `style` attribute
  */
-function serialize_style_directives(style_directives, element_id, context, is_attributes_reactive) {
+function serialize_style_directives(
+	style_directives,
+	element_id,
+	context,
+	is_attributes_reactive,
+	force_check
+) {
 	const state = context.state;
 
 	for (const directive of style_directives) {
@@ -99,7 +106,8 @@ function serialize_style_directives(style_directives, element_id, context, is_at
 				element_id,
 				b.literal(directive.name),
 				value,
-				/** @type {Expression} */ (directive.modifiers.includes('important') ? b.true : undefined)
+				/** @type {Expression} */ (directive.modifiers.includes('important') ? b.true : undefined),
+				force_check ? b.true : undefined
 			)
 		);
 
@@ -2062,6 +2070,7 @@ export const template_visitors = {
 		let img_might_be_lazy = false;
 		let might_need_event_replaying = false;
 		let has_direction_attribute = false;
+		let has_style_attribute = false;
 
 		if (is_custom_element) {
 			// cloneNode is faster, but it does not instantiate the underlying class of the
@@ -2080,6 +2089,9 @@ export const template_visitors = {
 				if (attribute.name === 'dir') {
 					has_direction_attribute = true;
 				}
+				if (attribute.name === 'style') {
+					has_style_attribute = true;
+				}
 				if (
 					(attribute.name === 'value' || attribute.name === 'checked') &&
 					!is_text_attribute(attribute)
@@ -2229,7 +2241,13 @@ export const template_visitors = {
 
 		// class/style directives must be applied last since they could override class/style attributes
 		serialize_class_directives(class_directives, node_id, context, is_attributes_reactive);
-		serialize_style_directives(style_directives, node_id, context, is_attributes_reactive);
+		serialize_style_directives(
+			style_directives,
+			node_id,
+			context,
+			is_attributes_reactive,
+			has_style_attribute || node.metadata.has_spread
+		);
 
 		if (might_need_event_replaying) {
 			context.state.after_update.push(b.stmt(b.call('$.replay_events', node_id)));
@@ -2390,7 +2408,13 @@ export const template_visitors = {
 
 		// class/style directives must be applied last since they could override class/style attributes
 		serialize_class_directives(class_directives, element_id, inner_context, is_attributes_reactive);
-		serialize_style_directives(style_directives, element_id, inner_context, is_attributes_reactive);
+		serialize_style_directives(
+			style_directives,
+			element_id,
+			inner_context,
+			is_attributes_reactive,
+			true
+		);
 
 		const get_tag = b.thunk(/** @type {Expression} */ (context.visit(node.tag)));
 

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

@@ -1,5 +1,6 @@
 /** @import { ActionPayload } from '#client' */
 import { effect, render_effect } from '../../reactivity/effects.js';
+import { safe_not_equal } from '../../reactivity/equality.js';
 import { deep_read_state, untrack } from '../../runtime.js';
 
 /**
@@ -15,6 +16,8 @@ export function action(dom, action, get_value) {
 
 		if (get_value && payload?.update) {
 			var inited = false;
+			/** @type {P} */
+			var prev = /** @type {any} */ ({}); // initialize with something so it's never equal on first run
 
 			render_effect(() => {
 				var value = get_value();
@@ -24,7 +27,8 @@ export function action(dom, action, get_value) {
 				// together with actions and mutation, it wouldn't notice the change without a deep read.
 				deep_read_state(value);
 
-				if (inited) {
+				if (inited && safe_not_equal(prev, value)) {
+					prev = value;
 					/** @type {Function} */ (payload.update)(value);
 				}
 			});

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

@@ -107,8 +107,10 @@ function to_class(value) {
  */
 export function toggle_class(dom, class_name, value) {
 	if (value) {
+		if (dom.classList.contains(class_name)) return;
 		dom.classList.add(class_name);
 	} else {
+		if (!dom.classList.contains(class_name)) return;
 		dom.classList.remove(class_name);
 	}
 }

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

@@ -3,15 +3,23 @@
  * @param {string} key
  * @param {string} value
  * @param {boolean} [important]
+ * @param {boolean} [force_check] Should be `true` if we can't rely on our cached value, because for example there's also a `style` attribute
  */
-export function set_style(dom, key, value, important) {
-	const style = dom.style;
-	const prev_value = style.getPropertyValue(key);
+export function set_style(dom, key, value, important, force_check) {
+	// @ts-expect-error
+	var attributes = (dom.__attributes ??= {});
+	var style = dom.style;
+	var style_key = 'style-' + key;
+
+	if (attributes[style_key] === value && (!force_check || style.getPropertyValue(key) === value)) {
+		return;
+	}
+
+	attributes[style_key] = value;
+
 	if (value == null) {
-		if (prev_value !== '') {
-			style.removeProperty(key);
-		}
-	} else if (prev_value !== value) {
+		style.removeProperty(key);
+	} else {
 		style.setProperty(key, value, important ? 'important' : '');
 	}
 }