From 2ff56dd676324eb05efa4602fde3a40bbee37b0d Mon Sep 17 00:00:00 2001 From: Rich Harris Date: Fri, 17 Jan 2025 14:24:46 -0500 Subject: [PATCH 01/22] WIP --- .../phases/3-transform/client/visitors/shared/utils.js | 8 ++------ .../_expected/client/index.svelte.js | 5 ++++- .../each-string-template/_expected/client/index.svelte.js | 5 ++++- .../_expected/client/index.svelte.js | 5 ++++- .../skip-static-subtree/_expected/client/index.svelte.js | 6 +++++- 5 files changed, 19 insertions(+), 10 deletions(-) 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 69fd8832143f..8cfbdf9efda7 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 @@ -25,13 +25,11 @@ export function build_template_chunk(values, visit, state) { let has_call = false; let has_state = false; - let contains_multiple_call_expression = false; for (const node of values) { if (node.type === 'ExpressionTag') { const metadata = node.metadata.expression; - contains_multiple_call_expression ||= has_call && metadata.has_call; has_call ||= metadata.has_call; has_state ||= metadata.has_state; } @@ -47,7 +45,7 @@ export function build_template_chunk(values, visit, state) { quasi.value.cooked += node.expression.value + ''; } } else { - if (node.metadata.expression.has_call && contains_multiple_call_expression) { + if (node.metadata.expression.has_call) { const id = b.id(state.scope.generate('expression')); state.init.push( b.const( @@ -101,9 +99,7 @@ export function build_update(statement) { * @param {Statement[]} update */ export function build_render_statement(update) { - return update.length === 1 - ? build_update(update[0]) - : b.stmt(b.call('$.template_effect', b.thunk(b.block(update)))); + return b.stmt(b.call('$.template_effect', b.thunk(b.block(update)))); } /** diff --git a/packages/svelte/tests/snapshot/samples/bind-component-snippet/_expected/client/index.svelte.js b/packages/svelte/tests/snapshot/samples/bind-component-snippet/_expected/client/index.svelte.js index fa990b33ee56..a46847da24af 100644 --- a/packages/svelte/tests/snapshot/samples/bind-component-snippet/_expected/client/index.svelte.js +++ b/packages/svelte/tests/snapshot/samples/bind-component-snippet/_expected/client/index.svelte.js @@ -29,6 +29,9 @@ export default function Bind_component_snippet($$anchor) { var text_1 = $.sibling(node); - $.template_effect(() => $.set_text(text_1, ` value: ${$.get(value) ?? ''}`)); + $.template_effect(() => { + $.set_text(text_1, ` value: ${$.get(value) ?? ''}`); + }); + $.append($$anchor, fragment); } \ No newline at end of file diff --git a/packages/svelte/tests/snapshot/samples/each-string-template/_expected/client/index.svelte.js b/packages/svelte/tests/snapshot/samples/each-string-template/_expected/client/index.svelte.js index c0626bd416c9..7a0db9f44a8b 100644 --- a/packages/svelte/tests/snapshot/samples/each-string-template/_expected/client/index.svelte.js +++ b/packages/svelte/tests/snapshot/samples/each-string-template/_expected/client/index.svelte.js @@ -11,7 +11,10 @@ export default function Each_string_template($$anchor) { var text = $.text(); - $.template_effect(() => $.set_text(text, `${thing ?? ''}, `)); + $.template_effect(() => { + $.set_text(text, `${thing ?? ''}, `); + }); + $.append($$anchor, text); }); diff --git a/packages/svelte/tests/snapshot/samples/function-prop-no-getter/_expected/client/index.svelte.js b/packages/svelte/tests/snapshot/samples/function-prop-no-getter/_expected/client/index.svelte.js index c545608bcacf..f413b5992fe6 100644 --- a/packages/svelte/tests/snapshot/samples/function-prop-no-getter/_expected/client/index.svelte.js +++ b/packages/svelte/tests/snapshot/samples/function-prop-no-getter/_expected/client/index.svelte.js @@ -19,7 +19,10 @@ export default function Function_prop_no_getter($$anchor) { var text = $.text(); - $.template_effect(() => $.set_text(text, `clicks: ${$.get(count) ?? ''}`)); + $.template_effect(() => { + $.set_text(text, `clicks: ${$.get(count) ?? ''}`); + }); + $.append($$anchor, text); }, $$slots: { default: true } diff --git a/packages/svelte/tests/snapshot/samples/skip-static-subtree/_expected/client/index.svelte.js b/packages/svelte/tests/snapshot/samples/skip-static-subtree/_expected/client/index.svelte.js index 9b203b97e82d..089b828c892b 100644 --- a/packages/svelte/tests/snapshot/samples/skip-static-subtree/_expected/client/index.svelte.js +++ b/packages/svelte/tests/snapshot/samples/skip-static-subtree/_expected/client/index.svelte.js @@ -46,7 +46,11 @@ export default function Skip_static_subtree($$anchor, $$props) { var img_1 = $.child(div_2); $.reset(div_2); - $.template_effect(() => $.set_text(text, $$props.title)); + + $.template_effect(() => { + $.set_text(text, $$props.title); + }); + $.handle_lazy_img(img); $.handle_lazy_img(img_1); $.append($$anchor, fragment); From 3d1daea75d051564ad9bb1ebfde5e2ed24f973a9 Mon Sep 17 00:00:00 2001 From: Rich Harris Date: Fri, 17 Jan 2025 14:25:10 -0500 Subject: [PATCH 02/22] drive-by --- .../3-transform/client/visitors/SvelteElement.js | 12 ++++-------- 1 file changed, 4 insertions(+), 8 deletions(-) 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 ba66fe29d691..6dad4bdfe198 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 @@ -1,12 +1,8 @@ -/** @import { BlockStatement, Expression, ExpressionStatement, Identifier, ObjectExpression, Statement } from 'estree' */ +/** @import { BlockStatement, Expression, ExpressionStatement, Statement } from 'estree' */ /** @import { AST } from '#compiler' */ /** @import { ComponentContext } from '../types' */ -import { dev, is_ignored, locator } from '../../../../state.js'; -import { - get_attribute_expression, - is_event_attribute, - is_text_attribute -} from '../../../../utils/ast.js'; +import { dev, locator } from '../../../../state.js'; +import { is_text_attribute } from '../../../../utils/ast.js'; import * as b from '../../../../utils/builders.js'; import { determine_namespace_for_children } from '../../utils.js'; import { @@ -15,7 +11,7 @@ import { build_set_attributes, build_style_directives } from './shared/element.js'; -import { build_render_statement, build_update } from './shared/utils.js'; +import { build_render_statement } from './shared/utils.js'; /** * @param {AST.SvelteElement} node From dff056115962a19279f13fb1f829b27bf99e50b8 Mon Sep 17 00:00:00 2001 From: Rich Harris Date: Fri, 17 Jan 2025 14:35:48 -0500 Subject: [PATCH 03/22] group attribute updates --- .../client/visitors/RegularElement.js | 6 +----- .../client/visitors/shared/element.js | 16 +++++++++++++++- 2 files changed, 16 insertions(+), 6 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 ffd06dfd866f..43ac96ef97e3 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 @@ -626,11 +626,7 @@ function build_element_attribute_update_assignment( } if (attribute.metadata.expression.has_state) { - if (has_call) { - state.init.push(build_update(update)); - } else { - state.update.push(update); - } + state.update.push(update); return true; } else { state.init.push(update); 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 1b0737e31e18..7fd323b6ec08 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 @@ -193,10 +193,24 @@ export function build_attribute_value(value, context) { return { has_state: false, has_call: false, value: b.literal(chunk.data) }; } + let expression = /** @type {Expression} */ (context.visit(chunk.expression)); + + if (chunk.metadata.expression.has_call) { + // TODO this is temporary + const id = b.id(context.state.scope.generate('expression')); + context.state.init.push( + b.const( + id, + create_derived(context.state, b.thunk(b.logical('??', expression, b.literal('')))) + ) + ); + expression = b.call('$.get', id); + } + return { has_state: chunk.metadata.expression.has_state, has_call: chunk.metadata.expression.has_call, - value: /** @type {Expression} */ (context.visit(chunk.expression)) + value: expression }; } From 30f075761af36842d2d01544109ae9f85cdc230c Mon Sep 17 00:00:00 2001 From: Rich Harris Date: Fri, 17 Jan 2025 14:42:21 -0500 Subject: [PATCH 04/22] fix --- .../phases/2-analyze/visitors/shared/function.js | 3 ++- .../_expected/client/main.svelte.js | 13 ++++++------- 2 files changed, 8 insertions(+), 8 deletions(-) diff --git a/packages/svelte/src/compiler/phases/2-analyze/visitors/shared/function.js b/packages/svelte/src/compiler/phases/2-analyze/visitors/shared/function.js index c6151992bfd0..c892efd421d1 100644 --- a/packages/svelte/src/compiler/phases/2-analyze/visitors/shared/function.js +++ b/packages/svelte/src/compiler/phases/2-analyze/visitors/shared/function.js @@ -15,6 +15,7 @@ export function visit_function(node, context) { context.next({ ...context.state, - function_depth: context.state.function_depth + 1 + function_depth: context.state.function_depth + 1, + expression: null }); } diff --git a/packages/svelte/tests/snapshot/samples/dynamic-attributes-casing/_expected/client/main.svelte.js b/packages/svelte/tests/snapshot/samples/dynamic-attributes-casing/_expected/client/main.svelte.js index ce77a27e190c..d5935c4fbb3c 100644 --- a/packages/svelte/tests/snapshot/samples/dynamic-attributes-casing/_expected/client/main.svelte.js +++ b/packages/svelte/tests/snapshot/samples/dynamic-attributes-casing/_expected/client/main.svelte.js @@ -12,21 +12,20 @@ export default function Main($$anchor) { var svg = $.sibling(div, 2); var custom_element = $.sibling(svg, 2); var div_1 = $.sibling(custom_element, 2); - - $.template_effect(() => $.set_attribute(div_1, 'foobar', y())); - + const expression = $.derived(() => y() ?? ''); var svg_1 = $.sibling(div_1, 2); - - $.template_effect(() => $.set_attribute(svg_1, 'viewBox', y())); - + const expression_1 = $.derived(() => y() ?? ''); var custom_element_1 = $.sibling(svg_1, 2); + const expression_2 = $.derived(() => y() ?? ''); - $.template_effect(() => $.set_custom_element_data(custom_element_1, 'fooBar', y())); + $.template_effect(() => $.set_custom_element_data(custom_element_1, 'fooBar', $.get(expression_2))); $.template_effect(() => { $.set_attribute(div, 'foobar', x); $.set_attribute(svg, 'viewBox', x); $.set_custom_element_data(custom_element, 'fooBar', x); + $.set_attribute(div_1, 'foobar', $.get(expression)); + $.set_attribute(svg_1, 'viewBox', $.get(expression_1)); }); $.append($$anchor, fragment); From 9becdf584f2e5a35f14c7c54e2f7f2caeca9e3b3 Mon Sep 17 00:00:00 2001 From: Rich Harris Date: Fri, 17 Jan 2025 15:03:44 -0500 Subject: [PATCH 05/22] more --- .../phases/3-transform/client/visitors/shared/fragment.js | 6 ++---- .../samples/lifecycle-render-order-for-children/_config.js | 4 ++-- .../lifecycle-render-order-for-children-2/_config.js | 4 ++-- .../lifecycle-render-order-for-children-3/_config.js | 4 ++-- .../lifecycle-render-order-for-children-4/_config.js | 4 ++-- .../samples/lifecycle-render-order-for-children/_config.js | 4 ++-- .../text-nodes-deriveds/_expected/client/index.svelte.js | 6 +++++- 7 files changed, 17 insertions(+), 15 deletions(-) diff --git a/packages/svelte/src/compiler/phases/3-transform/client/visitors/shared/fragment.js b/packages/svelte/src/compiler/phases/3-transform/client/visitors/shared/fragment.js index 7674fd1eb234..0191a3879277 100644 --- a/packages/svelte/src/compiler/phases/3-transform/client/visitors/shared/fragment.js +++ b/packages/svelte/src/compiler/phases/3-transform/client/visitors/shared/fragment.js @@ -69,7 +69,7 @@ export function process_children(nodes, initial, is_element, { visit, state }) { state.template.push(' '); - const { has_state, has_call, value } = build_template_chunk(sequence, visit, state); + const { has_state, value } = build_template_chunk(sequence, visit, state); // if this is a standalone `{expression}`, make sure we handle the case where // no text node was created because the expression was empty during SSR @@ -78,9 +78,7 @@ export function process_children(nodes, initial, is_element, { visit, state }) { const update = b.stmt(b.call('$.set_text', id, value)); - if (has_call && !within_bound_contenteditable) { - state.init.push(build_update(update)); - } else if (has_state && !within_bound_contenteditable) { + if (has_state && !within_bound_contenteditable) { state.update.push(update); } else { state.init.push(b.stmt(b.assignment('=', b.member(id, 'nodeValue'), value))); diff --git a/packages/svelte/tests/runtime-legacy/samples/lifecycle-render-order-for-children/_config.js b/packages/svelte/tests/runtime-legacy/samples/lifecycle-render-order-for-children/_config.js index 73bfd09ceb70..99f9681c4ba5 100644 --- a/packages/svelte/tests/runtime-legacy/samples/lifecycle-render-order-for-children/_config.js +++ b/packages/svelte/tests/runtime-legacy/samples/lifecycle-render-order-for-children/_config.js @@ -16,13 +16,13 @@ export default test({ test({ assert, compileOptions, component }) { assert.deepEqual(order, [ 'parent: beforeUpdate 0', - 'parent: render 0', '1: beforeUpdate 0', '1: render 0', '2: beforeUpdate 0', '2: render 0', '3: beforeUpdate 0', '3: render 0', + 'parent: render 0', '1: onMount 0', '1: afterUpdate 0', '2: onMount 0', @@ -39,13 +39,13 @@ export default test({ assert.deepEqual(order, [ 'parent: beforeUpdate 1', - 'parent: render 1', '1: beforeUpdate 1', '1: render 1', '2: beforeUpdate 1', '2: render 1', '3: beforeUpdate 1', '3: render 1', + 'parent: render 1', '1: afterUpdate 1', '2: afterUpdate 1', '3: afterUpdate 1', diff --git a/packages/svelte/tests/runtime-runes/samples/lifecycle-render-order-for-children-2/_config.js b/packages/svelte/tests/runtime-runes/samples/lifecycle-render-order-for-children-2/_config.js index 1066d9a2df19..cb8e6486457d 100644 --- a/packages/svelte/tests/runtime-runes/samples/lifecycle-render-order-for-children-2/_config.js +++ b/packages/svelte/tests/runtime-runes/samples/lifecycle-render-order-for-children-2/_config.js @@ -10,7 +10,6 @@ export default test({ assert.deepEqual(logs, [ 'parent: $effect.pre 0', 'parent: $effect.pre (2) 0', - 'parent: render 0', '1: $effect.pre 0', '1: $effect.pre (2) 0', '1: render 0', @@ -20,6 +19,7 @@ export default test({ '3: $effect.pre 0', '3: $effect.pre (2) 0', '3: render 0', + 'parent: render 0', '1: $effect 0', '2: $effect 0', '3: $effect 0', @@ -33,7 +33,6 @@ export default test({ assert.deepEqual(logs, [ 'parent: $effect.pre 1', 'parent: $effect.pre (2) 1', - 'parent: render 1', '1: $effect.pre 1', '1: $effect.pre (2) 1', '1: render 1', @@ -43,6 +42,7 @@ export default test({ '3: $effect.pre 1', '3: $effect.pre (2) 1', '3: render 1', + 'parent: render 1', '1: $effect 1', '2: $effect 1', '3: $effect 1', diff --git a/packages/svelte/tests/runtime-runes/samples/lifecycle-render-order-for-children-3/_config.js b/packages/svelte/tests/runtime-runes/samples/lifecycle-render-order-for-children-3/_config.js index 55847c35a2a3..6c063bcb3e45 100644 --- a/packages/svelte/tests/runtime-runes/samples/lifecycle-render-order-for-children-3/_config.js +++ b/packages/svelte/tests/runtime-runes/samples/lifecycle-render-order-for-children-3/_config.js @@ -8,13 +8,13 @@ export default test({ async test({ assert, component, logs }) { assert.deepEqual(logs, [ - 'parent: render 0', '1: $effect.pre 0', '1: render 0', '2: $effect.pre 0', '2: render 0', '3: $effect.pre 0', '3: render 0', + 'parent: render 0', '1: $effect 0', '2: $effect 0', '3: $effect 0', @@ -26,13 +26,13 @@ export default test({ flushSync(() => (component.n += 1)); assert.deepEqual(logs, [ - 'parent: render 1', '1: $effect.pre 1', '1: render 1', '2: $effect.pre 1', '2: render 1', '3: $effect.pre 1', '3: render 1', + 'parent: render 1', '1: $effect 1', '2: $effect 1', '3: $effect 1', diff --git a/packages/svelte/tests/runtime-runes/samples/lifecycle-render-order-for-children-4/_config.js b/packages/svelte/tests/runtime-runes/samples/lifecycle-render-order-for-children-4/_config.js index 0cd7e15a3726..29b0b67a5235 100644 --- a/packages/svelte/tests/runtime-runes/samples/lifecycle-render-order-for-children-4/_config.js +++ b/packages/svelte/tests/runtime-runes/samples/lifecycle-render-order-for-children-4/_config.js @@ -10,7 +10,6 @@ export default test({ assert.deepEqual(logs, [ 'parent: $effect.pre 0', 'parent: nested $effect.pre 0', - 'parent: render 0', '1: $effect.pre 0', '1: nested $effect.pre 0', '1: render 0', @@ -20,6 +19,7 @@ export default test({ '3: $effect.pre 0', '3: nested $effect.pre 0', '3: render 0', + 'parent: render 0', '1: $effect 0', '2: $effect 0', '3: $effect 0', @@ -33,7 +33,6 @@ export default test({ assert.deepEqual(logs, [ 'parent: $effect.pre 1', 'parent: nested $effect.pre 1', - 'parent: render 1', '1: $effect.pre 1', '1: nested $effect.pre 1', '1: render 1', @@ -43,6 +42,7 @@ export default test({ '3: $effect.pre 1', '3: nested $effect.pre 1', '3: render 1', + 'parent: render 1', '1: $effect 1', '2: $effect 1', '3: $effect 1', diff --git a/packages/svelte/tests/runtime-runes/samples/lifecycle-render-order-for-children/_config.js b/packages/svelte/tests/runtime-runes/samples/lifecycle-render-order-for-children/_config.js index 19b8fb39383c..3138ec7231d7 100644 --- a/packages/svelte/tests/runtime-runes/samples/lifecycle-render-order-for-children/_config.js +++ b/packages/svelte/tests/runtime-runes/samples/lifecycle-render-order-for-children/_config.js @@ -9,13 +9,13 @@ export default test({ async test({ assert, component, logs }) { assert.deepEqual(logs, [ 'parent: $effect.pre 0', - 'parent: render 0', '1: $effect.pre 0', '1: render 0', '2: $effect.pre 0', '2: render 0', '3: $effect.pre 0', '3: render 0', + 'parent: render 0', '1: $effect 0', '2: $effect 0', '3: $effect 0', @@ -28,13 +28,13 @@ export default test({ assert.deepEqual(logs, [ 'parent: $effect.pre 1', - 'parent: render 1', '1: $effect.pre 1', '1: render 1', '2: $effect.pre 1', '2: render 1', '3: $effect.pre 1', '3: render 1', + 'parent: render 1', '1: $effect 1', '2: $effect 1', '3: $effect 1', diff --git a/packages/svelte/tests/snapshot/samples/text-nodes-deriveds/_expected/client/index.svelte.js b/packages/svelte/tests/snapshot/samples/text-nodes-deriveds/_expected/client/index.svelte.js index 2e0bd651da38..d2220bd0a4fa 100644 --- a/packages/svelte/tests/snapshot/samples/text-nodes-deriveds/_expected/client/index.svelte.js +++ b/packages/svelte/tests/snapshot/samples/text-nodes-deriveds/_expected/client/index.svelte.js @@ -20,7 +20,11 @@ export default function Text_nodes_deriveds($$anchor) { const expression_1 = $.derived(() => text2() ?? ''); var text = $.child(p); - $.template_effect(() => $.set_text(text, `${$.get(expression)}${$.get(expression_1)}`)); $.reset(p); + + $.template_effect(() => { + $.set_text(text, `${$.get(expression)}${$.get(expression_1)}`); + }); + $.append($$anchor, p); } \ No newline at end of file From 947bfa14a4dd94db41f4b4d9f64212cc0fb7584d Mon Sep 17 00:00:00 2001 From: Rich Harris Date: Fri, 17 Jan 2025 15:03:59 -0500 Subject: [PATCH 06/22] unused --- .../phases/3-transform/client/visitors/shared/fragment.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/svelte/src/compiler/phases/3-transform/client/visitors/shared/fragment.js b/packages/svelte/src/compiler/phases/3-transform/client/visitors/shared/fragment.js index 0191a3879277..0b4ac873428e 100644 --- a/packages/svelte/src/compiler/phases/3-transform/client/visitors/shared/fragment.js +++ b/packages/svelte/src/compiler/phases/3-transform/client/visitors/shared/fragment.js @@ -4,7 +4,7 @@ import { cannot_be_set_statically } from '../../../../../../utils.js'; import { is_event_attribute, is_text_attribute } from '../../../../../utils/ast.js'; import * as b from '../../../../../utils/builders.js'; -import { build_template_chunk, build_update } from './utils.js'; +import { build_template_chunk } from './utils.js'; /** * Processes an array of template nodes, joining sibling text/expression nodes From 3aded8fc91d9917f1d9d8efc2c3862bbc211c8de Mon Sep 17 00:00:00 2001 From: Rich Harris Date: Fri, 17 Jan 2025 15:10:01 -0500 Subject: [PATCH 07/22] more --- .../3-transform/client/visitors/shared/element.js | 14 +++++--------- 1 file changed, 5 insertions(+), 9 deletions(-) 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 7fd323b6ec08..bba3f25fb959 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 @@ -1,12 +1,12 @@ /** @import { Expression, Identifier, ObjectExpression } from 'estree' */ -/** @import { AST, Namespace } from '#compiler' */ +/** @import { AST } from '#compiler' */ /** @import { ComponentClientTransformState, ComponentContext } from '../../types' */ import { normalize_attribute } from '../../../../../../utils.js'; import { is_ignored } from '../../../../../state.js'; -import { get_attribute_expression, is_event_attribute } from '../../../../../utils/ast.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_template_chunk, build_update } from './utils.js'; +import { build_template_chunk } from './utils.js'; /** * @param {Array} attributes @@ -128,9 +128,7 @@ export function build_style_directives( ) ); - if (!is_attributes_reactive && has_call) { - state.init.push(build_update(update)); - } else if (is_attributes_reactive || has_state || has_call) { + if (has_state || is_attributes_reactive) { state.update.push(update); } else { state.init.push(update); @@ -166,9 +164,7 @@ export function build_class_directives( const update = b.stmt(b.call('$.toggle_class', element_id, b.literal(directive.name), value)); - if (!is_attributes_reactive && has_call) { - state.init.push(build_update(update)); - } else if (is_attributes_reactive || has_state || has_call) { + if (is_attributes_reactive || has_state) { state.update.push(update); } else { state.init.push(update); From 1efc88eab43219c455fc17cedf39f78812b28f03 Mon Sep 17 00:00:00 2001 From: Rich Harris Date: Fri, 17 Jan 2025 15:18:30 -0500 Subject: [PATCH 08/22] WIP --- .../3-transform/client/visitors/RegularElement.js | 13 +++++-------- .../3-transform/client/visitors/shared/element.js | 5 +++-- .../3-transform/client/visitors/shared/utils.js | 10 ---------- .../_expected/client/main.svelte.js | 7 ++++--- 4 files changed, 12 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 43ac96ef97e3..5e4730efda2f 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,4 +1,4 @@ -/** @import { Expression, ExpressionStatement, Identifier, MemberExpression, ObjectExpression, Statement } from 'estree' */ +/** @import { Expression, ExpressionStatement, Identifier, MemberExpression, Statement } from 'estree' */ /** @import { AST } from '#compiler' */ /** @import { SourceLocation } from '#shared' */ /** @import { ComponentClientTransformState, ComponentContext } from '../types' */ @@ -28,7 +28,6 @@ import { process_children } from './shared/fragment.js'; import { build_render_statement, build_template_chunk, - build_update, build_update_assignment } from './shared/utils.js'; import { visit_event_attribute } from './shared/events.js'; @@ -644,7 +643,7 @@ function build_element_attribute_update_assignment( function build_custom_element_attribute_update_assignment(node_id, attribute, context) { const state = context.state; const name = attribute.name; // don't lowercase, as we set the element's property, which might be case sensitive - let { has_call, value } = build_attribute_value(attribute.value, context); + let { value } = build_attribute_value(attribute.value, context, true); // We assume that noone's going to redefine the semantics of the class attribute on custom elements, i.e. it's still used for CSS classes if (name === 'class' && attribute.metadata.needs_clsx) { @@ -657,11 +656,9 @@ function build_custom_element_attribute_update_assignment(node_id, attribute, co const update = b.stmt(b.call('$.set_custom_element_data', node_id, b.literal(name), value)); if (attribute.metadata.expression.has_state) { - if (has_call) { - state.init.push(build_update(update)); - } else { - state.update.push(update); - } + // this is different from other updates — it doesn't get grouped, + // because set_custom_element_data may not be idempotent + state.init.push(b.stmt(b.call('$.template_effect', b.thunk(update.expression)))); return true; } else { state.init.push(update); 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 bba3f25fb959..7de9c941013a 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 @@ -175,9 +175,10 @@ export function build_class_directives( /** * @param {AST.Attribute['value']} value * @param {ComponentContext} context + * @param {boolean} [is_custom_element] * @returns {{ value: Expression, has_state: boolean, has_call: boolean }} */ -export function build_attribute_value(value, context) { +export function build_attribute_value(value, context, is_custom_element = false) { if (value === true) { return { has_state: false, has_call: false, value: b.literal(true) }; } @@ -191,7 +192,7 @@ export function build_attribute_value(value, context) { let expression = /** @type {Expression} */ (context.visit(chunk.expression)); - if (chunk.metadata.expression.has_call) { + if (chunk.metadata.expression.has_call && !is_custom_element) { // TODO this is temporary const id = b.id(context.state.scope.generate('expression')); context.state.init.push( 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 8cfbdf9efda7..504bd278a889 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 @@ -85,16 +85,6 @@ export function build_template_chunk(values, visit, state) { return { value, has_state, has_call }; } -/** - * @param {Statement} statement - */ -export function build_update(statement) { - const body = - statement.type === 'ExpressionStatement' ? statement.expression : b.block([statement]); - - return b.stmt(b.call('$.template_effect', b.thunk(body))); -} - /** * @param {Statement[]} update */ diff --git a/packages/svelte/tests/snapshot/samples/dynamic-attributes-casing/_expected/client/main.svelte.js b/packages/svelte/tests/snapshot/samples/dynamic-attributes-casing/_expected/client/main.svelte.js index d5935c4fbb3c..2ac2c42cf71e 100644 --- a/packages/svelte/tests/snapshot/samples/dynamic-attributes-casing/_expected/client/main.svelte.js +++ b/packages/svelte/tests/snapshot/samples/dynamic-attributes-casing/_expected/client/main.svelte.js @@ -11,19 +11,20 @@ export default function Main($$anchor) { var div = $.first_child(fragment); var svg = $.sibling(div, 2); var custom_element = $.sibling(svg, 2); + + $.template_effect(() => $.set_custom_element_data(custom_element, 'fooBar', x)); + var div_1 = $.sibling(custom_element, 2); const expression = $.derived(() => y() ?? ''); var svg_1 = $.sibling(div_1, 2); const expression_1 = $.derived(() => y() ?? ''); var custom_element_1 = $.sibling(svg_1, 2); - const expression_2 = $.derived(() => y() ?? ''); - $.template_effect(() => $.set_custom_element_data(custom_element_1, 'fooBar', $.get(expression_2))); + $.template_effect(() => $.set_custom_element_data(custom_element_1, 'fooBar', y())); $.template_effect(() => { $.set_attribute(div, 'foobar', x); $.set_attribute(svg, 'viewBox', x); - $.set_custom_element_data(custom_element, 'fooBar', x); $.set_attribute(div_1, 'foobar', $.get(expression)); $.set_attribute(svg_1, 'viewBox', $.get(expression_1)); }); From d87b927f8158c3ef621f56ad3fded00df9fe825b Mon Sep 17 00:00:00 2001 From: Rich Harris Date: Fri, 17 Jan 2025 15:29:08 -0500 Subject: [PATCH 09/22] unused --- .../3-transform/client/transform-client.js | 1 - .../phases/3-transform/client/types.d.ts | 2 -- .../3-transform/client/visitors/Fragment.js | 17 ++++++----------- .../client/visitors/SvelteElement.js | 1 - 4 files changed, 6 insertions(+), 15 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 582c32b534ec..cf9e62fd9951 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 @@ -169,7 +169,6 @@ export function client_component(analysis, options) { module_level_snippets: [], // these are set inside the `Fragment` visitor, and cannot be used until then - before_init: /** @type {any} */ (null), init: /** @type {any} */ (null), update: /** @type {any} */ (null), after_update: /** @type {any} */ (null), 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 5c8476de3e3c..99d33384fab2 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 @@ -46,8 +46,6 @@ export interface ComponentClientTransformState extends ClientTransformState { readonly events: Set; readonly is_instance: boolean; - /** Stuff that happens before the render effect(s) */ - readonly before_init: Statement[]; /** Stuff that happens before the render effect(s) */ readonly init: Statement[]; /** Stuff that happens inside the render effect */ 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 0e6ea29614ff..e03cda3a0e1c 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 @@ -61,7 +61,6 @@ export function Fragment(node, context) { /** @type {ComponentClientTransformState} */ const state = { ...context.state, - before_init: [], init: [], update: [], after_update: [], @@ -124,18 +123,14 @@ export function Fragment(node, context) { add_template(template_name, args); - body.push(b.var(id, b.call(template_name)), ...state.before_init, ...state.init); + body.push(b.var(id, b.call(template_name)), ...state.init); close = b.stmt(b.call('$.append', b.id('$$anchor'), id)); } else if (is_single_child_not_needing_template) { context.visit(trimmed[0], state); - body.push(...state.before_init, ...state.init); + body.push(...state.init); } else if (trimmed.length === 1 && trimmed[0].type === 'Text') { const id = b.id(context.state.scope.generate('text')); - body.push( - b.var(id, b.call('$.text', b.literal(trimmed[0].data))), - ...state.before_init, - ...state.init - ); + body.push(b.var(id, b.call('$.text', b.literal(trimmed[0].data))), ...state.init); close = b.stmt(b.call('$.append', b.id('$$anchor'), id)); } else if (trimmed.length > 0) { const id = b.id(context.state.scope.generate('fragment')); @@ -153,7 +148,7 @@ export function Fragment(node, context) { state }); - body.push(b.var(id, b.call('$.text')), ...state.before_init, ...state.init); + body.push(b.var(id, b.call('$.text')), ...state.init); close = b.stmt(b.call('$.append', b.id('$$anchor'), id)); } else { if (is_standalone) { @@ -183,10 +178,10 @@ export function Fragment(node, context) { close = b.stmt(b.call('$.append', b.id('$$anchor'), id)); } - body.push(...state.before_init, ...state.init); + body.push(...state.init); } } else { - body.push(...state.before_init, ...state.init); + body.push(...state.init); } if (state.update.length > 0) { 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 6dad4bdfe198..e4cf7d05ebbf 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 @@ -45,7 +45,6 @@ export function SvelteElement(node, context) { state: { ...context.state, node: element_id, - before_init: [], init: [], update: [], after_update: [] From a75599502d23382a41bef097266ef2dc3bfdaeea Mon Sep 17 00:00:00 2001 From: Rich Harris Date: Fri, 17 Jan 2025 15:33:35 -0500 Subject: [PATCH 10/22] simplify --- .../phases/3-transform/client/visitors/Fragment.js | 13 +++++-------- 1 file changed, 5 insertions(+), 8 deletions(-) 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 e03cda3a0e1c..5c11d4599ecb 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 @@ -123,14 +123,13 @@ export function Fragment(node, context) { add_template(template_name, args); - body.push(b.var(id, b.call(template_name)), ...state.init); + body.push(b.var(id, b.call(template_name))); close = b.stmt(b.call('$.append', b.id('$$anchor'), id)); } else if (is_single_child_not_needing_template) { context.visit(trimmed[0], state); - body.push(...state.init); } else if (trimmed.length === 1 && trimmed[0].type === 'Text') { const id = b.id(context.state.scope.generate('text')); - body.push(b.var(id, b.call('$.text', b.literal(trimmed[0].data))), ...state.init); + body.push(b.var(id, b.call('$.text', b.literal(trimmed[0].data)))); close = b.stmt(b.call('$.append', b.id('$$anchor'), id)); } else if (trimmed.length > 0) { const id = b.id(context.state.scope.generate('fragment')); @@ -148,7 +147,7 @@ export function Fragment(node, context) { state }); - body.push(b.var(id, b.call('$.text')), ...state.init); + body.push(b.var(id, b.call('$.text'))); close = b.stmt(b.call('$.append', b.id('$$anchor'), id)); } else { if (is_standalone) { @@ -177,13 +176,11 @@ export function Fragment(node, context) { close = b.stmt(b.call('$.append', b.id('$$anchor'), id)); } - - body.push(...state.init); } - } else { - body.push(...state.init); } + body.push(...state.init); + if (state.update.length > 0) { body.push(build_render_statement(state.update)); } From cf5fc07c57db3085fd3462c080e074a5767c6c03 Mon Sep 17 00:00:00 2001 From: Rich Harris Date: Fri, 17 Jan 2025 15:42:24 -0500 Subject: [PATCH 11/22] simplify --- .../client/visitors/shared/utils.js | 20 +++++-------------- 1 file changed, 5 insertions(+), 15 deletions(-) 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 504bd278a889..4e5e79ffa502 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 @@ -45,30 +45,20 @@ export function build_template_chunk(values, visit, state) { quasi.value.cooked += node.expression.value + ''; } } else { + const value = /** @type {Expression} */ (visit(node.expression, state)); + if (node.metadata.expression.has_call) { const id = b.id(state.scope.generate('expression')); state.init.push( - b.const( - id, - create_derived( - state, - b.thunk( - b.logical( - '??', - /** @type {Expression} */ (visit(node.expression, state)), - b.literal('') - ) - ) - ) - ) + b.const(id, create_derived(state, b.thunk(b.logical('??', value, b.literal(''))))) ); expressions.push(b.call('$.get', id)); } else 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: visit(node.expression, state), has_state, has_call }; + return { value, has_state, has_call }; } else { - expressions.push(b.logical('??', visit(node.expression, state), b.literal(''))); + expressions.push(b.logical('??', value, b.literal(''))); } quasi = b.quasi('', i + 1 === values.length); From fcee5a94b92e1e8c36a7d29db877c0342d0007df Mon Sep 17 00:00:00 2001 From: Rich Harris Date: Fri, 17 Jan 2025 15:44:48 -0500 Subject: [PATCH 12/22] simplify --- .../3-transform/client/visitors/shared/utils.js | 12 ++++++------ .../_expected/client/index.svelte.js | 6 +++--- 2 files changed, 9 insertions(+), 9 deletions(-) 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 4e5e79ffa502..43bef8277009 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 @@ -45,15 +45,15 @@ export function build_template_chunk(values, visit, state) { quasi.value.cooked += node.expression.value + ''; } } else { - const value = /** @type {Expression} */ (visit(node.expression, state)); + let value = /** @type {Expression} */ (visit(node.expression, state)); if (node.metadata.expression.has_call) { const id = b.id(state.scope.generate('expression')); - state.init.push( - b.const(id, create_derived(state, b.thunk(b.logical('??', value, b.literal(''))))) - ); - expressions.push(b.call('$.get', id)); - } else if (values.length === 1) { + state.init.push(b.const(id, create_derived(state, b.thunk(value)))); + value = b.call('$.get', id); + } + + 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, has_call }; diff --git a/packages/svelte/tests/snapshot/samples/text-nodes-deriveds/_expected/client/index.svelte.js b/packages/svelte/tests/snapshot/samples/text-nodes-deriveds/_expected/client/index.svelte.js index d2220bd0a4fa..9fced7aa7af6 100644 --- a/packages/svelte/tests/snapshot/samples/text-nodes-deriveds/_expected/client/index.svelte.js +++ b/packages/svelte/tests/snapshot/samples/text-nodes-deriveds/_expected/client/index.svelte.js @@ -16,14 +16,14 @@ export default function Text_nodes_deriveds($$anchor) { } var p = root(); - const expression = $.derived(() => text1() ?? ''); - const expression_1 = $.derived(() => text2() ?? ''); + const expression = $.derived(text1); + const expression_1 = $.derived(text2); var text = $.child(p); $.reset(p); $.template_effect(() => { - $.set_text(text, `${$.get(expression)}${$.get(expression_1)}`); + $.set_text(text, `${$.get(expression) ?? ''}${$.get(expression_1) ?? ''}`); }); $.append($$anchor, p); From f558765a7d87fa328894963375b53b23c4f58a41 Mon Sep 17 00:00:00 2001 From: Rich Harris Date: Fri, 17 Jan 2025 16:48:31 -0500 Subject: [PATCH 13/22] more --- .../3-transform/client/transform-client.js | 1 + .../phases/3-transform/client/types.d.ts | 2 + .../3-transform/client/visitors/Fragment.js | 3 +- .../client/visitors/RegularElement.js | 21 +---- .../client/visitors/SvelteElement.js | 3 +- .../client/visitors/shared/utils.js | 77 +++++++++++++++++-- .../src/internal/client/reactivity/effects.js | 16 ++-- .../_expected/client/index.svelte.js | 11 +-- 8 files changed, 96 insertions(+), 38 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 cf9e62fd9951..f4a6c9a4147b 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 @@ -171,6 +171,7 @@ 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), locations: /** @type {any} */ (null) 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 99d33384fab2..db4adf451c2e 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 @@ -52,6 +52,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[]; /** The HTML template string */ readonly template: Array; readonly locations: SourceLocation[]; 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 5c11d4599ecb..e9cfd9c50684 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 @@ -63,6 +63,7 @@ export function Fragment(node, context) { ...context.state, init: [], update: [], + expressions: [], after_update: [], template: [], locations: [], @@ -182,7 +183,7 @@ export function Fragment(node, context) { body.push(...state.init); if (state.update.length > 0) { - body.push(build_render_statement(state.update)); + body.push(build_render_statement(state)); } body.push(...state.after_update); 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 5e4730efda2f..1d02df21e6fa 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 @@ -16,7 +16,7 @@ import { is_event_attribute, is_text_attribute } from '../../../../utils/ast.js' import * as b from '../../../../utils/builders.js'; import { is_custom_element_node } from '../../../nodes.js'; import { clean_nodes, determine_namespace_for_children } from '../../utils.js'; -import { build_getter, create_derived } from '../utils.js'; +import { build_getter } from '../utils.js'; import { get_attribute_name, build_attribute_value, @@ -408,7 +408,7 @@ export function RegularElement(node, context) { b.block([ ...child_state.init, ...element_state.init, - child_state.update.length > 0 ? build_render_statement(child_state.update) : b.empty, + child_state.update.length > 0 ? build_render_statement(child_state) : b.empty, ...child_state.after_update, ...element_state.after_update ]) @@ -556,15 +556,6 @@ function build_element_attribute_update_assignment( value = b.call('$.clsx', value); } - if (attribute.metadata.expression.has_state && has_call) { - // ensure we're not creating a separate template effect for this so that - // potential class directives are added to the same effect and therefore always apply - const id = b.id(state.scope.generate('class_derived')); - state.init.push(b.const(id, create_derived(state, b.thunk(value)))); - value = b.call('$.get', id); - has_call = false; - } - update = b.stmt( b.call( is_svg ? '$.set_svg_class' : is_mathml ? '$.set_mathml_class' : '$.set_class', @@ -604,14 +595,6 @@ function build_element_attribute_update_assignment( } else if (is_dom_property(name)) { update = b.stmt(b.assignment('=', b.member(node_id, name), value)); } else { - if (name === 'style' && attribute.metadata.expression.has_state && has_call) { - // ensure we're not creating a separate template effect for this so that - // potential style directives are added to the same effect and therefore always apply - const id = b.id(state.scope.generate('style_derived')); - state.init.push(b.const(id, create_derived(state, b.thunk(value)))); - value = b.call('$.get', id); - has_call = false; - } const callee = name.startsWith('xlink') ? '$.set_xlink_attribute' : '$.set_attribute'; update = b.stmt( b.call( 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 e4cf7d05ebbf..e27528365518 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 @@ -47,6 +47,7 @@ export function SvelteElement(node, context) { node: element_id, init: [], update: [], + expressions: [], after_update: [] } }; @@ -118,7 +119,7 @@ export function SvelteElement(node, context) { /** @type {Statement[]} */ const inner = inner_context.state.init; if (inner_context.state.update.length > 0) { - inner.push(build_render_statement(inner_context.state.update)); + inner.push(build_render_statement(inner_context.state)); } inner.push(...inner_context.state.after_update); inner.push( 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 43bef8277009..a2e09dfe1a6f 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 @@ -6,10 +6,64 @@ import { object } from '../../../../../utils/ast.js'; import * as b from '../../../../../utils/builders.js'; import { sanitize_template_string } from '../../../../../utils/sanitize_template_string.js'; import { regex_is_valid_identifier } from '../../../../patterns.js'; -import { create_derived } from '../../utils.js'; import is_reference from 'is-reference'; import { locator } from '../../../../../state.js'; +/** + * + * @param {ComponentClientTransformState} state + * @param {Expression} value + */ +function get_expression_id(state, value) { + for (let i = 0; i < state.expressions.length; i += 1) { + if (compare_expressions(state.expressions[i], value)) { + return b.id(`$${i}`); + } + } + + return b.id(`$${state.expressions.push(value) - 1}`); +} + +/** + * Returns true of two expressions have an identical AST shape + * @param {Expression} a + * @param {Expression} b + */ +function compare_expressions(a, b) { + if (a.type !== b.type) { + return false; + } + + for (const key in a) { + if (key === 'type' || key === 'metadata' || key === 'loc' || key === 'start' || key === 'end') { + continue; + } + + const va = /** @type {any} */ (a)[key]; + const vb = /** @type {any} */ (b)[key]; + + if ((typeof va === 'object') !== (typeof vb === 'object')) { + return false; + } + + if (typeof va !== 'object' || va === null || vb === null) { + if (va !== vb) return false; + } else if (Array.isArray(va)) { + if (va.length !== vb.length) { + return false; + } + + if (va.some((v, i) => !compare_expressions(v, vb[i]))) { + return false; + } + } else if (!compare_expressions(va, vb)) { + return false; + } + } + + return true; +} + /** * @param {Array} values * @param {(node: AST.SvelteNode, state: any) => any} visit @@ -48,9 +102,7 @@ export function build_template_chunk(values, visit, state) { let value = /** @type {Expression} */ (visit(node.expression, state)); if (node.metadata.expression.has_call) { - const id = b.id(state.scope.generate('expression')); - state.init.push(b.const(id, create_derived(state, b.thunk(value)))); - value = b.call('$.get', id); + value = get_expression_id(state, value); } if (values.length === 1) { @@ -76,10 +128,21 @@ export function build_template_chunk(values, visit, state) { } /** - * @param {Statement[]} update + * @param {ComponentClientTransformState} state */ -export function build_render_statement(update) { - return b.stmt(b.call('$.template_effect', b.thunk(b.block(update)))); +export function build_render_statement(state) { + return b.stmt( + b.call( + '$.template_effect', + b.arrow( + state.expressions.map((_, i) => b.id(`$${i}`)), + 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') + ) + ); } /** diff --git a/packages/svelte/src/internal/client/reactivity/effects.js b/packages/svelte/src/internal/client/reactivity/effects.js index 428f69281ba3..e8546331501a 100644 --- a/packages/svelte/src/internal/client/reactivity/effects.js +++ b/packages/svelte/src/internal/client/reactivity/effects.js @@ -43,7 +43,8 @@ import * as e from '../errors.js'; import { DEV } from 'esm-env'; import { define_property } from '../../shared/utils.js'; import { get_next_sibling } from '../dom/operations.js'; -import { destroy_derived } from './deriveds.js'; +import { derived, derived_safe_equal, destroy_derived } from './deriveds.js'; +import { legacy_mode_flag } from '../../flags/index.js'; /** * @param {'$effect' | '$effect.pre' | '$inspect'} rune @@ -343,16 +344,21 @@ export function render_effect(fn) { } /** - * @param {() => void | (() => void)} fn + * @param {(...expressions: any) => void | (() => void)} fn + * @param {Array<() => any>} thunks * @returns {Effect} */ -export function template_effect(fn) { +export function template_effect(fn, thunks = [], d = derived) { + const deriveds = thunks.map(d); + const effect = () => fn(...deriveds.map(get)); + if (DEV) { - define_property(fn, 'name', { + define_property(effect, 'name', { value: '{expression}' }); } - return block(fn); + + return block(effect); } /** diff --git a/packages/svelte/tests/snapshot/samples/text-nodes-deriveds/_expected/client/index.svelte.js b/packages/svelte/tests/snapshot/samples/text-nodes-deriveds/_expected/client/index.svelte.js index 9fced7aa7af6..3cb06d297745 100644 --- a/packages/svelte/tests/snapshot/samples/text-nodes-deriveds/_expected/client/index.svelte.js +++ b/packages/svelte/tests/snapshot/samples/text-nodes-deriveds/_expected/client/index.svelte.js @@ -16,15 +16,16 @@ export default function Text_nodes_deriveds($$anchor) { } var p = root(); - const expression = $.derived(text1); - const expression_1 = $.derived(text2); var text = $.child(p); $.reset(p); - $.template_effect(() => { - $.set_text(text, `${$.get(expression) ?? ''}${$.get(expression_1) ?? ''}`); - }); + $.template_effect( + ($0, $1) => { + $.set_text(text, `${$0 ?? ''}${$1 ?? ''}`); + }, + [text1, text2] + ); $.append($$anchor, p); } \ No newline at end of file From 4f48be969c302ae76aa62fce5dd57cd2068a2863 Mon Sep 17 00:00:00 2001 From: Rich Harris Date: Fri, 17 Jan 2025 17:09:17 -0500 Subject: [PATCH 14/22] unused --- .../phases/3-transform/client/visitors/RegularElement.js | 2 +- .../phases/3-transform/client/visitors/shared/element.js | 7 +++---- 2 files changed, 4 insertions(+), 5 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 1d02df21e6fa..8b76fa78fd70 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 @@ -535,7 +535,7 @@ function build_element_attribute_update_assignment( const name = get_attribute_name(element, attribute); const is_svg = context.state.metadata.namespace === 'svg' || element.name === 'svg'; const is_mathml = context.state.metadata.namespace === 'mathml'; - let { has_call, value } = build_attribute_value(attribute.value, context); + let { value } = build_attribute_value(attribute.value, context); if (name === 'autofocus') { state.init.push(b.stmt(b.call('$.autofocus', node_id, value))); 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 7de9c941013a..8f391bcece49 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 @@ -176,18 +176,18 @@ export function build_class_directives( * @param {AST.Attribute['value']} value * @param {ComponentContext} context * @param {boolean} [is_custom_element] - * @returns {{ value: Expression, has_state: boolean, has_call: boolean }} + * @returns {{ value: Expression, has_state: boolean }} */ export function build_attribute_value(value, context, is_custom_element = false) { if (value === true) { - return { has_state: false, has_call: false, value: b.literal(true) }; + return { has_state: false, value: b.literal(true) }; } if (!Array.isArray(value) || value.length === 1) { const chunk = Array.isArray(value) ? value[0] : value; if (chunk.type === 'Text') { - return { has_state: false, has_call: false, value: b.literal(chunk.data) }; + return { has_state: false, value: b.literal(chunk.data) }; } let expression = /** @type {Expression} */ (context.visit(chunk.expression)); @@ -206,7 +206,6 @@ export function build_attribute_value(value, context, is_custom_element = false) return { has_state: chunk.metadata.expression.has_state, - has_call: chunk.metadata.expression.has_call, value: expression }; } From bc09464ce67b91cdd4f7ed7a672f5b6ec1d89847 Mon Sep 17 00:00:00 2001 From: Rich Harris Date: Fri, 17 Jan 2025 17:10:42 -0500 Subject: [PATCH 15/22] unused --- .../phases/3-transform/client/visitors/shared/element.js | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) 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 8f391bcece49..cad2b7ce75b2 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 @@ -176,18 +176,18 @@ export function build_class_directives( * @param {AST.Attribute['value']} value * @param {ComponentContext} context * @param {boolean} [is_custom_element] - * @returns {{ value: Expression, has_state: boolean }} + * @returns {{ value: Expression }} */ export function build_attribute_value(value, context, is_custom_element = false) { if (value === true) { - return { has_state: false, value: b.literal(true) }; + return { value: b.literal(true) }; } if (!Array.isArray(value) || value.length === 1) { const chunk = Array.isArray(value) ? value[0] : value; if (chunk.type === 'Text') { - return { has_state: false, value: b.literal(chunk.data) }; + return { value: b.literal(chunk.data) }; } let expression = /** @type {Expression} */ (context.visit(chunk.expression)); @@ -205,7 +205,6 @@ export function build_attribute_value(value, context, is_custom_element = false) } return { - has_state: chunk.metadata.expression.has_state, value: expression }; } From 39302136db22403c21354db127462b767314c2f0 Mon Sep 17 00:00:00 2001 From: Rich Harris Date: Fri, 17 Jan 2025 17:16:22 -0500 Subject: [PATCH 16/22] more --- .../client/visitors/BindDirective.js | 2 +- .../client/visitors/RegularElement.js | 8 ++++---- .../3-transform/client/visitors/SlotElement.js | 2 +- .../client/visitors/SvelteElement.js | 2 +- .../client/visitors/shared/component.js | 4 ++-- .../client/visitors/shared/element.js | 16 +++++++--------- .../client/visitors/shared/utils.js | 18 +++++------------- 7 files changed, 21 insertions(+), 31 deletions(-) diff --git a/packages/svelte/src/compiler/phases/3-transform/client/visitors/BindDirective.js b/packages/svelte/src/compiler/phases/3-transform/client/visitors/BindDirective.js index 0a49e89cfe09..21f9c957bec0 100644 --- a/packages/svelte/src/compiler/phases/3-transform/client/visitors/BindDirective.js +++ b/packages/svelte/src/compiler/phases/3-transform/client/visitors/BindDirective.js @@ -222,7 +222,7 @@ export function BindDirective(node, context) { if (value !== undefined) { group_getter = b.thunk( - b.block([b.stmt(build_attribute_value(value, context).value), b.return(expression)]) + b.block([b.stmt(build_attribute_value(value, context)), b.return(expression)]) ); } } 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 8b76fa78fd70..851452a63ae0 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 @@ -105,7 +105,7 @@ export function RegularElement(node, context) { case 'Attribute': // `is` attributes need to be part of the template, otherwise they break if (attribute.name === 'is' && context.state.metadata.namespace === 'html') { - const { value } = build_attribute_value(attribute.value, context); + const value = build_attribute_value(attribute.value, context); if (value.type === 'Literal' && typeof value.value === 'string') { context.state.template.push(` is="${escape_html(value.value, true)}"`); @@ -535,7 +535,7 @@ function build_element_attribute_update_assignment( const name = get_attribute_name(element, attribute); const is_svg = context.state.metadata.namespace === 'svg' || element.name === 'svg'; const is_mathml = context.state.metadata.namespace === 'mathml'; - let { value } = build_attribute_value(attribute.value, context); + let value = build_attribute_value(attribute.value, context); if (name === 'autofocus') { state.init.push(b.stmt(b.call('$.autofocus', node_id, value))); @@ -626,7 +626,7 @@ function build_element_attribute_update_assignment( function build_custom_element_attribute_update_assignment(node_id, attribute, context) { const state = context.state; const name = attribute.name; // don't lowercase, as we set the element's property, which might be case sensitive - let { value } = build_attribute_value(attribute.value, context, true); + let value = build_attribute_value(attribute.value, context, true); // We assume that noone's going to redefine the semantics of the class attribute on custom elements, i.e. it's still used for CSS classes if (name === 'class' && attribute.metadata.needs_clsx) { @@ -661,7 +661,7 @@ 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 } = build_attribute_value(attribute.value, context); + const value = build_attribute_value(attribute.value, context); const inner_assignment = b.assignment( '=', diff --git a/packages/svelte/src/compiler/phases/3-transform/client/visitors/SlotElement.js b/packages/svelte/src/compiler/phases/3-transform/client/visitors/SlotElement.js index 86734a07ab12..8e9d77c04751 100644 --- a/packages/svelte/src/compiler/phases/3-transform/client/visitors/SlotElement.js +++ b/packages/svelte/src/compiler/phases/3-transform/client/visitors/SlotElement.js @@ -29,7 +29,7 @@ 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 } = build_attribute_value(attribute.value, context); + const value = build_attribute_value(attribute.value, context); if (attribute.name === 'name') { name = /** @type {Literal} */ (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 e27528365518..26e19296016e 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 @@ -144,7 +144,7 @@ export function SvelteElement(node, context) { get_tag, node.metadata.svg || node.metadata.mathml ? b.true : b.false, inner.length > 0 && b.arrow([element_id, b.id('$$anchor')], b.block(inner)), - dynamic_namespace && b.thunk(build_attribute_value(dynamic_namespace, context).value), + dynamic_namespace && b.thunk(build_attribute_value(dynamic_namespace, context)), location && b.array([b.literal(location.line), b.literal(location.column)]) ) ) diff --git a/packages/svelte/src/compiler/phases/3-transform/client/visitors/shared/component.js b/packages/svelte/src/compiler/phases/3-transform/client/visitors/shared/component.js index f509cb41a7d8..2fb434c2f6a9 100644 --- a/packages/svelte/src/compiler/phases/3-transform/client/visitors/shared/component.js +++ b/packages/svelte/src/compiler/phases/3-transform/client/visitors/shared/component.js @@ -132,7 +132,7 @@ export function build_component(node, component_name, context, anchor = context. } else if (attribute.type === 'Attribute') { if (attribute.name.startsWith('--')) { custom_css_props.push( - b.init(attribute.name, build_attribute_value(attribute.value, context).value) + b.init(attribute.name, build_attribute_value(attribute.value, context)) ); continue; } @@ -145,7 +145,7 @@ export function build_component(node, component_name, context, anchor = context. has_children_prop = true; } - const { value } = build_attribute_value(attribute.value, context); + const value = build_attribute_value(attribute.value, context); if (attribute.metadata.expression.has_state) { let arg = value; 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 cad2b7ce75b2..e49eea335fd7 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 @@ -35,7 +35,7 @@ export function build_set_attributes( for (const attribute of attributes) { if (attribute.type === 'Attribute') { - const { value } = build_attribute_value(attribute.value, context); + const value = build_attribute_value(attribute.value, context); if ( is_event_attribute(attribute) && @@ -109,7 +109,7 @@ 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; + : build_attribute_value(directive.value, context); if (has_call) { const id = b.id(state.scope.generate('style_directive')); @@ -176,18 +176,18 @@ export function build_class_directives( * @param {AST.Attribute['value']} value * @param {ComponentContext} context * @param {boolean} [is_custom_element] - * @returns {{ value: Expression }} + * @returns {Expression} */ export function build_attribute_value(value, context, is_custom_element = false) { if (value === true) { - return { value: b.literal(true) }; + return b.literal(true); } 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) }; + return b.literal(chunk.data); } let expression = /** @type {Expression} */ (context.visit(chunk.expression)); @@ -204,12 +204,10 @@ export function build_attribute_value(value, context, is_custom_element = false) expression = b.call('$.get', id); } - return { - value: expression - }; + return expression; } - return build_template_chunk(value, context.visit, context.state); + return build_template_chunk(value, context.visit, context.state).value; } /** 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 a2e09dfe1a6f..606dc6aa12d5 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 @@ -68,7 +68,7 @@ function compare_expressions(a, b) { * @param {Array} values * @param {(node: AST.SvelteNode, state: any) => any} visit * @param {ComponentClientTransformState} state - * @returns {{ value: Expression, has_state: boolean, has_call: boolean }} + * @returns {{ value: Expression, has_state: boolean }} */ export function build_template_chunk(values, visit, state) { /** @type {Expression[]} */ @@ -77,18 +77,8 @@ export function build_template_chunk(values, visit, state) { let quasi = b.quasi(''); const quasis = [quasi]; - let has_call = false; let has_state = false; - for (const node of values) { - if (node.type === 'ExpressionTag') { - const metadata = node.metadata.expression; - - has_call ||= metadata.has_call; - has_state ||= metadata.has_state; - } - } - for (let i = 0; i < values.length; i++) { const node = values[i]; @@ -101,6 +91,8 @@ export function build_template_chunk(values, visit, state) { } else { let value = /** @type {Expression} */ (visit(node.expression, state)); + has_state ||= node.metadata.expression.has_state; + if (node.metadata.expression.has_call) { value = get_expression_id(state, value); } @@ -108,7 +100,7 @@ export function build_template_chunk(values, visit, state) { 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, has_call }; + return { value, has_state }; } else { expressions.push(b.logical('??', value, b.literal(''))); } @@ -124,7 +116,7 @@ export function build_template_chunk(values, visit, state) { const value = b.template(quasis, expressions); - return { value, has_state, has_call }; + return { value, has_state }; } /** From 81a1ee22b6fb2b61fc8739c7afaa3f8aaaf7c39e Mon Sep 17 00:00:00 2001 From: Rich Harris Date: Fri, 17 Jan 2025 17:25:09 -0500 Subject: [PATCH 17/22] tweak --- .../phases/3-transform/client/visitors/shared/utils.js | 4 +++- .../_expected/client/index.svelte.js | 5 +---- .../_expected/client/index.svelte.js | 5 +---- .../_expected/client/index.svelte.js | 5 +---- .../skip-static-subtree/_expected/client/index.svelte.js | 6 +----- .../text-nodes-deriveds/_expected/client/index.svelte.js | 9 +-------- 6 files changed, 8 insertions(+), 26 deletions(-) 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 606dc6aa12d5..a06522acf847 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 @@ -128,7 +128,9 @@ export function build_render_statement(state) { '$.template_effect', b.arrow( state.expressions.map((_, i) => b.id(`$${i}`)), - b.block(state.update) + 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))), diff --git a/packages/svelte/tests/snapshot/samples/bind-component-snippet/_expected/client/index.svelte.js b/packages/svelte/tests/snapshot/samples/bind-component-snippet/_expected/client/index.svelte.js index a46847da24af..fa990b33ee56 100644 --- a/packages/svelte/tests/snapshot/samples/bind-component-snippet/_expected/client/index.svelte.js +++ b/packages/svelte/tests/snapshot/samples/bind-component-snippet/_expected/client/index.svelte.js @@ -29,9 +29,6 @@ export default function Bind_component_snippet($$anchor) { var text_1 = $.sibling(node); - $.template_effect(() => { - $.set_text(text_1, ` value: ${$.get(value) ?? ''}`); - }); - + $.template_effect(() => $.set_text(text_1, ` value: ${$.get(value) ?? ''}`)); $.append($$anchor, fragment); } \ No newline at end of file diff --git a/packages/svelte/tests/snapshot/samples/each-string-template/_expected/client/index.svelte.js b/packages/svelte/tests/snapshot/samples/each-string-template/_expected/client/index.svelte.js index 7a0db9f44a8b..c0626bd416c9 100644 --- a/packages/svelte/tests/snapshot/samples/each-string-template/_expected/client/index.svelte.js +++ b/packages/svelte/tests/snapshot/samples/each-string-template/_expected/client/index.svelte.js @@ -11,10 +11,7 @@ export default function Each_string_template($$anchor) { var text = $.text(); - $.template_effect(() => { - $.set_text(text, `${thing ?? ''}, `); - }); - + $.template_effect(() => $.set_text(text, `${thing ?? ''}, `)); $.append($$anchor, text); }); diff --git a/packages/svelte/tests/snapshot/samples/function-prop-no-getter/_expected/client/index.svelte.js b/packages/svelte/tests/snapshot/samples/function-prop-no-getter/_expected/client/index.svelte.js index f413b5992fe6..c545608bcacf 100644 --- a/packages/svelte/tests/snapshot/samples/function-prop-no-getter/_expected/client/index.svelte.js +++ b/packages/svelte/tests/snapshot/samples/function-prop-no-getter/_expected/client/index.svelte.js @@ -19,10 +19,7 @@ export default function Function_prop_no_getter($$anchor) { var text = $.text(); - $.template_effect(() => { - $.set_text(text, `clicks: ${$.get(count) ?? ''}`); - }); - + $.template_effect(() => $.set_text(text, `clicks: ${$.get(count) ?? ''}`)); $.append($$anchor, text); }, $$slots: { default: true } diff --git a/packages/svelte/tests/snapshot/samples/skip-static-subtree/_expected/client/index.svelte.js b/packages/svelte/tests/snapshot/samples/skip-static-subtree/_expected/client/index.svelte.js index 089b828c892b..9b203b97e82d 100644 --- a/packages/svelte/tests/snapshot/samples/skip-static-subtree/_expected/client/index.svelte.js +++ b/packages/svelte/tests/snapshot/samples/skip-static-subtree/_expected/client/index.svelte.js @@ -46,11 +46,7 @@ export default function Skip_static_subtree($$anchor, $$props) { var img_1 = $.child(div_2); $.reset(div_2); - - $.template_effect(() => { - $.set_text(text, $$props.title); - }); - + $.template_effect(() => $.set_text(text, $$props.title)); $.handle_lazy_img(img); $.handle_lazy_img(img_1); $.append($$anchor, fragment); diff --git a/packages/svelte/tests/snapshot/samples/text-nodes-deriveds/_expected/client/index.svelte.js b/packages/svelte/tests/snapshot/samples/text-nodes-deriveds/_expected/client/index.svelte.js index 3cb06d297745..d520d1ef2488 100644 --- a/packages/svelte/tests/snapshot/samples/text-nodes-deriveds/_expected/client/index.svelte.js +++ b/packages/svelte/tests/snapshot/samples/text-nodes-deriveds/_expected/client/index.svelte.js @@ -19,13 +19,6 @@ export default function Text_nodes_deriveds($$anchor) { var text = $.child(p); $.reset(p); - - $.template_effect( - ($0, $1) => { - $.set_text(text, `${$0 ?? ''}${$1 ?? ''}`); - }, - [text1, text2] - ); - + $.template_effect(($0, $1) => $.set_text(text, `${$0 ?? ''}${$1 ?? ''}`), [text1, text2]); $.append($$anchor, p); } \ No newline at end of file From 69e0c211b96636247323709175bdfe956390661a Mon Sep 17 00:00:00 2001 From: Rich Harris Date: Fri, 17 Jan 2025 17:40:22 -0500 Subject: [PATCH 18/22] update how class/style directives are handled --- .../client/visitors/shared/element.js | 16 +++------------- .../3-transform/client/visitors/shared/utils.js | 2 +- .../_config.js | 4 ++-- 3 files changed, 6 insertions(+), 16 deletions(-) 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 e49eea335fd7..78f0dffbe703 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 @@ -6,7 +6,7 @@ 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_template_chunk } from './utils.js'; +import { build_template_chunk, get_expression_id } from './utils.js'; /** * @param {Array} attributes @@ -104,20 +104,13 @@ export function build_style_directives( const state = context.state; for (const directive of style_directives) { - const { has_state, has_call } = directive.metadata.expression; + const { has_state } = directive.metadata.expression; let value = directive.value === true ? build_getter({ name: directive.name, type: 'Identifier' }, context.state) : build_attribute_value(directive.value, context); - if (has_call) { - const id = b.id(state.scope.generate('style_directive')); - - state.init.push(b.const(id, create_derived(state, b.thunk(value)))); - value = b.call('$.get', id); - } - const update = b.stmt( b.call( '$.set_style', @@ -156,10 +149,7 @@ export function build_class_directives( let value = /** @type {Expression} */ (context.visit(directive.expression)); if (has_call) { - const id = b.id(state.scope.generate('class_directive')); - - state.init.push(b.const(id, create_derived(state, b.thunk(value)))); - value = b.call('$.get', id); + value = get_expression_id(state, value); } const update = b.stmt(b.call('$.toggle_class', element_id, b.literal(directive.name), value)); 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 a06522acf847..fb48e42a943c 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 @@ -14,7 +14,7 @@ import { locator } from '../../../../../state.js'; * @param {ComponentClientTransformState} state * @param {Expression} value */ -function get_expression_id(state, value) { +export function get_expression_id(state, value) { for (let i = 0; i < state.expressions.length; i += 1) { if (compare_expressions(state.expressions[i], value)) { return b.id(`$${i}`); diff --git a/packages/svelte/tests/runtime-runes/samples/dynamic-attribute-and-attribute-directive-2/_config.js b/packages/svelte/tests/runtime-runes/samples/dynamic-attribute-and-attribute-directive-2/_config.js index ec50fc725351..dbd87ac33f9c 100644 --- a/packages/svelte/tests/runtime-runes/samples/dynamic-attribute-and-attribute-directive-2/_config.js +++ b/packages/svelte/tests/runtime-runes/samples/dynamic-attribute-and-attribute-directive-2/_config.js @@ -7,8 +7,8 @@ export default test({ const button = target.querySelector('button'); assert.deepEqual(logs, [ - 'updated class attribute', 'updated class directive', + 'updated class attribute', 'updated style attribute', 'updated style directive' ]); @@ -21,8 +21,8 @@ export default test({ flushSync(() => button?.click()); assert.deepEqual(logs, [ - 'updated class attribute', 'updated class directive', + 'updated class attribute', 'updated style attribute', 'updated style directive', 'updated class attribute', From 73dfeed69ed29e1ec58de5203fd2c5a0f611d1e7 Mon Sep 17 00:00:00 2001 From: Rich Harris Date: Fri, 17 Jan 2025 18:23:31 -0500 Subject: [PATCH 19/22] more --- .../client/visitors/RegularElement.js | 14 ++++++--- .../client/visitors/shared/element.js | 29 ++++++++++--------- .../client/visitors/shared/utils.js | 10 +++++-- .../_config.js | 4 +-- .../_expected/client/main.svelte.js | 17 ++++++----- 5 files changed, 45 insertions(+), 29 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 851452a63ae0..3c414b62d10f 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 @@ -28,7 +28,8 @@ import { process_children } from './shared/fragment.js'; import { build_render_statement, build_template_chunk, - build_update_assignment + build_update_assignment, + get_expression_id } from './shared/utils.js'; import { visit_event_attribute } from './shared/events.js'; @@ -535,7 +536,10 @@ function build_element_attribute_update_assignment( const name = get_attribute_name(element, attribute); const is_svg = context.state.metadata.namespace === 'svg' || element.name === 'svg'; const is_mathml = context.state.metadata.namespace === 'mathml'; - let value = build_attribute_value(attribute.value, context); + + let value = build_attribute_value(attribute.value, context, (value) => + get_expression_id(state, value) + ); if (name === 'autofocus') { state.init.push(b.stmt(b.call('$.autofocus', node_id, value))); @@ -626,7 +630,7 @@ function build_element_attribute_update_assignment( function build_custom_element_attribute_update_assignment(node_id, attribute, context) { const state = context.state; const name = attribute.name; // don't lowercase, as we set the element's property, which might be case sensitive - let value = build_attribute_value(attribute.value, context, true); + let value = build_attribute_value(attribute.value, context, (value) => value); // We assume that noone's going to redefine the semantics of the class attribute on custom elements, i.e. it's still used for CSS classes if (name === 'class' && attribute.metadata.needs_clsx) { @@ -661,7 +665,9 @@ 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 = build_attribute_value(attribute.value, context); + const value = build_attribute_value(attribute.value, context, (value) => + get_expression_id(state, value) + ); const inner_assignment = b.assignment( '=', 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 78f0dffbe703..6c29e21e78c6 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 @@ -165,10 +165,21 @@ export function build_class_directives( /** * @param {AST.Attribute['value']} value * @param {ComponentContext} context - * @param {boolean} [is_custom_element] + * @param {(value: Expression) => Expression} memoize * @returns {Expression} */ -export function build_attribute_value(value, context, is_custom_element = false) { +export function build_attribute_value( + value, + context, + memoize = (value) => { + // TODO this is temporary + const id = b.id(context.state.scope.generate('expression')); + context.state.init.push( + b.const(id, create_derived(context.state, b.thunk(b.logical('??', value, b.literal(''))))) + ); + return b.call('$.get', id); + } +) { if (value === true) { return b.literal(true); } @@ -182,22 +193,14 @@ export function build_attribute_value(value, context, is_custom_element = false) let expression = /** @type {Expression} */ (context.visit(chunk.expression)); - if (chunk.metadata.expression.has_call && !is_custom_element) { - // TODO this is temporary - const id = b.id(context.state.scope.generate('expression')); - context.state.init.push( - b.const( - id, - create_derived(context.state, b.thunk(b.logical('??', expression, b.literal('')))) - ) - ); - expression = b.call('$.get', id); + if (chunk.metadata.expression.has_call) { + expression = memoize(expression); } return expression; } - return build_template_chunk(value, context.visit, context.state).value; + return build_template_chunk(value, context.visit, context.state, memoize).value; } /** 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 fb48e42a943c..686da9451751 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 @@ -68,9 +68,15 @@ function compare_expressions(a, b) { * @param {Array} values * @param {(node: AST.SvelteNode, state: any) => any} visit * @param {ComponentClientTransformState} state + * @param {(value: Expression) => Expression} memoize * @returns {{ value: Expression, has_state: boolean }} */ -export function build_template_chunk(values, visit, state) { +export function build_template_chunk( + values, + visit, + state, + memoize = (value) => get_expression_id(state, value) +) { /** @type {Expression[]} */ const expressions = []; @@ -94,7 +100,7 @@ export function build_template_chunk(values, visit, state) { has_state ||= node.metadata.expression.has_state; if (node.metadata.expression.has_call) { - value = get_expression_id(state, value); + value = memoize(value); } if (values.length === 1) { diff --git a/packages/svelte/tests/runtime-runes/samples/dynamic-attribute-and-attribute-directive-2/_config.js b/packages/svelte/tests/runtime-runes/samples/dynamic-attribute-and-attribute-directive-2/_config.js index dbd87ac33f9c..ec50fc725351 100644 --- a/packages/svelte/tests/runtime-runes/samples/dynamic-attribute-and-attribute-directive-2/_config.js +++ b/packages/svelte/tests/runtime-runes/samples/dynamic-attribute-and-attribute-directive-2/_config.js @@ -7,8 +7,8 @@ export default test({ const button = target.querySelector('button'); assert.deepEqual(logs, [ - 'updated class directive', 'updated class attribute', + 'updated class directive', 'updated style attribute', 'updated style directive' ]); @@ -21,8 +21,8 @@ export default test({ flushSync(() => button?.click()); assert.deepEqual(logs, [ - 'updated class directive', 'updated class attribute', + 'updated class directive', 'updated style attribute', 'updated style directive', 'updated class attribute', diff --git a/packages/svelte/tests/snapshot/samples/dynamic-attributes-casing/_expected/client/main.svelte.js b/packages/svelte/tests/snapshot/samples/dynamic-attributes-casing/_expected/client/main.svelte.js index 2ac2c42cf71e..d97a58bf40d8 100644 --- a/packages/svelte/tests/snapshot/samples/dynamic-attributes-casing/_expected/client/main.svelte.js +++ b/packages/svelte/tests/snapshot/samples/dynamic-attributes-casing/_expected/client/main.svelte.js @@ -15,19 +15,20 @@ export default function Main($$anchor) { $.template_effect(() => $.set_custom_element_data(custom_element, 'fooBar', x)); var div_1 = $.sibling(custom_element, 2); - const expression = $.derived(() => y() ?? ''); var svg_1 = $.sibling(div_1, 2); - const expression_1 = $.derived(() => y() ?? ''); var custom_element_1 = $.sibling(svg_1, 2); $.template_effect(() => $.set_custom_element_data(custom_element_1, 'fooBar', y())); - $.template_effect(() => { - $.set_attribute(div, 'foobar', x); - $.set_attribute(svg, 'viewBox', x); - $.set_attribute(div_1, 'foobar', $.get(expression)); - $.set_attribute(svg_1, 'viewBox', $.get(expression_1)); - }); + $.template_effect( + ($0) => { + $.set_attribute(div, 'foobar', x); + $.set_attribute(svg, 'viewBox', x); + $.set_attribute(div_1, 'foobar', $0); + $.set_attribute(svg_1, 'viewBox', $0); + }, + [y] + ); $.append($$anchor, fragment); } \ No newline at end of file From 3a8f4e076f7ce04cd5c1469b6460d45e0f27c00c Mon Sep 17 00:00:00 2001 From: Rich Harris Date: Fri, 17 Jan 2025 19:13:06 -0500 Subject: [PATCH 20/22] more --- .../client/visitors/RegularElement.js | 2 +- .../client/visitors/SlotElement.js | 5 +++- .../client/visitors/shared/component.js | 14 +++++++--- .../client/visitors/shared/element.js | 27 ++++++------------- .../client/visitors/shared/utils.js | 11 ++++++++ 5 files changed, 35 insertions(+), 24 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 3c414b62d10f..83254235b80a 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 @@ -630,7 +630,7 @@ function build_element_attribute_update_assignment( function build_custom_element_attribute_update_assignment(node_id, attribute, context) { const state = context.state; const name = attribute.name; // don't lowercase, as we set the element's property, which might be case sensitive - let value = build_attribute_value(attribute.value, context, (value) => value); + let value = build_attribute_value(attribute.value, context); // We assume that noone's going to redefine the semantics of the class attribute on custom elements, i.e. it's still used for CSS classes if (name === 'class' && attribute.metadata.needs_clsx) { diff --git a/packages/svelte/src/compiler/phases/3-transform/client/visitors/SlotElement.js b/packages/svelte/src/compiler/phases/3-transform/client/visitors/SlotElement.js index 8e9d77c04751..1ff835b19ad9 100644 --- a/packages/svelte/src/compiler/phases/3-transform/client/visitors/SlotElement.js +++ b/packages/svelte/src/compiler/phases/3-transform/client/visitors/SlotElement.js @@ -3,6 +3,7 @@ /** @import { ComponentContext } from '../types' */ import * as b from '../../../../utils/builders.js'; import { build_attribute_value } from './shared/element.js'; +import { memoize_expression } from './shared/utils.js'; /** * @param {AST.SlotElement} node @@ -29,7 +30,9 @@ 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 = build_attribute_value(attribute.value, context); + const value = build_attribute_value(attribute.value, context, (value) => + memoize_expression(context.state, value) + ); if (attribute.name === 'name') { name = /** @type {Literal} */ (value); diff --git a/packages/svelte/src/compiler/phases/3-transform/client/visitors/shared/component.js b/packages/svelte/src/compiler/phases/3-transform/client/visitors/shared/component.js index 2fb434c2f6a9..a9314153a529 100644 --- a/packages/svelte/src/compiler/phases/3-transform/client/visitors/shared/component.js +++ b/packages/svelte/src/compiler/phases/3-transform/client/visitors/shared/component.js @@ -5,7 +5,7 @@ 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, validate_binding } from '../shared/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'; import { determine_slot } from '../../../../../utils/slot.js'; @@ -132,7 +132,13 @@ export function build_component(node, component_name, context, anchor = context. } else if (attribute.type === 'Attribute') { if (attribute.name.startsWith('--')) { custom_css_props.push( - b.init(attribute.name, build_attribute_value(attribute.value, context)) + b.init( + attribute.name, + build_attribute_value(attribute.value, context, (value) => + // TODO put the derived in the local block + memoize_expression(context.state, value) + ) + ) ); continue; } @@ -145,7 +151,9 @@ export function build_component(node, component_name, context, anchor = context. has_children_prop = true; } - const value = build_attribute_value(attribute.value, context); + const value = build_attribute_value(attribute.value, context, (value) => + memoize_expression(context.state, value) + ); if (attribute.metadata.expression.has_state) { let arg = value; 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 6c29e21e78c6..91b21a9d91e2 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 @@ -35,7 +35,9 @@ export function build_set_attributes( for (const attribute of attributes) { if (attribute.type === 'Attribute') { - const value = build_attribute_value(attribute.value, context); + const value = build_attribute_value(attribute.value, context, (value) => + get_expression_id(context.state, value) + ); if ( is_event_attribute(attribute) && @@ -109,7 +111,9 @@ 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); + : build_attribute_value(directive.value, context, (value) => + get_expression_id(context.state, value) + ); const update = b.stmt( b.call( @@ -168,18 +172,7 @@ export function build_class_directives( * @param {(value: Expression) => Expression} memoize * @returns {Expression} */ -export function build_attribute_value( - value, - context, - memoize = (value) => { - // TODO this is temporary - const id = b.id(context.state.scope.generate('expression')); - context.state.init.push( - b.const(id, create_derived(context.state, b.thunk(b.logical('??', value, b.literal(''))))) - ); - return b.call('$.get', id); - } -) { +export function build_attribute_value(value, context, memoize = (value) => value) { if (value === true) { return b.literal(true); } @@ -193,11 +186,7 @@ export function build_attribute_value( let expression = /** @type {Expression} */ (context.visit(chunk.expression)); - if (chunk.metadata.expression.has_call) { - expression = memoize(expression); - } - - return expression; + return chunk.metadata.expression.has_call ? memoize(expression) : expression; } return build_template_chunk(value, context.visit, context.state, memoize).value; 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 686da9451751..9dd29843299b 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 @@ -8,6 +8,17 @@ import { sanitize_template_string } from '../../../../../utils/sanitize_template import { regex_is_valid_identifier } from '../../../../patterns.js'; import is_reference from 'is-reference'; import { locator } from '../../../../../state.js'; +import { create_derived } from '../../utils.js'; + +/** + * @param {ComponentClientTransformState} state + * @param {Expression} value + */ +export function memoize_expression(state, value) { + const id = b.id(state.scope.generate('expression')); + state.init.push(b.const(id, create_derived(state, b.thunk(value)))); + return b.call('$.get', id); +} /** * From 226b264eed7660162a75f3e3c1547de52458c4ef Mon Sep 17 00:00:00 2001 From: Rich Harris Date: Fri, 17 Jan 2025 20:26:00 -0500 Subject: [PATCH 21/22] simplify --- .../phases/2-analyze/visitors/Attribute.js | 3 --- .../client/visitors/BindDirective.js | 2 +- .../client/visitors/RegularElement.js | 14 +++++----- .../client/visitors/SlotElement.js | 4 +-- .../client/visitors/SvelteBoundary.js | 2 +- .../client/visitors/SvelteElement.js | 2 +- .../client/visitors/shared/component.js | 6 ++--- .../client/visitors/shared/element.js | 27 ++++++++++--------- packages/svelte/src/compiler/phases/nodes.js | 1 - .../svelte/src/compiler/types/template.d.ts | 1 - 10 files changed, 30 insertions(+), 32 deletions(-) diff --git a/packages/svelte/src/compiler/phases/2-analyze/visitors/Attribute.js b/packages/svelte/src/compiler/phases/2-analyze/visitors/Attribute.js index 9d801e095e8d..41144fc74c5c 100644 --- a/packages/svelte/src/compiler/phases/2-analyze/visitors/Attribute.js +++ b/packages/svelte/src/compiler/phases/2-analyze/visitors/Attribute.js @@ -61,9 +61,6 @@ export function Attribute(node, context) { ) { continue; } - - node.metadata.expression.has_state ||= chunk.metadata.expression.has_state; - node.metadata.expression.has_call ||= chunk.metadata.expression.has_call; } if (is_event_attribute(node)) { diff --git a/packages/svelte/src/compiler/phases/3-transform/client/visitors/BindDirective.js b/packages/svelte/src/compiler/phases/3-transform/client/visitors/BindDirective.js index 21f9c957bec0..0a49e89cfe09 100644 --- a/packages/svelte/src/compiler/phases/3-transform/client/visitors/BindDirective.js +++ b/packages/svelte/src/compiler/phases/3-transform/client/visitors/BindDirective.js @@ -222,7 +222,7 @@ export function BindDirective(node, context) { if (value !== undefined) { group_getter = b.thunk( - b.block([b.stmt(build_attribute_value(value, context)), b.return(expression)]) + b.block([b.stmt(build_attribute_value(value, context).value), b.return(expression)]) ); } } 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 83254235b80a..21a78de032c4 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 @@ -106,7 +106,7 @@ export function RegularElement(node, context) { case 'Attribute': // `is` attributes need to be part of the template, otherwise they break if (attribute.name === 'is' && context.state.metadata.namespace === 'html') { - const value = build_attribute_value(attribute.value, context); + const { value } = build_attribute_value(attribute.value, context); if (value.type === 'Literal' && typeof value.value === 'string') { context.state.template.push(` is="${escape_html(value.value, true)}"`); @@ -537,7 +537,7 @@ 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 = build_attribute_value(attribute.value, context, (value) => + let { value, has_state } = build_attribute_value(attribute.value, context, (value) => get_expression_id(state, value) ); @@ -611,7 +611,7 @@ function build_element_attribute_update_assignment( ); } - if (attribute.metadata.expression.has_state) { + if (has_state) { state.update.push(update); return true; } else { @@ -630,7 +630,7 @@ function build_element_attribute_update_assignment( function build_custom_element_attribute_update_assignment(node_id, attribute, context) { const state = context.state; const name = attribute.name; // don't lowercase, as we set the element's property, which might be case sensitive - let value = build_attribute_value(attribute.value, context); + let { value, has_state } = build_attribute_value(attribute.value, context); // We assume that noone's going to redefine the semantics of the class attribute on custom elements, i.e. it's still used for CSS classes if (name === 'class' && attribute.metadata.needs_clsx) { @@ -642,7 +642,7 @@ function build_custom_element_attribute_update_assignment(node_id, attribute, co const update = b.stmt(b.call('$.set_custom_element_data', node_id, b.literal(name), value)); - if (attribute.metadata.expression.has_state) { + if (has_state) { // this is different from other updates — it doesn't get grouped, // because set_custom_element_data may not be idempotent state.init.push(b.stmt(b.call('$.template_effect', b.thunk(update.expression)))); @@ -665,7 +665,7 @@ 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 = build_attribute_value(attribute.value, context, (value) => + const { value, has_state } = build_attribute_value(attribute.value, context, (value) => get_expression_id(state, value) ); @@ -701,7 +701,7 @@ function build_element_special_value_attribute(element, node_id, attribute, cont state.init.push(b.stmt(b.call('$.init_select', node_id, b.thunk(value)))); } - if (attribute.metadata.expression.has_state) { + if (has_state) { const id = state.scope.generate(`${node_id.name}_value`); build_update_assignment( state, diff --git a/packages/svelte/src/compiler/phases/3-transform/client/visitors/SlotElement.js b/packages/svelte/src/compiler/phases/3-transform/client/visitors/SlotElement.js index 1ff835b19ad9..f1b08acbc695 100644 --- a/packages/svelte/src/compiler/phases/3-transform/client/visitors/SlotElement.js +++ b/packages/svelte/src/compiler/phases/3-transform/client/visitors/SlotElement.js @@ -30,7 +30,7 @@ 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 = build_attribute_value(attribute.value, context, (value) => + const { value, has_state } = build_attribute_value(attribute.value, context, (value) => memoize_expression(context.state, value) ); @@ -38,7 +38,7 @@ export function SlotElement(node, context) { name = /** @type {Literal} */ (value); is_default = false; } else if (attribute.name !== 'slot') { - if (attribute.metadata.expression.has_state) { + if (has_state) { props.push(b.get(attribute.name, [b.return(value)])); } else { props.push(b.init(attribute.name, value)); diff --git a/packages/svelte/src/compiler/phases/3-transform/client/visitors/SvelteBoundary.js b/packages/svelte/src/compiler/phases/3-transform/client/visitors/SvelteBoundary.js index 325485d4c003..6da650a591d9 100644 --- a/packages/svelte/src/compiler/phases/3-transform/client/visitors/SvelteBoundary.js +++ b/packages/svelte/src/compiler/phases/3-transform/client/visitors/SvelteBoundary.js @@ -23,7 +23,7 @@ export function SvelteBoundary(node, context) { const expression = /** @type {Expression} */ (context.visit(chunk.expression, context.state)); - if (attribute.metadata.expression.has_state) { + if (chunk.metadata.expression.has_state) { props.properties.push(b.get(attribute.name, [b.return(expression)])); } else { props.properties.push(b.init(attribute.name, expression)); 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 26e19296016e..e27528365518 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 @@ -144,7 +144,7 @@ export function SvelteElement(node, context) { get_tag, node.metadata.svg || node.metadata.mathml ? b.true : b.false, inner.length > 0 && b.arrow([element_id, b.id('$$anchor')], b.block(inner)), - dynamic_namespace && b.thunk(build_attribute_value(dynamic_namespace, context)), + dynamic_namespace && b.thunk(build_attribute_value(dynamic_namespace, context).value), location && b.array([b.literal(location.line), b.literal(location.column)]) ) ) diff --git a/packages/svelte/src/compiler/phases/3-transform/client/visitors/shared/component.js b/packages/svelte/src/compiler/phases/3-transform/client/visitors/shared/component.js index a9314153a529..9ac0bac12046 100644 --- a/packages/svelte/src/compiler/phases/3-transform/client/visitors/shared/component.js +++ b/packages/svelte/src/compiler/phases/3-transform/client/visitors/shared/component.js @@ -137,7 +137,7 @@ export function build_component(node, component_name, context, anchor = context. build_attribute_value(attribute.value, context, (value) => // TODO put the derived in the local block memoize_expression(context.state, value) - ) + ).value ) ); continue; @@ -151,11 +151,11 @@ export function build_component(node, component_name, context, anchor = context. has_children_prop = true; } - const value = build_attribute_value(attribute.value, context, (value) => + const { value, has_state } = build_attribute_value(attribute.value, context, (value) => memoize_expression(context.state, value) ); - if (attribute.metadata.expression.has_state) { + if (has_state) { let arg = value; // When we have a non-simple computation, anything other than an Identifier or Member expression, 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 91b21a9d91e2..8fb6b8bdde84 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 @@ -28,14 +28,14 @@ export function build_set_attributes( is_custom_element, state ) { - let has_state = false; + let is_dynamic = false; /** @type {ObjectExpression['properties']} */ const values = []; for (const attribute of attributes) { if (attribute.type === 'Attribute') { - const value = build_attribute_value(attribute.value, context, (value) => + const { value, has_state } = build_attribute_value(attribute.value, context, (value) => get_expression_id(context.state, value) ); @@ -51,10 +51,10 @@ export function build_set_attributes( values.push(b.init(attribute.name, value)); } - has_state ||= attribute.metadata.expression.has_state; + is_dynamic ||= has_state; } else { // objects could contain reactive getters -> play it safe and always assume spread attributes are reactive - has_state = true; + is_dynamic = true; let value = /** @type {Expression} */ (context.visit(attribute)); @@ -70,7 +70,7 @@ export function build_set_attributes( const call = b.call( '$.set_attributes', element_id, - has_state ? attributes_id : b.literal(null), + is_dynamic ? attributes_id : b.literal(null), b.object(values), context.state.analysis.css.hash !== '' && b.literal(context.state.analysis.css.hash), preserve_attribute_case, @@ -78,7 +78,7 @@ export function build_set_attributes( is_ignored(element, 'hydration_attribute_changed') && b.true ); - if (has_state) { + if (is_dynamic) { context.state.init.push(b.let(attributes_id)); const update = b.stmt(b.assignment('=', attributes_id, call)); context.state.update.push(update); @@ -113,7 +113,7 @@ export function build_style_directives( ? build_getter({ name: directive.name, type: 'Identifier' }, context.state) : build_attribute_value(directive.value, context, (value) => get_expression_id(context.state, value) - ); + ).value; const update = b.stmt( b.call( @@ -170,26 +170,29 @@ export function build_class_directives( * @param {AST.Attribute['value']} value * @param {ComponentContext} context * @param {(value: Expression) => Expression} memoize - * @returns {Expression} + * @returns {{ value: Expression, has_state: boolean }} */ export function build_attribute_value(value, context, memoize = (value) => value) { if (value === true) { - return b.literal(true); + return { value: b.literal(true), has_state: false }; } if (!Array.isArray(value) || value.length === 1) { const chunk = Array.isArray(value) ? value[0] : value; if (chunk.type === 'Text') { - return b.literal(chunk.data); + return { value: b.literal(chunk.data), has_state: false }; } let expression = /** @type {Expression} */ (context.visit(chunk.expression)); - return chunk.metadata.expression.has_call ? memoize(expression) : expression; + return { + value: chunk.metadata.expression.has_call ? memoize(expression) : expression, + has_state: chunk.metadata.expression.has_state + }; } - return build_template_chunk(value, context.visit, context.state, memoize).value; + return build_template_chunk(value, context.visit, context.state, memoize); } /** diff --git a/packages/svelte/src/compiler/phases/nodes.js b/packages/svelte/src/compiler/phases/nodes.js index 5066833feb8e..5ca4ce3380a3 100644 --- a/packages/svelte/src/compiler/phases/nodes.js +++ b/packages/svelte/src/compiler/phases/nodes.js @@ -44,7 +44,6 @@ export function create_attribute(name, start, end, value) { name, value, metadata: { - expression: create_expression_metadata(), delegated: null, needs_clsx: false } diff --git a/packages/svelte/src/compiler/types/template.d.ts b/packages/svelte/src/compiler/types/template.d.ts index 8be9aed17723..fb609668957d 100644 --- a/packages/svelte/src/compiler/types/template.d.ts +++ b/packages/svelte/src/compiler/types/template.d.ts @@ -479,7 +479,6 @@ export namespace AST { value: true | ExpressionTag | Array; /** @internal */ metadata: { - expression: ExpressionMetadata; /** May be set if this is an event attribute */ delegated: null | DelegatedEvent; /** May be `true` if this is a `class` attribute that needs `clsx` */ From 34ad6527188e174edfb6465fc556040062435413 Mon Sep 17 00:00:00 2001 From: Rich Harris Date: Fri, 17 Jan 2025 21:26:00 -0500 Subject: [PATCH 22/22] changeset --- .changeset/cyan-games-cheat.md | 5 +++++ 1 file changed, 5 insertions(+) create mode 100644 .changeset/cyan-games-cheat.md diff --git a/.changeset/cyan-games-cheat.md b/.changeset/cyan-games-cheat.md new file mode 100644 index 000000000000..d90901783b17 --- /dev/null +++ b/.changeset/cyan-games-cheat.md @@ -0,0 +1,5 @@ +--- +'svelte': patch +--- + +fix: more efficient template effect grouping