From 578b5df23fd6a8f0be863ac3c2ea9b1e0fd72113 Mon Sep 17 00:00:00 2001 From: Jack Goodall Date: Mon, 28 Jul 2025 14:13:47 +0200 Subject: [PATCH] support spreading function bindings --- .../compiler/phases/1-parse/state/element.js | 29 ++++++ .../2-analyze/visitors/BindDirective.js | 21 +++++ .../client/visitors/BindDirective.js | 89 +++++++++++-------- .../client/visitors/shared/utils.js | 20 ++++- packages/svelte/src/compiler/phases/nodes.js | 3 +- packages/svelte/src/compiler/types/index.d.ts | 2 + .../svelte/src/compiler/types/template.d.ts | 5 +- .../samples/bind-spread/_config.js | 27 ++++++ .../samples/bind-spread/main.svelte | 24 +++++ packages/svelte/types/index.d.ts | 4 +- 10 files changed, 179 insertions(+), 45 deletions(-) create mode 100644 packages/svelte/tests/runtime-runes/samples/bind-spread/_config.js create mode 100644 packages/svelte/tests/runtime-runes/samples/bind-spread/main.svelte diff --git a/packages/svelte/src/compiler/phases/1-parse/state/element.js b/packages/svelte/src/compiler/phases/1-parse/state/element.js index ed1b047d55..f281c61f75 100644 --- a/packages/svelte/src/compiler/phases/1-parse/state/element.js +++ b/packages/svelte/src/compiler/phases/1-parse/state/element.js @@ -618,6 +618,15 @@ function read_attribute(parser) { e.directive_missing_name({ start, end: start + colon_index + 1 }, name); } + if ( + type !== 'BindDirective' && + value !== true && + 'metadata' in value && + value.metadata.expression.has_spread + ) { + e.directive_invalid_value(value.start); + } + if (type === 'StyleDirective') { return { start, @@ -646,6 +655,17 @@ function read_attribute(parser) { // TODO throw a parser error in a future version here if this `[ExpressionTag]` instead of `ExpressionTag`, // which means stringified value, which isn't allowed for some directives? expression = first_value.expression; + + // Handle spread syntax in bind directives + if (type === 'BindDirective' && first_value.metadata.expression.has_spread) { + // Create a SpreadElement to represent ...array syntax + expression = { + type: 'SpreadElement', + start: first_value.start, + end: first_value.end, + argument: expression + }; + } } } @@ -812,6 +832,13 @@ function read_sequence(parser, done, location) { flush(parser.index - 1); parser.allow_whitespace(); + + const has_spread = parser.match('...'); + if (has_spread) { + parser.eat('...', true); + parser.allow_whitespace(); + } + const expression = read_expression(parser); parser.allow_whitespace(); parser.eat('}', true); @@ -827,6 +854,8 @@ function read_sequence(parser, done, location) { } }; + chunk.metadata.expression.has_spread = has_spread; + chunks.push(chunk); current_chunk = { diff --git a/packages/svelte/src/compiler/phases/2-analyze/visitors/BindDirective.js b/packages/svelte/src/compiler/phases/2-analyze/visitors/BindDirective.js index 9f02e7fa5a..4ff27a68d8 100644 --- a/packages/svelte/src/compiler/phases/2-analyze/visitors/BindDirective.js +++ b/packages/svelte/src/compiler/phases/2-analyze/visitors/BindDirective.js @@ -158,6 +158,27 @@ export function BindDirective(node, context) { return; } + // Handle spread syntax for bind directives: bind:value={...bindings} + if (node.expression.type === 'SpreadElement') { + if (node.name === 'group') { + e.bind_group_invalid_expression(node); + } + + // Validate that the spread is applied to a valid expression that returns an array + const argument = node.expression.argument; + if ( + argument.type !== 'Identifier' && + argument.type !== 'MemberExpression' && + argument.type !== 'CallExpression' + ) { + e.bind_invalid_expression(node); + } + + mark_subtree_dynamic(context.path); + + return; + } + validate_assignment(node, node.expression, context); const assignee = node.expression; 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 506fd4aafd..5a87d72dbc 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 @@ -13,52 +13,67 @@ import { build_bind_this, validate_binding } from './shared/utils.js'; * @param {ComponentContext} context */ export function BindDirective(node, context) { - const expression = /** @type {Expression} */ (context.visit(node.expression)); - const property = binding_properties[node.name]; - - const parent = /** @type {AST.SvelteNode} */ (context.path.at(-1)); - let get, set; - - if (expression.type === 'SequenceExpression') { - [get, set] = expression.expressions; + + // Handle SpreadElement by creating a variable declaration before visiting + if (node.expression.type === 'SpreadElement') { + // Generate a unique variable name for this spread binding + const id = b.id(context.state.scope.generate('$$bindings')); + + // Store the spread expression in a variable at the component init level + const spread_expression = /** @type {Expression} */ (context.visit(node.expression.argument)); + context.state.init.push(b.const(id, spread_expression)); + + // Use member access to get getter and setter + get = b.member(id, b.literal(0), true); + set = b.member(id, b.literal(1), true); } else { - if ( - dev && - context.state.analysis.runes && - expression.type === 'MemberExpression' && - (node.name !== 'this' || - context.path.some( - ({ type }) => - type === 'IfBlock' || - type === 'EachBlock' || - type === 'AwaitBlock' || - type === 'KeyBlock' - )) && - !is_ignored(node, 'binding_property_non_reactive') - ) { - validate_binding(context.state, node, expression); - } + const expression = /** @type {Expression} */ (context.visit(node.expression)); - get = b.thunk(expression); + if (expression.type === 'SequenceExpression') { + [get, set] = expression.expressions; + } else { + if ( + dev && + context.state.analysis.runes && + expression.type === 'MemberExpression' && + (node.name !== 'this' || + context.path.some( + ({ type }) => + type === 'IfBlock' || + type === 'EachBlock' || + type === 'AwaitBlock' || + type === 'KeyBlock' + )) && + !is_ignored(node, 'binding_property_non_reactive') + ) { + validate_binding(context.state, node, expression); + } - /** @type {Expression | undefined} */ - set = b.unthunk( - b.arrow( - [b.id('$$value')], - /** @type {Expression} */ ( - context.visit( - b.assignment('=', /** @type {Pattern} */ (node.expression), b.id('$$value')) + get = b.thunk(expression); + + /** @type {Expression | undefined} */ + set = b.unthunk( + b.arrow( + [b.id('$$value')], + /** @type {Expression} */ ( + context.visit( + b.assignment('=', /** @type {Pattern} */ (node.expression), b.id('$$value')) + ) ) ) - ) - ); + ); - if (get === set) { - set = undefined; + if (get === set) { + set = undefined; + } } } + const property = binding_properties[node.name]; + + const parent = /** @type {AST.SvelteNode} */ (context.path.at(-1)); + /** @type {CallExpression} */ let call; @@ -222,7 +237,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).value), b.return(get)]) ); } } 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 014547cf2d..2fd6277660 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 @@ -1,4 +1,4 @@ -/** @import { AssignmentExpression, Expression, Identifier, MemberExpression, SequenceExpression, Literal, Super, UpdateExpression, ExpressionStatement } from 'estree' */ +/** @import { AssignmentExpression, Expression, Identifier, MemberExpression, SequenceExpression, SpreadElement, Literal, Super, UpdateExpression, ExpressionStatement } from 'estree' */ /** @import { AST, ExpressionMetadata } from '#compiler' */ /** @import { ComponentClientTransformState, ComponentContext, Context } from '../../types' */ import { walk } from 'zimmerframe'; @@ -204,11 +204,25 @@ export function parse_directive_name(name) { /** * Serializes `bind:this` for components and elements. - * @param {Identifier | MemberExpression | SequenceExpression} expression + * @param {Identifier | MemberExpression | SequenceExpression | SpreadElement} expression * @param {Expression} value * @param {import('zimmerframe').Context} context */ export function build_bind_this(expression, value, { state, visit }) { + if (expression.type === 'SpreadElement') { + // Generate a unique variable name for this spread binding + const id = b.id(state.scope.generate('$$bindings')); + + // Store the spread expression in a variable at the component init level + const spread_expression = /** @type {Expression} */ (visit(expression.argument)); + state.init.push(b.const(id, spread_expression)); + + // Use member access to get getter and setter + const get = b.member(id, b.literal(0), true); + const set = b.member(id, b.literal(1), true); + return b.call('$.bind_this', value, set, get); + } + if (expression.type === 'SequenceExpression') { const [get, set] = /** @type {SequenceExpression} */ (visit(expression)).expressions; return b.call('$.bind_this', value, set, get); @@ -290,7 +304,7 @@ export function build_bind_this(expression, value, { state, visit }) { * @param {MemberExpression} expression */ export function validate_binding(state, binding, expression) { - if (binding.expression.type === 'SequenceExpression') { + if (binding.expression.type === 'SequenceExpression' || binding.expression.type === 'SpreadElement') { return; } // If we are referencing a $store.foo then we don't need to add validation diff --git a/packages/svelte/src/compiler/phases/nodes.js b/packages/svelte/src/compiler/phases/nodes.js index 4874554ff0..6e29c23be8 100644 --- a/packages/svelte/src/compiler/phases/nodes.js +++ b/packages/svelte/src/compiler/phases/nodes.js @@ -67,7 +67,8 @@ export function create_expression_metadata() { has_call: false, has_member_expression: false, has_assignment: false, - has_await: false + has_await: false, + has_spread: false }; } diff --git a/packages/svelte/src/compiler/types/index.d.ts b/packages/svelte/src/compiler/types/index.d.ts index 6211e69bd3..7d8c51db0f 100644 --- a/packages/svelte/src/compiler/types/index.d.ts +++ b/packages/svelte/src/compiler/types/index.d.ts @@ -304,6 +304,8 @@ export interface ExpressionMetadata { has_member_expression: boolean; /** True if the expression includes an assignment or an update */ has_assignment: boolean; + /** True if the expression includes a spread element */ + has_spread: boolean; } export interface StateField { diff --git a/packages/svelte/src/compiler/types/template.d.ts b/packages/svelte/src/compiler/types/template.d.ts index 060df2dcb2..b3ae4fae5f 100644 --- a/packages/svelte/src/compiler/types/template.d.ts +++ b/packages/svelte/src/compiler/types/template.d.ts @@ -15,7 +15,8 @@ import type { Program, ChainExpression, SimpleCallExpression, - SequenceExpression + SequenceExpression, + SpreadElement } from 'estree'; import type { Scope } from '../phases/scope'; import type { _CSS } from './css'; @@ -211,7 +212,7 @@ export namespace AST { /** The 'x' in `bind:x` */ name: string; /** The y in `bind:x={y}` */ - expression: Identifier | MemberExpression | SequenceExpression; + expression: Identifier | MemberExpression | SequenceExpression | SpreadElement; /** @internal */ metadata: { binding_group_name: Identifier; diff --git a/packages/svelte/tests/runtime-runes/samples/bind-spread/_config.js b/packages/svelte/tests/runtime-runes/samples/bind-spread/_config.js new file mode 100644 index 0000000000..ae971acaf8 --- /dev/null +++ b/packages/svelte/tests/runtime-runes/samples/bind-spread/_config.js @@ -0,0 +1,27 @@ +import { flushSync } from 'svelte'; +import { test } from '../../test'; + +export default test({ + async test({ assert, target, logs }) { + const checkboxes = target.querySelectorAll('input'); + + // input.value = '2'; + // input.dispatchEvent(new window.Event('input')); + + flushSync(); + + assert.htmlEqual(target.innerHTML, ``.repeat(3)); + + // assert.deepEqual(logs, ['b', '2', 'a', '2']); + + flushSync(() => { + checkboxes.forEach((checkbox) => checkbox.click()); + }); + assert.deepEqual(logs, ['getBindings', ...repeatArray(3, ['check', false])]); + } +}); + +/** @template T */ +function repeatArray(/** @type {number} */ times = 1, /** @type {T[]} */ array) { + return /** @type {T[]} */ Array.from({ length: times }, () => array).flat(); +} diff --git a/packages/svelte/tests/runtime-runes/samples/bind-spread/main.svelte b/packages/svelte/tests/runtime-runes/samples/bind-spread/main.svelte new file mode 100644 index 0000000000..13646ef83a --- /dev/null +++ b/packages/svelte/tests/runtime-runes/samples/bind-spread/main.svelte @@ -0,0 +1,24 @@ + + + + + + + + + diff --git a/packages/svelte/types/index.d.ts b/packages/svelte/types/index.d.ts index 97e6f0f5a3..90b5e18fb5 100644 --- a/packages/svelte/types/index.d.ts +++ b/packages/svelte/types/index.d.ts @@ -795,7 +795,7 @@ declare module 'svelte/attachments' { declare module 'svelte/compiler' { import type { SourceMap } from 'magic-string'; - import type { ArrayExpression, ArrowFunctionExpression, VariableDeclaration, VariableDeclarator, Expression, Identifier, MemberExpression, Node, ObjectExpression, Pattern, Program, ChainExpression, SimpleCallExpression, SequenceExpression } from 'estree'; + import type { ArrayExpression, ArrowFunctionExpression, VariableDeclaration, VariableDeclarator, Expression, Identifier, MemberExpression, Node, ObjectExpression, Pattern, Program, ChainExpression, SimpleCallExpression, SequenceExpression, SpreadElement } from 'estree'; import type { Location } from 'locate-character'; /** * `compile` converts your `.svelte` source code into a JavaScript module that exports a component @@ -1268,7 +1268,7 @@ declare module 'svelte/compiler' { /** The 'x' in `bind:x` */ name: string; /** The y in `bind:x={y}` */ - expression: Identifier | MemberExpression | SequenceExpression; + expression: Identifier | MemberExpression | SequenceExpression | SpreadElement; } /** A `class:` directive */