From 156a14c3030b99047fb571056d37f2ef5db97510 Mon Sep 17 00:00:00 2001 From: adiguba Date: Wed, 5 Mar 2025 21:09:42 +0100 Subject: [PATCH 1/5] memoize clsx + directives --- .../client/visitors/RegularElement.js | 43 +++++++++++-------- .../client/visitors/SvelteElement.js | 4 +- .../client/visitors/shared/element.js | 9 ++-- .../src/internal/client/dom/elements/class.js | 2 +- 4 files changed, 35 insertions(+), 23 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 3dd303921369..5cca3f8bb73d 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 @@ -1,5 +1,5 @@ /** @import { Expression, ExpressionStatement, Identifier, MemberExpression, ObjectExpression, Statement } from 'estree' */ -/** @import { AST } from '#compiler' */ +/** @import { AST, ExpressionMetadata } from '#compiler' */ /** @import { SourceLocation } from '#shared' */ /** @import { ComponentClientTransformState, ComponentContext } from '../types' */ /** @import { Scope } from '../../../scope' */ @@ -501,22 +501,23 @@ function setup_select_synchronization(value_binding, context) { /** * @param {AST.ClassDirective[]} class_directives * @param {ComponentContext} context - * @return {ObjectExpression} + * @return {ObjectExpression | Identifier} */ export function build_class_directives_object(class_directives, context) { let properties = []; + let has_call_or_state = false; + for (const d of class_directives) { let expression = /** @type Expression */ (context.visit(d.expression)); - - if (d.metadata.expression.has_call) { - expression = get_expression_id(context.state, expression); - } - properties.push(b.init(d.name, expression)); + has_call_or_state ||= d.metadata.expression.has_call || d.metadata.expression.has_state; } - - return b.object(properties); + let directives = b.object(properties); + if (has_call_or_state) { + return get_expression_id(context.state, directives); + } + return directives; } /** @@ -564,15 +565,21 @@ 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 - ); + /** @type {(value: Expression, metadata: ExpressionMetadata) => Expression} */ + let memoize; + if (is_autofocus) { + // 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 + memoize = (value, metadata) => (metadata.has_call ? memoize_expression(state, value) : value); + } else if (attribute.metadata.needs_clsx && name === 'class') { + // if class needs clsx() we do nothing here ! + // => The entire value will be memoized inside build_set_class() + memoize = (value) => value; + } else { + memoize = (value, metadata) => (metadata.has_call ? get_expression_id(state, value) : value); + } + + let { value, has_state } = build_attribute_value(attribute.value, context, memoize); if (is_autofocus) { state.init.push(b.stmt(b.call('$.autofocus', node_id, 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 3250c2439290..f7d9b32ca64a 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 @@ -90,7 +90,9 @@ export function SvelteElement(node, context) { let { value, has_state } = build_attribute_value( attributes[0].value, context, - (value, metadata) => (metadata.has_call ? get_expression_id(context.state, value) : value) + attributes[0].metadata.needs_clsx + ? (value) => value + : (value, metadata) => (metadata.has_call ? get_expression_id(context.state, value) : value) ); is_attributes_reactive = build_set_class( 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 db8f2e4aa083..eae6d989da72 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 @@ -189,7 +189,7 @@ export function get_attribute_name(element, attribute) { /** * @param {AST.RegularElement | AST.SvelteElement} element * @param {Identifier} node_id - * @param {AST.Attribute | null} attribute + * @param {AST.Attribute} attribute * @param {Expression} value * @param {boolean} has_state * @param {AST.ClassDirective[]} class_directives @@ -207,8 +207,11 @@ export function build_set_class( context, is_html ) { - if (attribute && attribute.metadata.needs_clsx) { + if (attribute.metadata.needs_clsx) { value = b.call('$.clsx', value); + if (has_state) { + value = get_expression_id(context.state, value); + } } /** @type {Identifier | undefined} */ @@ -217,7 +220,7 @@ export function build_set_class( /** @type {ObjectExpression | Identifier | undefined} */ let prev; - /** @type {ObjectExpression | undefined} */ + /** @type {ObjectExpression | Identifier | undefined} */ let next; if (class_directives.length) { diff --git a/packages/svelte/src/internal/client/dom/elements/class.js b/packages/svelte/src/internal/client/dom/elements/class.js index 7027c84f6260..ecbfcbc010fd 100644 --- a/packages/svelte/src/internal/client/dom/elements/class.js +++ b/packages/svelte/src/internal/client/dom/elements/class.js @@ -33,7 +33,7 @@ export function set_class(dom, is_html, value, hash, prev_classes, next_classes) // @ts-expect-error need to add __className to patched prototype dom.__className = value; - } else if (next_classes) { + } else if (next_classes && prev_classes !== next_classes) { for (var key in next_classes) { var is_present = !!next_classes[key]; From 54fe12a61365c4fe8f8cdd8df0ad6687ed87dd60 Mon Sep 17 00:00:00 2001 From: adiguba Date: Wed, 5 Mar 2025 21:10:10 +0100 Subject: [PATCH 2/5] changeset --- .changeset/flat-jars-search.md | 5 +++++ 1 file changed, 5 insertions(+) create mode 100644 .changeset/flat-jars-search.md diff --git a/.changeset/flat-jars-search.md b/.changeset/flat-jars-search.md new file mode 100644 index 000000000000..a372c834abca --- /dev/null +++ b/.changeset/flat-jars-search.md @@ -0,0 +1,5 @@ +--- +'svelte': patch +--- + +chore: memoize clsx() From 93919933ff5266c0a6a39378ca72ab70fa3b9331 Mon Sep 17 00:00:00 2001 From: Rich Harris Date: Wed, 5 Mar 2025 20:47:41 -0500 Subject: [PATCH 3/5] unused --- .../phases/3-transform/client/visitors/RegularElement.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) 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 9beae70c38af..b1ded1b6a5b7 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 @@ -1,5 +1,5 @@ /** @import { ArrayExpression, Expression, ExpressionStatement, Identifier, MemberExpression, ObjectExpression, Statement } from 'estree' */ -/** @import { AST, ExpressionMetadata } from '#compiler' */ +/** @import { AST } from '#compiler' */ /** @import { SourceLocation } from '#shared' */ /** @import { ComponentClientTransformState, ComponentContext } from '../types' */ /** @import { Scope } from '../../../scope' */ From 5ff5bcad5595f00cd2b1af25f7d299adc6c86291 Mon Sep 17 00:00:00 2001 From: Rich Harris Date: Wed, 5 Mar 2025 20:49:15 -0500 Subject: [PATCH 4/5] tweak --- .../3-transform/client/visitors/RegularElement.js | 12 +++++------- 1 file changed, 5 insertions(+), 7 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 b1ded1b6a5b7..9b3ecc922d89 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 @@ -515,19 +515,17 @@ function setup_select_synchronization(value_binding, context) { */ export function build_class_directives_object(class_directives, context) { let properties = []; - let has_call_or_state = false; for (const d of class_directives) { - let expression = /** @type Expression */ (context.visit(d.expression)); + const expression = /** @type Expression */ (context.visit(d.expression)); properties.push(b.init(d.name, expression)); has_call_or_state ||= d.metadata.expression.has_call || d.metadata.expression.has_state; } - let directives = b.object(properties); - if (has_call_or_state) { - return get_expression_id(context.state, directives); - } - return directives; + + const directives = b.object(properties); + + return has_call_or_state ? get_expression_id(context.state, directives) : directives; } /** From 153875c809b63e14c62fd3983e715f1b4c40ed30 Mon Sep 17 00:00:00 2001 From: Rich Harris Date: Wed, 5 Mar 2025 20:52:13 -0500 Subject: [PATCH 5/5] tweak changeset --- .changeset/flat-jars-search.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.changeset/flat-jars-search.md b/.changeset/flat-jars-search.md index a372c834abca..fc0de76f95b4 100644 --- a/.changeset/flat-jars-search.md +++ b/.changeset/flat-jars-search.md @@ -2,4 +2,4 @@ 'svelte': patch --- -chore: memoize clsx() +fix: memoize `clsx` calls