fix: avoid double deriveds in component props

.changeset/tasty-pigs-greet.md

@@ -0,0 +1,5 @@
+---
+'svelte': patch
+---
+
+fix: avoid double deriveds in component props

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

@@ -269,41 +269,6 @@ export function should_proxy(node, scope) {
 	return true;
 }
 
-/**
- * @param {Pattern} node
- * @param {import('zimmerframe').Context<AST.SvelteNode, ComponentClientTransformState>} context
- * @returns {{ id: Pattern, declarations: null | Statement[] }}
- */
-export function create_derived_block_argument(node, context) {
-	if (node.type === 'Identifier') {
-		context.state.transform[node.name] = { read: get_value };
-		return { id: node, declarations: null };
-	}
-
-	const pattern = /** @type {Pattern} */ (context.visit(node));
-	const identifiers = extract_identifiers(node);
-
-	const id = b.id('$$source');
-	const value = b.id('$$value');
-
-	const block = b.block([
-		b.var(pattern, b.call('$.get', id)),
-		b.return(b.object(identifiers.map((identifier) => b.prop('init', identifier, identifier))))
-	]);
-
-	const declarations = [b.var(value, create_derived(context.state, b.thunk(block)))];
-
-	for (const id of identifiers) {
-		context.state.transform[id.name] = { read: get_value };
-
-		declarations.push(
-			b.var(id, create_derived(context.state, b.thunk(b.member(b.call('$.get', value), id))))
-		);
-	}
-
-	return { id, declarations };
-}
-
 /**
  * Svelte legacy mode should use safe equals in most places, runes mode shouldn't
  * @param {ComponentClientTransformState} state

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

@@ -1,8 +1,10 @@
 /** @import { BlockStatement, Expression, Pattern, Statement } from 'estree' */
 /** @import { AST } from '#compiler' */
-/** @import { ComponentContext } from '../types' */
+/** @import { ComponentClientTransformState, ComponentContext } from '../types' */
+import { extract_identifiers } from '../../../../utils/ast.js';
 import * as b from '../../../../utils/builders.js';
-import { create_derived_block_argument } from '../utils.js';
+import { create_derived } from '../utils.js';
+import { get_value } from './shared/declarations.js';
 
 /**
  * @param {AST.AwaitBlock} node
@@ -65,3 +67,38 @@ export function AwaitBlock(node, context) {
 		)
 	);
 }
+
+/**
+ * @param {Pattern} node
+ * @param {import('zimmerframe').Context<AST.SvelteNode, ComponentClientTransformState>} context
+ * @returns {{ id: Pattern, declarations: null | Statement[] }}
+ */
+function create_derived_block_argument(node, context) {
+	if (node.type === 'Identifier') {
+		context.state.transform[node.name] = { read: get_value };
+		return { id: node, declarations: null };
+	}
+
+	const pattern = /** @type {Pattern} */ (context.visit(node));
+	const identifiers = extract_identifiers(node);
+
+	const id = b.id('$$source');
+	const value = b.id('$$value');
+
+	const block = b.block([
+		b.var(pattern, b.call('$.get', id)),
+		b.return(b.object(identifiers.map((identifier) => b.prop('init', identifier, identifier))))
+	]);
+
+	const declarations = [b.var(value, create_derived(context.state, b.thunk(block)))];
+
+	for (const id of identifiers) {
+		context.state.transform[id.name] = { read: get_value };
+
+		declarations.push(
+			b.var(id, create_derived(context.state, b.thunk(b.member(b.call('$.get', value), id))))
+		);
+	}
+
+	return { id, declarations };
+}

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

@@ -537,8 +537,8 @@ function build_element_attribute_update_assignment(
 	const is_svg = context.state.metadata.namespace === 'svg' || element.name === 'svg';
 	const is_mathml = context.state.metadata.namespace === 'mathml';
 
-	let { value, has_state } = build_attribute_value(attribute.value, context, (value) =>
-		get_expression_id(state, value)
+	let { value, has_state } = build_attribute_value(attribute.value, context, (value, metadata) =>
+		metadata.has_call ? get_expression_id(state, value) : value
 	);
 
 	if (name === 'autofocus') {
@@ -665,8 +665,8 @@ function build_custom_element_attribute_update_assignment(node_id, attribute, co
  */
 function build_element_special_value_attribute(element, node_id, attribute, context) {
 	const state = context.state;
-	const { value, has_state } = build_attribute_value(attribute.value, context, (value) =>
-		get_expression_id(state, value)
+	const { value, has_state } = build_attribute_value(attribute.value, context, (value, metadata) =>
+		metadata.has_call ? get_expression_id(state, value) : value
 	);
 
 	const inner_assignment = b.assignment(

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

@@ -30,8 +30,10 @@ export function SlotElement(node, context) {
 		if (attribute.type === 'SpreadAttribute') {
 			spreads.push(b.thunk(/** @type {Expression} */ (context.visit(attribute))));
 		} else if (attribute.type === 'Attribute') {
-			const { value, has_state } = build_attribute_value(attribute.value, context, (value) =>
-				memoize_expression(context.state, value)
+			const { value, has_state } = build_attribute_value(
+				attribute.value,
+				context,
+				(value, metadata) => (metadata.has_call ? memoize_expression(context.state, value) : value)
 			);
 
 			if (attribute.name === 'name') {

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

@@ -4,7 +4,6 @@
 import { dev, is_ignored } from '../../../../../state.js';
 import { get_attribute_chunks, object } from '../../../../../utils/ast.js';
 import * as b from '../../../../../utils/builders.js';
-import { create_derived } from '../../utils.js';
 import { build_bind_this, memoize_expression, validate_binding } from '../shared/utils.js';
 import { build_attribute_value } from '../shared/element.js';
 import { build_event_handler } from './events.js';
@@ -134,9 +133,9 @@ export function build_component(node, component_name, context, anchor = context.
 				custom_css_props.push(
 					b.init(
 						attribute.name,
-						build_attribute_value(attribute.value, context, (value) =>
+						build_attribute_value(attribute.value, context, (value, metadata) =>
 							// TODO put the derived in the local block
-							memoize_expression(context.state, value)
+							metadata.has_call ? memoize_expression(context.state, value) : value
 						).value
 					)
 				);
@@ -151,31 +150,29 @@ export function build_component(node, component_name, context, anchor = context.
 				has_children_prop = true;
 			}
 
-			const { value, has_state } = build_attribute_value(attribute.value, context, (value) =>
-				memoize_expression(context.state, value)
-			);
-
-			if (has_state) {
-				let arg = value;
-
-				// When we have a non-simple computation, anything other than an Identifier or Member expression,
-				// then there's a good chance it needs to be memoized to avoid over-firing when read within the
-				// child component.
-				const should_wrap_in_derived = get_attribute_chunks(attribute.value).some((n) => {
-					return (
-						n.type === 'ExpressionTag' &&
-						n.expression.type !== 'Identifier' &&
-						n.expression.type !== 'MemberExpression'
-					);
-				});
+			const { value, has_state } = build_attribute_value(
+				attribute.value,
+				context,
+				(value, metadata) => {
+					if (!metadata.has_state) return value;
+
+					// When we have a non-simple computation, anything other than an Identifier or Member expression,
+					// then there's a good chance it needs to be memoized to avoid over-firing when read within the
+					// child component (e.g. `active={i === index}`)
+					const should_wrap_in_derived = get_attribute_chunks(attribute.value).some((n) => {
+						return (
+							n.type === 'ExpressionTag' &&
+							n.expression.type !== 'Identifier' &&
+							n.expression.type !== 'MemberExpression'
+						);
+					});
 
-				if (should_wrap_in_derived) {
-					const id = b.id(context.state.scope.generate(attribute.name));
-					context.state.init.push(b.var(id, create_derived(context.state, b.thunk(value))));
-					arg = b.call('$.get', id);
+					return should_wrap_in_derived ? memoize_expression(context.state, value) : value;
 				}
+			);
 
-				push_prop(b.get(attribute.name, [b.return(arg)]));
+			if (has_state) {
+				push_prop(b.get(attribute.name, [b.return(value)]));
 			} else {
 				push_prop(b.init(attribute.name, value));
 			}

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

@@ -1,11 +1,11 @@
 /** @import { Expression, Identifier, ObjectExpression } from 'estree' */
-/** @import { AST } from '#compiler' */
+/** @import { AST, ExpressionMetadata } from '#compiler' */
 /** @import { ComponentClientTransformState, ComponentContext } from '../../types' */
 import { normalize_attribute } from '../../../../../../utils.js';
 import { is_ignored } from '../../../../../state.js';
 import { is_event_attribute } from '../../../../../utils/ast.js';
 import * as b from '../../../../../utils/builders.js';
-import { build_getter, create_derived } from '../../utils.js';
+import { build_getter } from '../../utils.js';
 import { build_template_chunk, get_expression_id } from './utils.js';
 
 /**
@@ -35,8 +35,10 @@ export function build_set_attributes(
 
 	for (const attribute of attributes) {
 		if (attribute.type === 'Attribute') {
-			const { value, has_state } = build_attribute_value(attribute.value, context, (value) =>
-				get_expression_id(context.state, value)
+			const { value, has_state } = build_attribute_value(
+				attribute.value,
+				context,
+				(value, metadata) => (metadata.has_call ? get_expression_id(context.state, value) : value)
 			);
 
 			if (
@@ -59,10 +61,9 @@ export function build_set_attributes(
 			let value = /** @type {Expression} */ (context.visit(attribute));
 
 			if (attribute.metadata.expression.has_call) {
-				const id = b.id(state.scope.generate('spread_with_call'));
-				state.init.push(b.const(id, create_derived(state, b.thunk(value))));
-				value = b.call('$.get', id);
+				value = get_expression_id(context.state, value);
 			}
+
 			values.push(b.spread(value));
 		}
 	}
@@ -111,8 +112,8 @@ export function build_style_directives(
 		let value =
 			directive.value === true
 				? build_getter({ name: directive.name, type: 'Identifier' }, context.state)
-				: build_attribute_value(directive.value, context, (value) =>
-						get_expression_id(context.state, value)
+				: build_attribute_value(directive.value, context, (value, metadata) =>
+						metadata.has_call ? get_expression_id(context.state, value) : value
 					).value;
 
 		const update = b.stmt(
@@ -169,7 +170,7 @@ export function build_class_directives(
 /**
  * @param {AST.Attribute['value']} value
  * @param {ComponentContext} context
- * @param {(value: Expression) => Expression} memoize
+ * @param {(value: Expression, metadata: ExpressionMetadata) => Expression} memoize
  * @returns {{ value: Expression, has_state: boolean }}
  */
 export function build_attribute_value(value, context, memoize = (value) => value) {
@@ -187,7 +188,7 @@ export function build_attribute_value(value, context, memoize = (value) => value
 		let expression = /** @type {Expression} */ (context.visit(chunk.expression));
 
 		return {
-			value: chunk.metadata.expression.has_call ? memoize(expression) : expression,
+			value: memoize(expression, chunk.metadata.expression),
 			has_state: chunk.metadata.expression.has_state
 		};
 	}

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

@@ -1,5 +1,5 @@
 /** @import { Expression, ExpressionStatement, Identifier, MemberExpression, SequenceExpression, Statement, Super } from 'estree' */
-/** @import { AST } from '#compiler' */
+/** @import { AST, ExpressionMetadata } from '#compiler' */
 /** @import { ComponentClientTransformState } from '../../types' */
 import { walk } from 'zimmerframe';
 import { object } from '../../../../../utils/ast.js';
@@ -79,14 +79,14 @@ function compare_expressions(a, b) {
  * @param {Array<AST.Text | AST.ExpressionTag>} values
  * @param {(node: AST.SvelteNode, state: any) => any} visit
  * @param {ComponentClientTransformState} state
- * @param {(value: Expression) => Expression} memoize
+ * @param {(value: Expression, metadata: ExpressionMetadata) => Expression} memoize
  * @returns {{ value: Expression, has_state: boolean }}
  */
 export function build_template_chunk(
 	values,
 	visit,
 	state,
-	memoize = (value) => get_expression_id(state, value)
+	memoize = (value, metadata) => (metadata.has_call ? get_expression_id(state, value) : value)
 ) {
 	/** @type {Expression[]} */
 	const expressions = [];
@@ -106,14 +106,13 @@ export function build_template_chunk(
 				quasi.value.cooked += node.expression.value + '';
 			}
 		} else {
-			let value = /** @type {Expression} */ (visit(node.expression, state));
+			let value = memoize(
+				/** @type {Expression} */ (visit(node.expression, state)),
+				node.metadata.expression
+			);
 
 			has_state ||= node.metadata.expression.has_state;
 
-			if (node.metadata.expression.has_call) {
-				value = memoize(value);
-			}
-
 			if (values.length === 1) {
 				// If we have a single expression, then pass that in directly to possibly avoid doing
 				// extra work in the template_effect (instead we do the work in set_text).