From d427ffd8b96c5a5509ee602bd77d58a90f24e784 Mon Sep 17 00:00:00 2001 From: Rich Harris Date: Tue, 1 Jul 2025 10:37:37 -0400 Subject: [PATCH] chore: encapsulate expression memoization (#16269) * chore: encapsulate expression memoization * add comment * tweak * use b.id --- .../3-transform/client/transform-client.js | 4 +- .../phases/3-transform/client/types.d.ts | 5 +- .../3-transform/client/visitors/Fragment.js | 4 +- .../client/visitors/RegularElement.js | 61 ++++++++++--------- .../client/visitors/SvelteElement.js | 6 +- .../client/visitors/shared/element.js | 37 +++++------ .../client/visitors/shared/utils.js | 52 ++++++++++++---- 7 files changed, 99 insertions(+), 70 deletions(-) diff --git a/packages/svelte/src/compiler/phases/3-transform/client/transform-client.js b/packages/svelte/src/compiler/phases/3-transform/client/transform-client.js index e85a35cf8e..a9c0651e0f 100644 --- a/packages/svelte/src/compiler/phases/3-transform/client/transform-client.js +++ b/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} */ ( diff --git a/packages/svelte/src/compiler/phases/3-transform/client/types.d.ts b/packages/svelte/src/compiler/phases/3-transform/client/types.d.ts index 2388ee1b00..cf5c942268 100644 --- a/packages/svelte/src/compiler/phases/3-transform/client/types.d.ts +++ b/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 */ - readonly expressions: Expression[]; + /** Memoized expressions */ + readonly memoizer: Memoizer; /** The HTML template string */ readonly template: Template; readonly metadata: { diff --git a/packages/svelte/src/compiler/phases/3-transform/client/visitors/Fragment.js b/packages/svelte/src/compiler/phases/3-transform/client/visitors/Fragment.js index 4825184d31..7cd2bd90ed 100644 --- a/packages/svelte/src/compiler/phases/3-transform/client/visitors/Fragment.js +++ b/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: { diff --git a/packages/svelte/src/compiler/phases/3-transform/client/visitors/RegularElement.js b/packages/svelte/src/compiler/phases/3-transform/client/visitors/RegularElement.js index ae8680f594..eed2a75506 100644 --- a/packages/svelte/src/compiler/phases/3-transform/client/visitors/RegularElement.js +++ b/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) => - metadata.has_call ? get_expression_id(context.state.expressions, value) : value + (value, metadata) => (metadata.has_call ? context.state.memoizer.add(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) { - let normal_properties = []; - let important_properties = []; +export function build_style_directives_object( + style_directives, + 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 - ? build_getter({ name: directive.name, type: 'Identifier' }, context.state) - : build_attribute_value(directive.value, context, (value, metadata) => - metadata.has_call ? get_expression_id(expressions, value) : value - ).value; - const property = b.init(directive.name, expression); - - if (directive.modifiers.includes('important')) { - important_properties.push(property); - } else { - normal_properties.push(property); - } + d.value === true + ? build_getter(b.id(d.name), context.state) + : build_attribute_value(d.value, context).value; + + const object = d.modifiers.includes('important') ? important : normal; + object.properties.push(b.init(d.name, expression)); + + has_call_or_state ||= d.metadata.expression.has_call || d.metadata.expression.has_state; } - return important_properties.length - ? b.array([b.object(normal_properties), b.object(important_properties)]) - : b.object(normal_properties); + const directives = important.properties.length ? b.array([normal, important]) : normal; + + 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); diff --git a/packages/svelte/src/compiler/phases/3-transform/client/visitors/SvelteElement.js b/packages/svelte/src/compiler/phases/3-transform/client/visitors/SvelteElement.js index 7381553dbe..6af5f5770e 100644 --- a/packages/svelte/src/compiler/phases/3-transform/client/visitors/SvelteElement.js +++ b/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() } }; diff --git a/packages/svelte/src/compiler/phases/3-transform/client/visitors/shared/element.js b/packages/svelte/src/compiler/phases/3-transform/client/visitors/shared/element.js index 10f942b7d4..d119b0457b 100644 --- a/packages/svelte/src/compiler/phases/3-transform/client/visitors/shared/element.js +++ b/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} attributes @@ -28,18 +28,12 @@ export function build_attribute_effect( /** @type {ObjectExpression['properties']} */ const values = []; - /** @type {Expression[]} */ - const expressions = []; - - /** @param {Expression} value */ - function memoize(value) { - return b.id(`$${expressions.push(value) - 1}`); - } + const memoizer = new Memoizer(); 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( - expressions.map((_, i) => b.id(`$${i}`)), - b.object(values) - ), - expressions.length > 0 && b.array(expressions.map((expression) => b.thunk(expression))), + b.arrow(ids, b.object(values)), + memoizer.sync_values(), 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) { diff --git a/packages/svelte/src/compiler/phases/3-transform/client/visitors/shared/utils.js b/packages/svelte/src/compiler/phases/3-transform/client/visitors/shared/utils.js index 0bd9bfb128..b80466ccc9 100644 --- a/packages/svelte/src/compiler/phases/3-transform/client/visitors/shared/utils.js +++ b/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 - * @param {Expression[]} expressions - * @param {Expression} value + * A utility for extracting complex expressions (such as call expressions) + * from templates and replacing them with `$0`, `$1` etc */ -export function get_expression_id(expressions, value) { - return b.id(`$${expressions.push(value) - 1}`); +export class Memoizer { + /** @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) => - metadata.has_call ? get_expression_id(state.expressions, value) : value + memoize = (value, metadata) => (metadata.has_call ? state.memoizer.add(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 && - b.array(state.expressions.map((expression) => b.thunk(expression))), - state.expressions.length > 0 && !state.analysis.runes && b.id('$.derived_safe_equal') + values, + values && !state.analysis.runes && b.id('$.derived_safe_equal') ) ); }