From b6a5065d5e670cd238864f35bab50421ce8c2163 Mon Sep 17 00:00:00 2001 From: adiguba Date: Tue, 4 Mar 2025 21:28:54 +0100 Subject: [PATCH 1/2] memoize clsx --- .../client/visitors/RegularElement.js | 20 +++++++++++-------- .../client/visitors/SvelteElement.js | 3 ++- .../client/visitors/shared/element.js | 14 +++++++++---- .../client/visitors/shared/utils.js | 8 +++++--- 4 files changed, 29 insertions(+), 16 deletions(-) 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 434b49caa1e6..132244caf371 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 @@ -573,14 +573,17 @@ function build_element_attribute_update_assignment( const is_autofocus = name === 'autofocus'; - let { value, has_state } = build_attribute_value(attribute.value, context, (value, metadata) => - metadata.has_call - ? // if it's autofocus we will not add this to a template effect so we don't want to get the expression id - // but separately memoize the expression - is_autofocus - ? memoize_expression(state, value) - : get_expression_id(state, value) - : value + let { value, has_state, has_call } = build_attribute_value( + attribute.value, + context, + (value, metadata) => + metadata.has_call + ? // if it's autofocus we will not add this to a template effect so we don't want to get the expression id + // but separately memoize the expression + is_autofocus + ? memoize_expression(state, value) + : get_expression_id(state, value) + : value ); if (is_autofocus) { @@ -608,6 +611,7 @@ function build_element_attribute_update_assignment( attribute, value, has_state, + has_call, class_directives, context, !is_svg && !is_mathml 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 ac284c818d3d..a665c5b5beb3 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 @@ -87,7 +87,7 @@ export function SvelteElement(node, context) { is_text_attribute(attributes[0]) ) { // special case when there only a class attribute, without call expression - let { value, has_state } = build_attribute_value( + let { value, has_state, has_call } = build_attribute_value( attributes[0].value, context, (value, metadata) => (metadata.has_call ? get_expression_id(context.state, value) : value) @@ -99,6 +99,7 @@ export function SvelteElement(node, context) { attributes[0], value, has_state, + has_call, class_directives, inner_context, false 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 81a4b45288eb..dc6e5f752b85 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 @@ -155,25 +155,26 @@ export function build_style_directives( * @param {AST.Attribute['value']} value * @param {ComponentContext} context * @param {(value: Expression, metadata: ExpressionMetadata) => Expression} memoize - * @returns {{ value: Expression, has_state: boolean }} + * @returns {{ value: Expression, has_state: boolean, has_call: boolean }} */ export function build_attribute_value(value, context, memoize = (value) => value) { if (value === true) { - return { value: b.literal(true), has_state: false }; + return { value: b.literal(true), has_state: false, has_call: false }; } if (!Array.isArray(value) || value.length === 1) { const chunk = Array.isArray(value) ? value[0] : value; if (chunk.type === 'Text') { - return { value: b.literal(chunk.data), has_state: false }; + return { value: b.literal(chunk.data), has_state: false, has_call: false }; } let expression = /** @type {Expression} */ (context.visit(chunk.expression)); return { value: memoize(expression, chunk.metadata.expression), - has_state: chunk.metadata.expression.has_state + has_state: chunk.metadata.expression.has_state, + has_call: chunk.metadata.expression.has_call }; } @@ -198,6 +199,7 @@ export function get_attribute_name(element, attribute) { * @param {AST.Attribute | null} attribute * @param {Expression} value * @param {boolean} has_state + * @param {boolean} has_call * @param {AST.ClassDirective[]} class_directives * @param {ComponentContext} context * @param {boolean} is_html @@ -209,12 +211,16 @@ export function build_set_class( attribute, value, has_state, + has_call, class_directives, context, is_html ) { if (attribute && attribute.metadata.needs_clsx) { value = b.call('$.clsx', value); + if (has_state && !has_call) { + value = get_expression_id(context.state, value); + } } /** @type {Identifier | undefined} */ 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 c25ef3ab50e3..cb557e14f8db 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 @@ -80,7 +80,7 @@ function compare_expressions(a, b) { * @param {(node: AST.SvelteNode, state: any) => any} visit * @param {ComponentClientTransformState} state * @param {(value: Expression, metadata: ExpressionMetadata) => Expression} memoize - * @returns {{ value: Expression, has_state: boolean }} + * @returns {{ value: Expression, has_state: boolean, has_call: boolean }} */ export function build_template_chunk( values, @@ -95,6 +95,7 @@ export function build_template_chunk( const quasis = [quasi]; let has_state = false; + let has_call = false; for (let i = 0; i < values.length; i++) { const node = values[i]; @@ -116,11 +117,12 @@ export function build_template_chunk( ); has_state ||= node.metadata.expression.has_state; + has_call ||= node.metadata.expression.has_call; 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). - return { value, has_state }; + return { value, has_state, has_call }; } if ( @@ -162,7 +164,7 @@ export function build_template_chunk( ? b.template(quasis, expressions) : b.literal(/** @type {string} */ (quasi.value.cooked)); - return { value, has_state }; + return { value, has_state, has_call }; } /** From ca3e432ec06134056e24cf005adc9364755c6f9b Mon Sep 17 00:00:00 2001 From: Rich Harris Date: Tue, 4 Mar 2025 16:59:38 -0500 Subject: [PATCH 2/2] changeset --- .changeset/tame-trees-deliver.md | 5 +++++ 1 file changed, 5 insertions(+) create mode 100644 .changeset/tame-trees-deliver.md diff --git a/.changeset/tame-trees-deliver.md b/.changeset/tame-trees-deliver.md new file mode 100644 index 000000000000..e48ba3c91bfb --- /dev/null +++ b/.changeset/tame-trees-deliver.md @@ -0,0 +1,5 @@ +--- +"svelte": patch +--- + +chore: memoize `clsx` calls