chore: encapsulate expression memoization (#16269)

* chore: encapsulate expression memoization

* add comment

* tweak

* use b.id

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

@@ -168,9 +168,9 @@ export function client_component(analysis, options) {
 		// these are set inside the `Fragment` visitor, and cannot be used until then
 		init: /** @type {any} */ (null),
 		update: /** @type {any} */ (null),
-		expressions: /** @type {any} */ (null),
 		after_update: /** @type {any} */ (null),
-		template: /** @type {any} */ (null)
+		template: /** @type {any} */ (null),
+		memoizer: /** @type {any} */ (null)
 	};
 
 	const module = /** @type {ESTree.Program} */ (

packages/svelte/src/compiler/phases/3-transform/client/types.d.ts

@@ -12,6 +12,7 @@ import type { AST, Namespace, ValidatedCompileOptions } from '#compiler';
 import type { TransformState } from '../types.js';
 import type { ComponentAnalysis } from '../../types.js';
 import type { Template } from './transform-template/template.js';
+import type { Memoizer } from './visitors/shared/utils.js';
 
 export interface ClientTransformState extends TransformState {
 	/**
@@ -49,8 +50,8 @@ export interface ComponentClientTransformState extends ClientTransformState {
 	readonly update: Statement[];
 	/** Stuff that happens after the render effect (control blocks, dynamic elements, bindings, actions, etc) */
 	readonly after_update: Statement[];
-	/** Expressions used inside the render effect */
+	/** Memoized expressions */
-	readonly expressions: Expression[];
+	readonly memoizer: Memoizer;
 	/** The HTML template string */
 	readonly template: Template;
 	readonly metadata: {

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

@@ -6,7 +6,7 @@ import * as b from '#compiler/builders';
 import { clean_nodes, infer_namespace } from '../../utils.js';
 import { transform_template } from '../transform-template/index.js';
 import { process_children } from './shared/fragment.js';
-import { build_render_statement } from './shared/utils.js';
+import { build_render_statement, Memoizer } from './shared/utils.js';
 import { Template } from '../transform-template/template.js';
 
 /**
@@ -64,8 +64,8 @@ export function Fragment(node, context) {
 		...context.state,
 		init: [],
 		update: [],
-		expressions: [],
 		after_update: [],
+		memoizer: new Memoizer(),
 		template: new Template(),
 		transform: { ...context.state.transform },
 		metadata: {

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

@@ -22,7 +22,7 @@ import {
 	build_set_style
 } from './shared/element.js';
 import { process_children } from './shared/fragment.js';
-import { build_render_statement, build_template_chunk, get_expression_id } from './shared/utils.js';
+import { build_render_statement, build_template_chunk, Memoizer } from './shared/utils.js';
 import { visit_event_attribute } from './shared/events.js';
 
 /**
@@ -253,8 +253,7 @@ export function RegularElement(node, context) {
 				const { value, has_state } = build_attribute_value(
 					attribute.value,
 					context,
-					(value, metadata) =>
+					(value, metadata) => (metadata.has_call ? context.state.memoizer.add(value) : value)
-						metadata.has_call ? get_expression_id(context.state.expressions, value) : value
 				);
 
 				const update = build_element_attribute_update(node, node_id, name, value, attributes);
@@ -455,11 +454,15 @@ function setup_select_synchronization(value_binding, context) {
 
 /**
  * @param {AST.ClassDirective[]} class_directives
- * @param {Expression[]} expressions
  * @param {ComponentContext} context
+ * @param {Memoizer} memoizer
  * @return {ObjectExpression | Identifier}
  */
-export function build_class_directives_object(class_directives, expressions, context) {
+export function build_class_directives_object(
+	class_directives,
+	context,
+	memoizer = context.state.memoizer
+) {
 	let properties = [];
 	let has_call_or_state = false;
 
@@ -471,38 +474,40 @@ export function build_class_directives_object(class_directives, expressions, con
 
 	const directives = b.object(properties);
 
-	return has_call_or_state ? get_expression_id(expressions, directives) : directives;
+	return has_call_or_state ? memoizer.add(directives) : directives;
 }
 
 /**
  * @param {AST.StyleDirective[]} style_directives
- * @param {Expression[]} expressions
  * @param {ComponentContext} context
- * @return {ObjectExpression | ArrayExpression}}
+ * @param {Memoizer} memoizer
+ * @return {ObjectExpression | ArrayExpression | Identifier}}
  */
-export function build_style_directives_object(style_directives, expressions, context) {
+export function build_style_directives_object(
-	let normal_properties = [];
+	style_directives,
-	let important_properties = [];
+	context,
+	memoizer = context.state.memoizer
+) {
+	const normal = b.object([]);
+	const important = b.object([]);
 
-	for (const directive of style_directives) {
+	let has_call_or_state = false;
+
+	for (const d of style_directives) {
 		const expression =
-			directive.value === true
+			d.value === true
-				? build_getter({ name: directive.name, type: 'Identifier' }, context.state)
+				? build_getter(b.id(d.name), context.state)
-				: build_attribute_value(directive.value, context, (value, metadata) =>
+				: build_attribute_value(d.value, context).value;
-						metadata.has_call ? get_expression_id(expressions, value) : value
+
-					).value;
+		const object = d.modifiers.includes('important') ? important : normal;
-		const property = b.init(directive.name, expression);
+		object.properties.push(b.init(d.name, expression));
-
+
-		if (directive.modifiers.includes('important')) {
+		has_call_or_state ||= d.metadata.expression.has_call || d.metadata.expression.has_state;
-			important_properties.push(property);
-		} else {
-			normal_properties.push(property);
-		}
 	}
 
-	return important_properties.length
+	const directives = important.properties.length ? b.array([normal, important]) : normal;
-		? b.array([b.object(normal_properties), b.object(important_properties)])
+
-		: b.object(normal_properties);
+	return has_call_or_state ? memoizer.add(directives) : directives;
 }
 
 /**
@@ -624,7 +629,7 @@ function build_element_special_value_attribute(element, node_id, attribute, cont
 		element === 'select' && attribute.value !== true && !is_text_attribute(attribute);
 
 	const { value, has_state } = build_attribute_value(attribute.value, context, (value, metadata) =>
-		metadata.has_call ? get_expression_id(state.expressions, value) : value
+		metadata.has_call ? state.memoizer.add(value) : value
 	);
 
 	const evaluated = context.state.scope.evaluate(value);

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

@@ -10,7 +10,7 @@ import {
 	build_attribute_effect,
 	build_set_class
 } from './shared/element.js';
-import { build_render_statement } from './shared/utils.js';
+import { build_render_statement, Memoizer } from './shared/utils.js';
 
 /**
  * @param {AST.SvelteElement} node
@@ -46,8 +46,8 @@ export function SvelteElement(node, context) {
 			node: element_id,
 			init: [],
 			update: [],
-			expressions: [],
+			after_update: [],
-			after_update: []
+			memoizer: new Memoizer()
 		}
 	};
 

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

@@ -7,7 +7,7 @@ import { is_ignored } from '../../../../../state.js';
 import { is_event_attribute } from '../../../../../utils/ast.js';
 import * as b from '#compiler/builders';
 import { build_class_directives_object, build_style_directives_object } from '../RegularElement.js';
-import { build_expression, build_template_chunk, get_expression_id } from './utils.js';
+import { build_expression, build_template_chunk, Memoizer } from './utils.js';
 
 /**
  * @param {Array<AST.Attribute | AST.SpreadAttribute>} attributes
@@ -28,18 +28,12 @@ export function build_attribute_effect(
 	/** @type {ObjectExpression['properties']} */
 	const values = [];
 
-	/** @type {Expression[]} */
+	const memoizer = new Memoizer();
-	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 } = build_attribute_value(attribute.value, context, (value, metadata) =>
-				metadata.has_call ? memoize(value) : value
+				metadata.has_call ? memoizer.add(value) : value
 			);
 
 			if (
@@ -57,7 +51,7 @@ export function build_attribute_effect(
 			let value = /** @type {Expression} */ (context.visit(attribute));
 
 			if (attribute.metadata.expression.has_call) {
-				value = memoize(value);
+				value = memoizer.add(value);
 			}
 
 			values.push(b.spread(value));
@@ -69,7 +63,7 @@ export function build_attribute_effect(
 			b.prop(
 				'init',
 				b.array([b.id('$.CLASS')]),
-				build_class_directives_object(class_directives, expressions, context)
+				build_class_directives_object(class_directives, context, memoizer)
 			)
 		);
 	}
@@ -79,21 +73,20 @@ export function build_attribute_effect(
 			b.prop(
 				'init',
 				b.array([b.id('$.STYLE')]),
-				build_style_directives_object(style_directives, expressions, context)
+				build_style_directives_object(style_directives, context, memoizer)
 			)
 		);
 	}
 
+	const ids = memoizer.apply();
+
 	context.state.init.push(
 		b.stmt(
 			b.call(
 				'$.attribute_effect',
 				element_id,
-				b.arrow(
+				b.arrow(ids, b.object(values)),
-					expressions.map((_, i) => b.id(`$${i}`)),
+				memoizer.sync_values(),
-					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),
@@ -158,7 +151,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.expressions, value) : value;
+		return metadata.has_call ? context.state.memoizer.add(value) : value;
 	});
 
 	/** @type {Identifier | undefined} */
@@ -171,7 +164,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.state.expressions, context);
+		next = build_class_directives_object(class_directives, context);
 		has_state ||= class_directives.some((d) => d.metadata.expression.has_state);
 
 		if (has_state) {
@@ -226,7 +219,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.expressions, value) : value
+		metadata.has_call ? context.state.memoizer.add(value) : value
 	);
 
 	/** @type {Identifier | undefined} */
@@ -235,11 +228,11 @@ export function build_set_style(node_id, attribute, style_directives, context) {
 	/** @type {ObjectExpression | Identifier | undefined} */
 	let prev;
 
-	/** @type {ArrayExpression | ObjectExpression | undefined} */
+	/** @type {Expression | undefined} */
 	let next;
 
 	if (style_directives.length) {
-		next = build_style_directives_object(style_directives, context.state.expressions, context);
+		next = build_style_directives_object(style_directives, 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,41 @@ export function memoize_expression(state, value) {
 }
 
 /**
- * Pushes `value` into `expressions` and returns a new id
+ * A utility for extracting complex expressions (such as call expressions)
- * @param {Expression[]} expressions
+ * from templates and replacing them with `$0`, `$1` etc
- * @param {Expression} value
  */
-export function get_expression_id(expressions, value) {
+export class Memoizer {
-	return b.id(`$${expressions.push(value) - 1}`);
+	/** @type {Array<{ id: Identifier, expression: Expression }>} */
+	#sync = [];
+
+	/**
+	 * @param {Expression} expression
+	 */
+	add(expression) {
+		const id = b.id('#'); // filled in later
+
+		this.#sync.push({ id, expression });
+
+		return id;
+	}
+
+	apply() {
+		return this.#sync.map((memo, i) => {
+			memo.id.name = `$${i}`;
+			return memo.id;
+		});
+	}
+
+	deriveds(runes = true) {
+		return this.#sync.map((memo) =>
+			b.let(memo.id, b.call(runes ? '$.derived' : '$.derived_safe_equal', b.thunk(memo.expression)))
+		);
+	}
+
+	sync_values() {
+		if (this.#sync.length === 0) return;
+		return b.array(this.#sync.map((memo) => b.thunk(memo.expression)));
+	}
 }
 
 /**
@@ -40,8 +69,7 @@ export function build_template_chunk(
 	values,
 	context,
 	state = context.state,
-	memoize = (value, metadata) =>
+	memoize = (value, metadata) => (metadata.has_call ? state.memoizer.add(value) : value)
-		metadata.has_call ? get_expression_id(state.expressions, value) : value
 ) {
 	/** @type {Expression[]} */
 	const expressions = [];
@@ -128,18 +156,20 @@ export function build_template_chunk(
  * @param {ComponentClientTransformState} state
  */
 export function build_render_statement(state) {
+	const ids = state.memoizer.apply();
+	const values = state.memoizer.sync_values();
+
 	return b.stmt(
 		b.call(
 			'$.template_effect',
 			b.arrow(
-				state.expressions.map((_, i) => b.id(`$${i}`)),
+				ids,
 				state.update.length === 1 && state.update[0].type === 'ExpressionStatement'
 					? state.update[0].expression
 					: b.block(state.update)
 			),
-			state.expressions.length > 0 &&
+			values,
-				b.array(state.expressions.map((expression) => b.thunk(expression))),
+			values && !state.analysis.runes && b.id('$.derived_safe_equal')
-			state.expressions.length > 0 && !state.analysis.runes && b.id('$.derived_safe_equal')
 		)
 	);
 }