From dfcc5f682913a100c38d804be14c370bee3931b2 Mon Sep 17 00:00:00 2001 From: Jack Goodall Date: Tue, 12 Aug 2025 21:58:33 +0200 Subject: [PATCH] use BindDirective metadata to mark a spread element instead of setting the SpreadElement as the expression --- .../compiler/phases/1-parse/state/element.js | 34 ++++++------------ .../2-analyze/visitors/BindDirective.js | 14 ++------ .../client/visitors/BindDirective.js | 3 +- .../client/visitors/RegularElement.js | 2 +- .../client/visitors/shared/component.js | 7 ++-- .../client/visitors/shared/utils.js | 7 ++-- .../server/visitors/shared/component.js | 2 +- .../server/visitors/shared/element.js | 5 ++- .../3-transform/shared/spread_bindings.js | 10 +++--- .../src/compiler/types/legacy-nodes.d.ts | 5 +-- .../svelte/src/compiler/types/template.d.ts | 3 +- packages/svelte/src/internal/shared/errors.js | 36 +++++++++---------- packages/svelte/types/index.d.ts | 4 +-- 13 files changed, 56 insertions(+), 76 deletions(-) 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 731f4b5997..68db2c9df1 100644 --- a/packages/svelte/src/compiler/phases/1-parse/state/element.js +++ b/packages/svelte/src/compiler/phases/1-parse/state/element.js @@ -1,4 +1,4 @@ -/** @import { Expression, SpreadElement } from 'estree' */ +/** @import { Expression } from 'estree' */ /** @import { AST } from '#compiler' */ /** @import { Parser } from '../index.js' */ import { is_void } from '../../../../utils.js'; @@ -618,15 +618,6 @@ 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, @@ -643,7 +634,7 @@ function read_attribute(parser) { const first_value = value === true ? undefined : Array.isArray(value) ? value[0] : value; - /** @type {Expression | SpreadElement | null} */ + /** @type {Expression | null} */ let expression = null; if (first_value) { @@ -655,29 +646,26 @@ 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; - - if (type === 'BindDirective' && first_value.metadata.expression.has_spread) { - expression = { - type: 'SpreadElement', - start: first_value.start, - end: first_value.end, - argument: expression - }; - } } } - /** @type {AST.Directive} */ - const directive = { + const directive = /** @type {AST.Directive} */ ({ start, end, type, name: directive_name, + modifiers: [], expression, metadata: { expression: create_expression_metadata() } - }; + }); + if (first_value?.metadata.expression.has_spread) { + if (directive.type !== 'BindDirective') { + e.directive_invalid_value(first_value.start); + } + directive.metadata.spread_binding = true; + } // @ts-expect-error we do this separately from the declaration to avoid upsetting typescript directive.modifiers = modifiers; 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 a4aa13ad67..a562a0d46d 100644 --- a/packages/svelte/src/compiler/phases/2-analyze/visitors/BindDirective.js +++ b/packages/svelte/src/compiler/phases/2-analyze/visitors/BindDirective.js @@ -158,20 +158,11 @@ export function BindDirective(node, context) { return; } - if (node.expression.type === 'SpreadElement') { + if (node.metadata.spread_binding) { if (node.name === 'group') { e.bind_group_invalid_expression(node); } - 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; @@ -261,7 +252,8 @@ export function BindDirective(node, context) { node.metadata = { binding_group_name: group_name, - parent_each_blocks: each_blocks + parent_each_blocks: each_blocks, + spread_binding: false }; } 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 8fdecd76e1..a55ba2a37a 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 @@ -21,8 +21,7 @@ export function BindDirective(node, context) { let get, set; - // Handle SpreadElement by creating a variable declaration before visiting - if (node.expression.type === 'SpreadElement') { + if (node.metadata.spread_binding) { const { get: getter, set: setter } = init_spread_bindings(node.expression, context); get = getter; set = setter; 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 4296aa959e..c305a0cb6a 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 @@ -414,7 +414,7 @@ function setup_select_synchronization(value_binding, context) { let bound = value_binding.expression; - if (bound.type === 'SequenceExpression') { + if (bound.type === 'SequenceExpression' || value_binding.metadata.spread_binding) { return; } 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 fc2d5b0833..5e678f4332 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 @@ -197,11 +197,14 @@ export function build_component(node, component_name, context) { push_prop(b.init(attribute.name, value)); } } else if (attribute.type === 'BindDirective') { - if (attribute.expression.type === 'SpreadElement') { + if (attribute.metadata.spread_binding) { const { get, set } = init_spread_bindings(attribute.expression, context); if (attribute.name === 'this') { - bind_this = attribute.expression; + bind_this = { + type: 'SpreadElement', + argument: attribute.expression + }; } else { push_prop(b.get(attribute.name, [b.return(b.call(get))]), true); push_prop(b.set(attribute.name, [b.stmt(b.call(set, b.id('$$value')))]), true); 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 c29f70ef98..7a443693d3 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 @@ -212,7 +212,7 @@ export function parse_directive_name(name) { export function build_bind_this(expression, value, context) { const { state, visit } = context; if (expression.type === 'SpreadElement') { - const { get, set } = init_spread_bindings(expression, context); + const { get, set } = init_spread_bindings(expression.argument, context); return b.call('$.bind_this', value, set, get); } @@ -297,10 +297,7 @@ export function build_bind_this(expression, value, context) { * @param {MemberExpression} expression */ export function validate_binding(state, binding, expression) { - if ( - binding.expression.type === 'SequenceExpression' || - binding.expression.type === 'SpreadElement' - ) { + if (binding.expression.type === 'SequenceExpression' || binding.metadata.spread_binding) { return; } // If we are referencing a $store.foo then we don't need to add validation diff --git a/packages/svelte/src/compiler/phases/3-transform/server/visitors/shared/component.js b/packages/svelte/src/compiler/phases/3-transform/server/visitors/shared/component.js index d2ba1091f4..050e5077bc 100644 --- a/packages/svelte/src/compiler/phases/3-transform/server/visitors/shared/component.js +++ b/packages/svelte/src/compiler/phases/3-transform/server/visitors/shared/component.js @@ -94,7 +94,7 @@ export function build_inline_component(node, expression, context) { const value = build_attribute_value(attribute.value, context, false, true); push_prop(b.prop('init', b.key(attribute.name), value)); } else if (attribute.type === 'BindDirective' && attribute.name !== 'this') { - if (attribute.expression.type === 'SpreadElement') { + if (attribute.metadata.spread_binding) { const { get, set } = init_spread_bindings(attribute.expression, context); push_prop(b.get(attribute.name, [b.return(b.call(get))])); diff --git a/packages/svelte/src/compiler/phases/3-transform/server/visitors/shared/element.js b/packages/svelte/src/compiler/phases/3-transform/server/visitors/shared/element.js index 2f9b8ab81e..0bb4ca87a6 100644 --- a/packages/svelte/src/compiler/phases/3-transform/server/visitors/shared/element.js +++ b/packages/svelte/src/compiler/phases/3-transform/server/visitors/shared/element.js @@ -119,8 +119,7 @@ export function build_element_attributes(node, context) { let expression = /** @type {Expression} */ (context.visit(attribute.expression)); - // Handle SpreadElement for bind directives - if (attribute.expression.type === 'SpreadElement') { + if (attribute.metadata.spread_binding) { const { get } = init_spread_bindings(attribute.expression, context); expression = b.call(get); } else if (expression.type === 'SequenceExpression') { @@ -134,7 +133,7 @@ export function build_element_attributes(node, context) { } else if ( attribute.name === 'group' && attribute.expression.type !== 'SequenceExpression' && - attribute.expression.type !== 'SpreadElement' + !attribute.metadata.spread_binding ) { const value_attribute = /** @type {AST.Attribute | undefined} */ ( node.attributes.find((attr) => attr.type === 'Attribute' && attr.name === 'value') diff --git a/packages/svelte/src/compiler/phases/3-transform/shared/spread_bindings.js b/packages/svelte/src/compiler/phases/3-transform/shared/spread_bindings.js index cf496c8b85..6414be66b5 100644 --- a/packages/svelte/src/compiler/phases/3-transform/shared/spread_bindings.js +++ b/packages/svelte/src/compiler/phases/3-transform/shared/spread_bindings.js @@ -1,4 +1,4 @@ -/** @import { Expression, SpreadElement } from 'estree' */ +/** @import { Expression } from 'estree' */ /** @import { ComponentContext as ClientContext } from '../client/types.js' */ /** @import { ComponentContext as ServerContext } from '../server/types.js' */ import * as b from '#compiler/builders'; @@ -6,14 +6,14 @@ import { dev, source } from '../../../state.js'; /** * Initializes spread bindings for a SpreadElement in a bind directive. - * @param {SpreadElement} spread_expression + * @param {Expression} spread_expression * @param {ClientContext | ServerContext} context * @returns {{ get: Expression, set: Expression }} */ export function init_spread_bindings(spread_expression, { state, visit }) { - const visited_expression = /** @type {Expression} */ (visit(spread_expression.argument)); + const expression = /** @type {Expression} */ (visit(spread_expression)); const expression_text = dev - ? b.literal(source.slice(spread_expression.argument.start, spread_expression.argument.end)) + ? b.literal(source.slice(spread_expression.start, spread_expression.end)) : undefined; const id = state.scope.generate('$$spread_binding'); @@ -22,7 +22,7 @@ export function init_spread_bindings(spread_expression, { state, visit }) { state.init.push( b.const( b.array_pattern([get, set]), - b.call('$.validate_spread_bindings', visited_expression, expression_text) + b.call('$.validate_spread_bindings', expression, expression_text) ) ); diff --git a/packages/svelte/src/compiler/types/legacy-nodes.d.ts b/packages/svelte/src/compiler/types/legacy-nodes.d.ts index 389fc92332..259320ca40 100644 --- a/packages/svelte/src/compiler/types/legacy-nodes.d.ts +++ b/packages/svelte/src/compiler/types/legacy-nodes.d.ts @@ -7,7 +7,8 @@ import type { MemberExpression, ObjectExpression, Pattern, - SequenceExpression + SequenceExpression, + SpreadElement } from 'estree'; interface BaseNode { @@ -50,7 +51,7 @@ export interface LegacyBinding extends BaseNode { /** The 'x' in `bind:x` */ name: string; /** The y in `bind:x={y}` */ - expression: Identifier | MemberExpression | SequenceExpression; + expression: Identifier | MemberExpression | SequenceExpression | SpreadElement; } export interface LegacyBody extends BaseElement { diff --git a/packages/svelte/src/compiler/types/template.d.ts b/packages/svelte/src/compiler/types/template.d.ts index b3ae4fae5f..a48a7c15e7 100644 --- a/packages/svelte/src/compiler/types/template.d.ts +++ b/packages/svelte/src/compiler/types/template.d.ts @@ -212,11 +212,12 @@ export namespace AST { /** The 'x' in `bind:x` */ name: string; /** The y in `bind:x={y}` */ - expression: Identifier | MemberExpression | SequenceExpression | SpreadElement; + expression: Identifier | MemberExpression | SequenceExpression; /** @internal */ metadata: { binding_group_name: Identifier; parent_each_blocks: EachBlock[]; + spread_binding: boolean; }; } diff --git a/packages/svelte/src/internal/shared/errors.js b/packages/svelte/src/internal/shared/errors.js index ac06e510f0..a2056dd071 100644 --- a/packages/svelte/src/internal/shared/errors.js +++ b/packages/svelte/src/internal/shared/errors.js @@ -50,6 +50,23 @@ export function invalid_snippet_arguments() { } } +/** + * `%name%` must be a function or `undefined` + * @param {string} name + * @returns {never} + */ +export function invalid_spread_bindings(name) { + if (DEV) { + const error = new Error(`invalid_spread_bindings\n\`${name}\` must be a function or \`undefined\`\nhttps://svelte.dev/e/invalid_spread_bindings`); + + error.name = 'Svelte error'; + + throw error; + } else { + throw new Error(`https://svelte.dev/e/invalid_spread_bindings`); + } +} + /** * `%name%(...)` can only be used during component initialisation * @param {string} name @@ -114,21 +131,4 @@ export function svelte_element_invalid_this_value() { } else { throw new Error(`https://svelte.dev/e/svelte_element_invalid_this_value`); } -} - -/** - * `%name%%member%` must be a function or `undefined` - * @param {string} name - * @returns {never} - */ -export function invalid_spread_bindings(name) { - if (DEV) { - const error = new Error(`invalid_spread_bindings\n\`${name}\` must be a function or \`undefined\`\nhttps://svelte.dev/e/invalid_spread_bindings`); - - error.name = 'Svelte error'; - - throw error; - } else { - throw new Error(`https://svelte.dev/e/invalid_spread_bindings`); - } -} +} \ No newline at end of file diff --git a/packages/svelte/types/index.d.ts b/packages/svelte/types/index.d.ts index 90b5e18fb5..97e6f0f5a3 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, SpreadElement } from 'estree'; + import type { ArrayExpression, ArrowFunctionExpression, VariableDeclaration, VariableDeclarator, Expression, Identifier, MemberExpression, Node, ObjectExpression, Pattern, Program, ChainExpression, SimpleCallExpression, SequenceExpression } 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 | SpreadElement; + expression: Identifier | MemberExpression | SequenceExpression; } /** A `class:` directive */