fix: stable attachments

.changeset/soft-jobs-sip.md

@@ -0,0 +1,5 @@
+---
+'svelte': patch
+---
+
+fix: avoid rerunning attachments when unrelated spread attributes change

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

@@ -17,7 +17,7 @@ import { build_getter } from '../utils.js';
 import {
 	get_attribute_name,
 	build_attribute_value,
-	build_set_attributes,
+	build_attribute_effect,
 	build_set_class,
 	build_set_style
 } from './shared/element.js';
@@ -201,37 +201,7 @@ export function RegularElement(node, context) {
 	const node_id = context.state.node;
 
 	if (has_spread) {
-		const attributes_id = b.id(context.state.scope.generate('attributes'));
-
-		build_set_attributes(
-			attributes,
-			class_directives,
-			style_directives,
-			context,
-			node,
-			node_id,
-			attributes_id
-		);
-
-		// If value binding exists, that one takes care of calling $.init_select
-		if (node.name === 'select' && !bindings.has('value')) {
-			context.state.init.push(
-				b.stmt(b.call('$.init_select', node_id, b.thunk(b.member(attributes_id, 'value'))))
-			);
-
-			context.state.update.push(
-				b.if(
-					b.binary('in', b.literal('value'), attributes_id),
-					b.block([
-						// This ensures a one-way street to the DOM in case it's <select {value}>
-						// and not <select bind:value>. We need it in addition to $.init_select
-						// because the select value is not reflected as an attribute, so the
-						// mutation observer wouldn't notice.
-						b.stmt(b.call('$.select_option', node_id, b.member(attributes_id, 'value')))
-					])
-				)
-			);
-		}
+		build_attribute_effect(attributes, class_directives, style_directives, context, node, node_id);
 	} else {
 		/** If true, needs `__value` for inputs */
 		const needs_special_value_handling =
@@ -290,7 +260,8 @@ export function RegularElement(node, context) {
 				const { value, has_state } = build_attribute_value(
 					attribute.value,
 					context,
-					(value, metadata) => (metadata.has_call ? get_expression_id(context.state, value) : value)
+					(value, metadata) =>
+						metadata.has_call ? get_expression_id(context.state.expressions, value) : value
 				);
 
 				const update = build_element_attribute_update(node, node_id, name, value, attributes);
@@ -482,10 +453,11 @@ function setup_select_synchronization(value_binding, context) {
 
 /**
  * @param {AST.ClassDirective[]} class_directives
+ * @param {Expression[]} expressions
  * @param {ComponentContext} context
  * @return {ObjectExpression | Identifier}
  */
-export function build_class_directives_object(class_directives, context) {
+export function build_class_directives_object(class_directives, expressions, context) {
 	let properties = [];
 	let has_call_or_state = false;
 
@@ -497,15 +469,16 @@ export function build_class_directives_object(class_directives, context) {
 
 	const directives = b.object(properties);
 
-	return has_call_or_state ? get_expression_id(context.state, directives) : directives;
+	return has_call_or_state ? get_expression_id(expressions, directives) : directives;
 }
 
 /**
  * @param {AST.StyleDirective[]} style_directives
+ * @param {Expression[]} expressions
  * @param {ComponentContext} context
  * @return {ObjectExpression | ArrayExpression}}
  */
-export function build_style_directives_object(style_directives, context) {
+export function build_style_directives_object(style_directives, expressions, context) {
 	let normal_properties = [];
 	let important_properties = [];
 
@@ -514,7 +487,7 @@ export function build_style_directives_object(style_directives, context) {
 			directive.value === true
 				? build_getter({ name: directive.name, type: 'Identifier' }, context.state)
 				: build_attribute_value(directive.value, context, (value, metadata) =>
-						metadata.has_call ? get_expression_id(context.state, value) : value
+						metadata.has_call ? get_expression_id(expressions, value) : value
 					).value;
 		const property = b.init(directive.name, expression);
 
@@ -653,7 +626,7 @@ function build_element_special_value_attribute(element, node_id, attribute, cont
 			? // if is a select with value we will also invoke `init_select` which need a reference before the template effect so we memoize separately
 				is_select_with_value
 				? memoize_expression(state, value)
-				: get_expression_id(state, value)
+				: get_expression_id(state.expressions, value)
 			: value
 	);
 

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

@@ -5,7 +5,11 @@ import { dev, locator } from '../../../../state.js';
 import { is_text_attribute } from '../../../../utils/ast.js';
 import * as b from '#compiler/builders';
 import { determine_namespace_for_children } from '../../utils.js';
-import { build_attribute_value, build_set_attributes, build_set_class } from './shared/element.js';
+import {
+	build_attribute_value,
+	build_attribute_effect,
+	build_set_class
+} from './shared/element.js';
 import { build_render_statement } from './shared/utils.js';
 
 /**
@@ -80,18 +84,15 @@ export function SvelteElement(node, context) {
 	) {
 		build_set_class(node, element_id, attributes[0], class_directives, inner_context, false);
 	} else if (attributes.length) {
-		const attributes_id = b.id(context.state.scope.generate('attributes'));
-
 		// Always use spread because we don't know whether the element is a custom element or not,
 		// therefore we need to do the "how to set an attribute" logic at runtime.
-		build_set_attributes(
+		build_attribute_effect(
 			attributes,
 			class_directives,
 			style_directives,
 			inner_context,
 			node,
-			element_id,
-			attributes_id
+			element_id
 		);
 	}
 

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

@@ -16,28 +16,30 @@ import { build_template_chunk, get_expression_id } from './utils.js';
  * @param {ComponentContext} context
  * @param {AST.RegularElement | AST.SvelteElement} element
  * @param {Identifier} element_id
- * @param {Identifier} attributes_id
  */
-export function build_set_attributes(
+export function build_attribute_effect(
 	attributes,
 	class_directives,
 	style_directives,
 	context,
 	element,
-	element_id,
-	attributes_id
+	element_id
 ) {
-	let is_dynamic = false;
-
 	/** @type {ObjectExpression['properties']} */
 	const values = [];
 
+	/** @type {Expression[]} */
+	const expressions = [];
+
+	/** @param {Expression} value */
+	function memoize(value) {
+		return b.id(`$${expressions.push(value) - 1}`);
+	}
+
 	for (const attribute of attributes) {
 		if (attribute.type === 'Attribute') {
-			const { value, has_state } = build_attribute_value(
-				attribute.value,
-				context,
-				(value, metadata) => (metadata.has_call ? get_expression_id(context.state, value) : value)
+			const { value } = build_attribute_value(attribute.value, context, (value, metadata) =>
+				metadata.has_call ? memoize(value) : value
 			);
 
 			if (
@@ -51,16 +53,11 @@ export function build_set_attributes(
 			} else {
 				values.push(b.init(attribute.name, value));
 			}
-
-			is_dynamic ||= has_state;
 		} else {
-			// objects could contain reactive getters -> play it safe and always assume spread attributes are reactive
-			is_dynamic = true;
-
 			let value = /** @type {Expression} */ (context.visit(attribute));
 
 			if (attribute.metadata.expression.has_call) {
-				value = get_expression_id(context.state, value);
+				value = memoize(value);
 			}
 
 			values.push(b.spread(value));
@@ -72,44 +69,38 @@ export function build_set_attributes(
 			b.prop(
 				'init',
 				b.array([b.id('$.CLASS')]),
-				build_class_directives_object(class_directives, context)
+				build_class_directives_object(class_directives, expressions, context)
 			)
 		);
-
-		is_dynamic ||=
-			class_directives.find((directive) => directive.metadata.expression.has_state) !== null;
 	}
 
 	if (style_directives.length) {
 		values.push(
 			b.prop(
 				'init',
 				b.array([b.id('$.STYLE')]),
-				build_style_directives_object(style_directives, context)
+				build_style_directives_object(style_directives, expressions, context)
 			)
 		);
-
-		is_dynamic ||= style_directives.some((directive) => directive.metadata.expression.has_state);
 	}
 
-	const call = b.call(
-		'$.set_attributes',
-		element_id,
-		is_dynamic ? attributes_id : b.null,
-		b.object(values),
-		element.metadata.scoped &&
-			context.state.analysis.css.hash !== '' &&
-			b.literal(context.state.analysis.css.hash),
-		is_ignored(element, 'hydration_attribute_changed') && b.true
+	context.state.init.push(
+		b.stmt(
+			b.call(
+				'$.attribute_effect',
+				element_id,
+				b.arrow(
+					expressions.map((_, i) => b.id(`$${i}`)),
+					b.object(values)
+				),
+				expressions.length > 0 && b.array(expressions.map((expression) => b.thunk(expression))),
+				element.metadata.scoped &&
+					context.state.analysis.css.hash !== '' &&
+					b.literal(context.state.analysis.css.hash),
+				is_ignored(element, 'hydration_attribute_changed') && b.true
+			)
+		)
 	);
-
-	if (is_dynamic) {
-		context.state.init.push(b.let(attributes_id));
-		const update = b.stmt(b.assignment('=', attributes_id, call));
-		context.state.update.push(update);
-	} else {
-		context.state.init.push(b.stmt(call));
-	}
 }
 
 /**
@@ -167,7 +158,7 @@ export function build_set_class(element, node_id, attribute, class_directives, c
 			value = b.call('$.clsx', value);
 		}
 
-		return metadata.has_call ? get_expression_id(context.state, value) : value;
+		return metadata.has_call ? get_expression_id(context.state.expressions, value) : value;
 	});
 
 	/** @type {Identifier | undefined} */
@@ -180,7 +171,7 @@ export function build_set_class(element, node_id, attribute, class_directives, c
 	let next;
 
 	if (class_directives.length) {
-		next = build_class_directives_object(class_directives, context);
+		next = build_class_directives_object(class_directives, context.state.expressions, context);
 		has_state ||= class_directives.some((d) => d.metadata.expression.has_state);
 
 		if (has_state) {
@@ -235,7 +226,7 @@ export function build_set_class(element, node_id, attribute, class_directives, c
  */
 export function build_set_style(node_id, attribute, style_directives, context) {
 	let { value, has_state } = build_attribute_value(attribute.value, context, (value, metadata) =>
-		metadata.has_call ? get_expression_id(context.state, value) : value
+		metadata.has_call ? get_expression_id(context.state.expressions, value) : value
 	);
 
 	/** @type {Identifier | undefined} */
@@ -248,7 +239,7 @@ export function build_set_style(node_id, attribute, style_directives, context) {
 	let next;
 
 	if (style_directives.length) {
-		next = build_style_directives_object(style_directives, context);
+		next = build_style_directives_object(style_directives, context.state.expressions, context);
 		has_state ||= style_directives.some((d) => d.metadata.expression.has_state);
 
 		if (has_state) {

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

@@ -21,12 +21,12 @@ export function memoize_expression(state, value) {
 }
 
 /**
- *
- * @param {ComponentClientTransformState} state
+ * Pushes `value` into `expressions` and returns a new id
+ * @param {Expression[]} expressions
  * @param {Expression} value
  */
-export function get_expression_id(state, value) {
-	return b.id(`$${state.expressions.push(value) - 1}`);
+export function get_expression_id(expressions, value) {
+	return b.id(`$${expressions.push(value) - 1}`);
 }
 
 /**
@@ -40,7 +40,8 @@ export function build_template_chunk(
 	values,
 	visit,
 	state,
-	memoize = (value, metadata) => (metadata.has_call ? get_expression_id(state, value) : value)
+	memoize = (value, metadata) =>
+		metadata.has_call ? get_expression_id(state.expressions, value) : value
 ) {
 	/** @type {Expression[]} */
 	const expressions = [];