From 5771b455c0caf860bb063499feb2100acad4fbd5 Mon Sep 17 00:00:00 2001 From: Dominic Gannaway Date: Sun, 8 Dec 2024 12:28:37 +0000 Subject: [PATCH] feat: add support for bind getter/setters (#14307) * feat: add support for bind getters/setters * different direction * oops * oops * build * add changeset and tests * move validation * add comment * build * bind:group error * simpler to just keep it as a SequenceExpression * fix * lint * fix * move validation to visitor * fix * no longer needed * fix * parser changes are no longer needed * simplify * simplify * update messages * docs --------- Co-authored-by: Rich Harris Co-authored-by: Simon Holthausen --- .changeset/slimy-donkeys-hang.md | 5 + .../docs/03-template-syntax/11-bind.md | 24 ++ .../98-reference/.generated/compile-errors.md | 14 +- .../messages/compile-errors/template.md | 10 +- packages/svelte/src/compiler/errors.js | 23 +- .../2-analyze/visitors/BindDirective.js | 214 ++++++++++-------- .../client/visitors/BindDirective.js | 74 +++--- .../client/visitors/RegularElement.js | 10 +- .../client/visitors/shared/component.js | 113 +++++---- .../client/visitors/shared/utils.js | 12 +- .../server/visitors/shared/component.js | 52 +++-- .../server/visitors/shared/element.js | 18 +- .../src/compiler/types/legacy-nodes.d.ts | 5 +- .../svelte/src/compiler/types/template.d.ts | 5 +- .../samples/bind-getter-setter-2/Child.svelte | 11 + .../samples/bind-getter-setter-2/_config.js | 9 + .../samples/bind-getter-setter-2/main.svelte | 11 + .../samples/bind-getter-setter/Child.svelte | 12 + .../samples/bind-getter-setter/_config.js | 20 ++ .../samples/bind-getter-setter/main.svelte | 16 ++ .../bind_group_invalid_expression/errors.json | 14 ++ .../input.svelte | 12 + packages/svelte/types/index.d.ts | 4 +- 23 files changed, 471 insertions(+), 217 deletions(-) create mode 100644 .changeset/slimy-donkeys-hang.md create mode 100644 packages/svelte/tests/runtime-runes/samples/bind-getter-setter-2/Child.svelte create mode 100644 packages/svelte/tests/runtime-runes/samples/bind-getter-setter-2/_config.js create mode 100644 packages/svelte/tests/runtime-runes/samples/bind-getter-setter-2/main.svelte create mode 100644 packages/svelte/tests/runtime-runes/samples/bind-getter-setter/Child.svelte create mode 100644 packages/svelte/tests/runtime-runes/samples/bind-getter-setter/_config.js create mode 100644 packages/svelte/tests/runtime-runes/samples/bind-getter-setter/main.svelte create mode 100644 packages/svelte/tests/validator/samples/bind_group_invalid_expression/errors.json create mode 100644 packages/svelte/tests/validator/samples/bind_group_invalid_expression/input.svelte diff --git a/.changeset/slimy-donkeys-hang.md b/.changeset/slimy-donkeys-hang.md new file mode 100644 index 0000000000..d63141660e --- /dev/null +++ b/.changeset/slimy-donkeys-hang.md @@ -0,0 +1,5 @@ +--- +'svelte': patch +--- + +feat: add support for bind getters/setters diff --git a/documentation/docs/03-template-syntax/11-bind.md b/documentation/docs/03-template-syntax/11-bind.md index fe3cf727e2..90046c8c45 100644 --- a/documentation/docs/03-template-syntax/11-bind.md +++ b/documentation/docs/03-template-syntax/11-bind.md @@ -12,10 +12,34 @@ The general syntax is `bind:property={expression}`, where `expression` is an _lv ``` + Svelte creates an event listener that updates the bound value. If an element already has a listener for the same event, that listener will be fired before the bound value is updated. Most bindings are _two-way_, meaning that changes to the value will affect the element and vice versa. A few bindings are _readonly_, meaning that changing their value will have no effect on the element. +## Function bindings + +You can also use `bind:property={get, set}`, where `get` and `set` are functions, allowing you to perform validation and transformation: + +```svelte + value, + (v) => value = v.toLowerCase()} +/> +``` + +In the case of readonly bindings like [dimension bindings](#Dimensions), the `get` value should be `null`: + +```svelte +
...
+``` + +> [!NOTE] +> Function bindings are available in Svelte 5.9.0 and newer. + ## `` A `bind:value` directive on an `` element binds the input's `value` property: diff --git a/documentation/docs/98-reference/.generated/compile-errors.md b/documentation/docs/98-reference/.generated/compile-errors.md index 3bd162d8d7..d726d25fa1 100644 --- a/documentation/docs/98-reference/.generated/compile-errors.md +++ b/documentation/docs/98-reference/.generated/compile-errors.md @@ -78,10 +78,16 @@ Sequence expressions are not allowed as attribute/directive values in runes mode Attribute values containing `{...}` must be enclosed in quote marks, unless the value only contains the expression ``` +### bind_group_invalid_expression + +``` +`bind:group` can only bind to an Identifier or MemberExpression +``` + ### bind_invalid_expression ``` -Can only bind to an Identifier or MemberExpression +Can only bind to an Identifier or MemberExpression or a `{get, set}` pair ``` ### bind_invalid_name @@ -94,6 +100,12 @@ Can only bind to an Identifier or MemberExpression `bind:%name%` is not a valid binding. %explanation% ``` +### bind_invalid_parens + +``` +`bind:%name%={get, set}` must not have surrounding parentheses +``` + ### bind_invalid_target ``` diff --git a/packages/svelte/messages/compile-errors/template.md b/packages/svelte/messages/compile-errors/template.md index 9621a6457b..02961b61fc 100644 --- a/packages/svelte/messages/compile-errors/template.md +++ b/packages/svelte/messages/compile-errors/template.md @@ -50,9 +50,13 @@ > Attribute values containing `{...}` must be enclosed in quote marks, unless the value only contains the expression +## bind_group_invalid_expression + +> `bind:group` can only bind to an Identifier or MemberExpression + ## bind_invalid_expression -> Can only bind to an Identifier or MemberExpression +> Can only bind to an Identifier or MemberExpression or a `{get, set}` pair ## bind_invalid_name @@ -60,6 +64,10 @@ > `bind:%name%` is not a valid binding. %explanation% +## bind_invalid_parens + +> `bind:%name%={get, set}` must not have surrounding parentheses + ## bind_invalid_target > `bind:%name%` can only be used with %elements% diff --git a/packages/svelte/src/compiler/errors.js b/packages/svelte/src/compiler/errors.js index 901ea1983e..1a4525ef5c 100644 --- a/packages/svelte/src/compiler/errors.js +++ b/packages/svelte/src/compiler/errors.js @@ -716,12 +716,21 @@ export function attribute_unquoted_sequence(node) { } /** - * Can only bind to an Identifier or MemberExpression + * `bind:group` can only bind to an Identifier or MemberExpression + * @param {null | number | NodeLike} node + * @returns {never} + */ +export function bind_group_invalid_expression(node) { + e(node, "bind_group_invalid_expression", "`bind:group` can only bind to an Identifier or MemberExpression"); +} + +/** + * Can only bind to an Identifier or MemberExpression or a `{get, set}` pair * @param {null | number | NodeLike} node * @returns {never} */ export function bind_invalid_expression(node) { - e(node, "bind_invalid_expression", "Can only bind to an Identifier or MemberExpression"); + e(node, "bind_invalid_expression", "Can only bind to an Identifier or MemberExpression or a `{get, set}` pair"); } /** @@ -735,6 +744,16 @@ export function bind_invalid_name(node, name, explanation) { e(node, "bind_invalid_name", explanation ? `\`bind:${name}\` is not a valid binding. ${explanation}` : `\`bind:${name}\` is not a valid binding`); } +/** + * `bind:%name%={get, set}` must not have surrounding parentheses + * @param {null | number | NodeLike} node + * @param {string} name + * @returns {never} + */ +export function bind_invalid_parens(node, name) { + e(node, "bind_invalid_parens", `\`bind:${name}={get, set}\` must not have surrounding parentheses`); +} + /** * `bind:%name%` can only be used with %elements% * @param {null | number | NodeLike} node 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 5b56d9ddac..b062365380 100644 --- a/packages/svelte/src/compiler/phases/2-analyze/visitors/BindDirective.js +++ b/packages/svelte/src/compiler/phases/2-analyze/visitors/BindDirective.js @@ -17,102 +17,6 @@ import { is_content_editable_binding, is_svg } from '../../../../utils.js'; * @param {Context} context */ export function BindDirective(node, context) { - validate_no_const_assignment(node, node.expression, context.state.scope, true); - - const assignee = node.expression; - const left = object(assignee); - - if (left === null) { - e.bind_invalid_expression(node); - } - - const binding = context.state.scope.get(left.name); - - if (assignee.type === 'Identifier') { - // reassignment - if ( - node.name !== 'this' && // bind:this also works for regular variables - (!binding || - (binding.kind !== 'state' && - binding.kind !== 'raw_state' && - binding.kind !== 'prop' && - binding.kind !== 'bindable_prop' && - binding.kind !== 'each' && - binding.kind !== 'store_sub' && - !binding.updated)) // TODO wut? - ) { - e.bind_invalid_value(node.expression); - } - - if (context.state.analysis.runes && binding?.kind === 'each') { - e.each_item_invalid_assignment(node); - } - - if (binding?.kind === 'snippet') { - e.snippet_parameter_assignment(node); - } - } - - if (node.name === 'group') { - if (!binding) { - throw new Error('Cannot find declaration for bind:group'); - } - - // Traverse the path upwards and find all EachBlocks who are (indirectly) contributing to bind:group, - // i.e. one of their declarations is referenced in the binding. This allows group bindings to work - // correctly when referencing a variable declared in an EachBlock by using the index of the each block - // entries as keys. - const each_blocks = []; - const [keypath, expression_ids] = extract_all_identifiers_from_expression(node.expression); - let ids = expression_ids; - - let i = context.path.length; - while (i--) { - const parent = context.path[i]; - - if (parent.type === 'EachBlock') { - const references = ids.filter((id) => parent.metadata.declarations.has(id.name)); - - if (references.length > 0) { - parent.metadata.contains_group_binding = true; - - each_blocks.push(parent); - ids = ids.filter((id) => !references.includes(id)); - ids.push(...extract_all_identifiers_from_expression(parent.expression)[1]); - } - } - } - - // The identifiers that make up the binding expression form they key for the binding group. - // If the same identifiers in the same order are used in another bind:group, they will be in the same group. - // (there's an edge case where `bind:group={a[i]}` will be in a different group than `bind:group={a[j]}` even when i == j, - // but this is a limitation of the current static analysis we do; it also never worked in Svelte 4) - const bindings = expression_ids.map((id) => context.state.scope.get(id.name)); - let group_name; - - outer: for (const [[key, b], group] of context.state.analysis.binding_groups) { - if (b.length !== bindings.length || key !== keypath) continue; - for (let i = 0; i < bindings.length; i++) { - if (bindings[i] !== b[i]) continue outer; - } - group_name = group; - } - - if (!group_name) { - group_name = context.state.scope.root.unique('binding_group'); - context.state.analysis.binding_groups.set([keypath, bindings], group_name); - } - - node.metadata = { - binding_group_name: group_name, - parent_each_blocks: each_blocks - }; - } - - if (binding?.kind === 'each' && binding.metadata?.inside_rest) { - w.bind_invalid_each_rest(binding.node, binding.node.name); - } - const parent = context.path.at(-1); if ( @@ -218,5 +122,123 @@ export function BindDirective(node, context) { } } + // When dealing with bind getters/setters skip the specific binding validation + // Group bindings aren't supported for getter/setters so we don't need to handle + // the metadata + if (node.expression.type === 'SequenceExpression') { + if (node.name === 'group') { + e.bind_group_invalid_expression(node); + } + + let i = /** @type {number} */ (node.expression.start); + while (context.state.analysis.source[--i] !== '{') { + if (context.state.analysis.source[i] === '(') { + e.bind_invalid_parens(node, node.name); + } + } + + if (node.expression.expressions.length !== 2) { + e.bind_invalid_expression(node); + } + + return; + } + + validate_no_const_assignment(node, node.expression, context.state.scope, true); + + const assignee = node.expression; + const left = object(assignee); + + if (left === null) { + e.bind_invalid_expression(node); + } + + const binding = context.state.scope.get(left.name); + + if (assignee.type === 'Identifier') { + // reassignment + if ( + node.name !== 'this' && // bind:this also works for regular variables + (!binding || + (binding.kind !== 'state' && + binding.kind !== 'raw_state' && + binding.kind !== 'prop' && + binding.kind !== 'bindable_prop' && + binding.kind !== 'each' && + binding.kind !== 'store_sub' && + !binding.updated)) // TODO wut? + ) { + e.bind_invalid_value(node.expression); + } + + if (context.state.analysis.runes && binding?.kind === 'each') { + e.each_item_invalid_assignment(node); + } + + if (binding?.kind === 'snippet') { + e.snippet_parameter_assignment(node); + } + } + + if (node.name === 'group') { + if (!binding) { + throw new Error('Cannot find declaration for bind:group'); + } + + // Traverse the path upwards and find all EachBlocks who are (indirectly) contributing to bind:group, + // i.e. one of their declarations is referenced in the binding. This allows group bindings to work + // correctly when referencing a variable declared in an EachBlock by using the index of the each block + // entries as keys. + const each_blocks = []; + const [keypath, expression_ids] = extract_all_identifiers_from_expression(node.expression); + let ids = expression_ids; + + let i = context.path.length; + while (i--) { + const parent = context.path[i]; + + if (parent.type === 'EachBlock') { + const references = ids.filter((id) => parent.metadata.declarations.has(id.name)); + + if (references.length > 0) { + parent.metadata.contains_group_binding = true; + + each_blocks.push(parent); + ids = ids.filter((id) => !references.includes(id)); + ids.push(...extract_all_identifiers_from_expression(parent.expression)[1]); + } + } + } + + // The identifiers that make up the binding expression form they key for the binding group. + // If the same identifiers in the same order are used in another bind:group, they will be in the same group. + // (there's an edge case where `bind:group={a[i]}` will be in a different group than `bind:group={a[j]}` even when i == j, + // but this is a limitation of the current static analysis we do; it also never worked in Svelte 4) + const bindings = expression_ids.map((id) => context.state.scope.get(id.name)); + let group_name; + + outer: for (const [[key, b], group] of context.state.analysis.binding_groups) { + if (b.length !== bindings.length || key !== keypath) continue; + for (let i = 0; i < bindings.length; i++) { + if (bindings[i] !== b[i]) continue outer; + } + group_name = group; + } + + if (!group_name) { + group_name = context.state.scope.root.unique('binding_group'); + context.state.analysis.binding_groups.set([keypath, bindings], group_name); + } + + node.metadata = { + binding_group_name: group_name, + parent_each_blocks: each_blocks + }; + } + + if (binding?.kind === 'each' && binding.metadata?.inside_rest) { + w.bind_invalid_each_rest(binding.node, binding.node.name); + } + context.next(); } 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 79969240c7..f129e059f2 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 @@ -1,4 +1,4 @@ -/** @import { CallExpression, Expression, MemberExpression } from 'estree' */ +/** @import { CallExpression, Expression, MemberExpression, Pattern } from 'estree' */ /** @import { AST, SvelteNode } from '#compiler' */ /** @import { ComponentContext } from '../types' */ import { dev, is_ignored } from '../../../../state.js'; @@ -13,41 +13,50 @@ import { build_bind_this, validate_binding } from './shared/utils.js'; * @param {ComponentContext} context */ export function BindDirective(node, context) { - const expression = node.expression; + const expression = /** @type {Expression} */ (context.visit(node.expression)); const property = binding_properties[node.name]; const parent = /** @type {SvelteNode} */ (context.path.at(-1)); - 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, - /**@type {MemberExpression} */ (context.visit(expression)) - ); - } + let get, set; - const get = b.thunk(/** @type {Expression} */ (context.visit(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} */ - let set = b.unthunk( - b.arrow( - [b.id('$$value')], - /** @type {Expression} */ (context.visit(b.assignment('=', expression, b.id('$$value')))) - ) - ); + get = b.thunk(expression); - if (get === set) { - set = undefined; + /** @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; + } } /** @type {CallExpression} */ @@ -162,7 +171,7 @@ export function BindDirective(node, context) { break; case 'this': - call = build_bind_this(expression, context.state.node, context); + call = build_bind_this(node.expression, context.state.node, context); break; case 'textContent': @@ -213,10 +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(/** @type {Expression} */ (context.visit(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 3c0be589c3..2c2c287f12 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 @@ -450,6 +450,11 @@ function setup_select_synchronization(value_binding, context) { if (context.state.analysis.runes) return; let bound = value_binding.expression; + + if (bound.type === 'SequenceExpression') { + return; + } + while (bound.type === 'MemberExpression') { bound = /** @type {Identifier | MemberExpression} */ (bound.object); } @@ -484,10 +489,7 @@ function setup_select_synchronization(value_binding, context) { b.call( '$.template_effect', b.thunk( - b.block([ - b.stmt(/** @type {Expression} */ (context.visit(value_binding.expression))), - b.stmt(invalidator) - ]) + b.block([b.stmt(/** @type {Expression} */ (context.visit(bound))), b.stmt(invalidator)]) ) ) ) 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 aa7be93cb5..c94c1e1b0e 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 @@ -1,4 +1,4 @@ -/** @import { BlockStatement, Expression, ExpressionStatement, Identifier, MemberExpression, Property, Statement } from 'estree' */ +/** @import { BlockStatement, Expression, ExpressionStatement, Identifier, MemberExpression, Pattern, Property, SequenceExpression, Statement } from 'estree' */ /** @import { AST, TemplateNode } from '#compiler' */ /** @import { ComponentContext } from '../../types.js' */ import { dev, is_ignored } from '../../../../../state.js'; @@ -44,7 +44,7 @@ export function build_component(node, component_name, context, anchor = context. /** @type {Property[]} */ const custom_css_props = []; - /** @type {Identifier | MemberExpression | null} */ + /** @type {Identifier | MemberExpression | SequenceExpression | null} */ let bind_this = null; /** @type {ExpressionStatement[]} */ @@ -174,60 +174,83 @@ export function build_component(node, component_name, context, anchor = context. } else if (attribute.type === 'BindDirective') { const expression = /** @type {Expression} */ (context.visit(attribute.expression)); - if ( - dev && - expression.type === 'MemberExpression' && - context.state.analysis.runes && - !is_ignored(node, 'binding_property_non_reactive') - ) { - validate_binding(context.state, attribute, expression); + if (dev && attribute.name !== 'this' && attribute.expression.type !== 'SequenceExpression') { + const left = object(attribute.expression); + let binding; + + if (left?.type === 'Identifier') { + binding = context.state.scope.get(left.name); + } + + // Only run ownership addition on $state fields. + // Theoretically someone could create a `$state` while creating `$state.raw` or inside a `$derived.by`, + // but that feels so much of an edge case that it doesn't warrant a perf hit for the common case. + if (binding?.kind !== 'derived' && binding?.kind !== 'raw_state') { + binding_initializers.push( + b.stmt( + b.call( + b.id('$.add_owner_effect'), + b.thunk(expression), + b.id(component_name), + is_ignored(node, 'ownership_invalid_binding') && b.true + ) + ) + ); + } } - if (attribute.name === 'this') { - bind_this = attribute.expression; + if (expression.type === 'SequenceExpression') { + if (attribute.name === 'this') { + bind_this = attribute.expression; + } else { + const [get, set] = expression.expressions; + const get_id = b.id(context.state.scope.generate('bind_get')); + const set_id = b.id(context.state.scope.generate('bind_set')); + + context.state.init.push(b.var(get_id, get)); + context.state.init.push(b.var(set_id, set)); + + push_prop(b.get(attribute.name, [b.return(b.call(get_id))])); + push_prop(b.set(attribute.name, [b.stmt(b.call(set_id, b.id('$$value')))])); + } } else { - if (dev) { - const left = object(attribute.expression); - let binding; - if (left?.type === 'Identifier') { - binding = context.state.scope.get(left.name); - } - // Only run ownership addition on $state fields. - // Theoretically someone could create a `$state` while creating `$state.raw` or inside a `$derived.by`, - // but that feels so much of an edge case that it doesn't warrant a perf hit for the common case. - if (binding?.kind !== 'derived' && binding?.kind !== 'raw_state') { - binding_initializers.push( - b.stmt( - b.call( - b.id('$.add_owner_effect'), - b.thunk(expression), - b.id(component_name), - is_ignored(node, 'ownership_invalid_binding') && b.true - ) - ) + if ( + dev && + expression.type === 'MemberExpression' && + context.state.analysis.runes && + !is_ignored(node, 'binding_property_non_reactive') + ) { + validate_binding(context.state, attribute, expression); + } + + if (attribute.name === 'this') { + bind_this = attribute.expression; + } else { + const is_store_sub = + attribute.expression.type === 'Identifier' && + context.state.scope.get(attribute.expression.name)?.kind === 'store_sub'; + + // Delay prop pushes so bindings come at the end, to avoid spreads overwriting them + if (is_store_sub) { + push_prop( + b.get(attribute.name, [b.stmt(b.call('$.mark_store_binding')), b.return(expression)]), + true ); + } else { + push_prop(b.get(attribute.name, [b.return(expression)]), true); } - } - const is_store_sub = - attribute.expression.type === 'Identifier' && - context.state.scope.get(attribute.expression.name)?.kind === 'store_sub'; + const assignment = b.assignment( + '=', + /** @type {Pattern} */ (attribute.expression), + b.id('$$value') + ); - // Delay prop pushes so bindings come at the end, to avoid spreads overwriting them - if (is_store_sub) { push_prop( - b.get(attribute.name, [b.stmt(b.call('$.mark_store_binding')), b.return(expression)]), + b.set(attribute.name, [b.stmt(/** @type {Expression} */ (context.visit(assignment)))]), true ); - } else { - push_prop(b.get(attribute.name, [b.return(expression)]), true); } - - const assignment = b.assignment('=', attribute.expression, b.id('$$value')); - push_prop( - b.set(attribute.name, [b.stmt(/** @type {Expression} */ (context.visit(assignment)))]), - 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 59beacbb0c..11f76aa025 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 { Expression, ExpressionStatement, Identifier, MemberExpression, Statement, Super } from 'estree' */ +/** @import { Expression, ExpressionStatement, Identifier, MemberExpression, SequenceExpression, Statement, Super } from 'estree' */ /** @import { AST, SvelteNode } from '#compiler' */ /** @import { ComponentClientTransformState } from '../../types' */ import { walk } from 'zimmerframe'; @@ -143,11 +143,16 @@ export function build_update_assignment(state, id, init, value, update) { /** * Serializes `bind:this` for components and elements. - * @param {Identifier | MemberExpression} expression + * @param {Identifier | MemberExpression | SequenceExpression} expression * @param {Expression} value * @param {import('zimmerframe').Context} context */ export function build_bind_this(expression, value, { state, visit }) { + if (expression.type === 'SequenceExpression') { + const [get, set] = /** @type {SequenceExpression} */ (visit(expression)).expressions; + return b.call('$.bind_this', value, set, get); + } + /** @type {Identifier[]} */ const ids = []; @@ -224,6 +229,9 @@ export function build_bind_this(expression, value, { state, visit }) { * @param {MemberExpression} expression */ export function validate_binding(state, binding, expression) { + if (binding.expression.type === 'SequenceExpression') { + return; + } // If we are referencing a $store.foo then we don't need to add validation const left = object(binding.expression); const left_binding = left && state.scope.get(left.name); 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 7cabfb06c5..0d04444335 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 @@ -1,4 +1,4 @@ -/** @import { BlockStatement, Expression, Pattern, Property, Statement } from 'estree' */ +/** @import { BlockStatement, Expression, Pattern, Property, SequenceExpression, Statement } from 'estree' */ /** @import { AST, TemplateNode } from '#compiler' */ /** @import { ComponentContext } from '../../types.js' */ import { empty_comment, build_attribute_value } from './utils.js'; @@ -92,24 +92,38 @@ 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') { - // Delay prop pushes so bindings come at the end, to avoid spreads overwriting them - push_prop( - b.get(attribute.name, [ - b.return(/** @type {Expression} */ (context.visit(attribute.expression))) - ]), - true - ); - push_prop( - b.set(attribute.name, [ - b.stmt( - /** @type {Expression} */ ( - context.visit(b.assignment('=', attribute.expression, b.id('$$value'))) - ) - ), - b.stmt(b.assignment('=', b.id('$$settled'), b.false)) - ]), - true - ); + if (attribute.expression.type === 'SequenceExpression') { + const [get, set] = /** @type {SequenceExpression} */ (context.visit(attribute.expression)) + .expressions; + const get_id = b.id(context.state.scope.generate('bind_get')); + const set_id = b.id(context.state.scope.generate('bind_set')); + + context.state.init.push(b.var(get_id, get)); + context.state.init.push(b.var(set_id, set)); + + push_prop(b.get(attribute.name, [b.return(b.call(get_id))])); + push_prop(b.set(attribute.name, [b.stmt(b.call(set_id, b.id('$$value')))])); + } else { + // Delay prop pushes so bindings come at the end, to avoid spreads overwriting them + push_prop( + b.get(attribute.name, [ + b.return(/** @type {Expression} */ (context.visit(attribute.expression))) + ]), + true + ); + + push_prop( + b.set(attribute.name, [ + b.stmt( + /** @type {Expression} */ ( + context.visit(b.assignment('=', attribute.expression, b.id('$$value'))) + ) + ), + b.stmt(b.assignment('=', b.id('$$settled'), b.false)) + ]), + true + ); + } } } 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 434447727b..d626bb08db 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 @@ -110,14 +110,17 @@ export function build_element_attributes(node, context) { const binding = binding_properties[attribute.name]; if (binding?.omit_in_ssr) continue; + let expression = /** @type {Expression} */ (context.visit(attribute.expression)); + + if (expression.type === 'SequenceExpression') { + expression = b.call(expression.expressions[0]); + } + if (is_content_editable_binding(attribute.name)) { - content = /** @type {Expression} */ (context.visit(attribute.expression)); + content = expression; } else if (attribute.name === 'value' && node.name === 'textarea') { - content = b.call( - '$.escape', - /** @type {Expression} */ (context.visit(attribute.expression)) - ); - } else if (attribute.name === 'group') { + content = b.call('$.escape', expression); + } else if (attribute.name === 'group' && attribute.expression.type !== 'SequenceExpression') { const value_attribute = /** @type {AST.Attribute | undefined} */ ( node.attributes.find((attr) => attr.type === 'Attribute' && attr.name === 'value') ); @@ -130,6 +133,7 @@ export function build_element_attributes(node, context) { is_text_attribute(attr) && attr.value[0].data === 'checkbox' ); + attributes.push( create_attribute('checked', -1, -1, [ { @@ -159,7 +163,7 @@ export function build_element_attributes(node, context) { type: 'ExpressionTag', start: -1, end: -1, - expression: attribute.expression, + expression, metadata: { expression: create_expression_metadata() } diff --git a/packages/svelte/src/compiler/types/legacy-nodes.d.ts b/packages/svelte/src/compiler/types/legacy-nodes.d.ts index 2bd5fbbfa6..0013f5c17a 100644 --- a/packages/svelte/src/compiler/types/legacy-nodes.d.ts +++ b/packages/svelte/src/compiler/types/legacy-nodes.d.ts @@ -6,7 +6,8 @@ import type { Identifier, MemberExpression, ObjectExpression, - Pattern + Pattern, + SequenceExpression } from 'estree'; interface BaseNode { @@ -49,7 +50,7 @@ export interface LegacyBinding extends BaseNode { /** The 'x' in `bind:x` */ name: string; /** The y in `bind:x={y}` */ - expression: Identifier | MemberExpression; + expression: Identifier | MemberExpression | SequenceExpression; } 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 a409cf5704..b8724f28dc 100644 --- a/packages/svelte/src/compiler/types/template.d.ts +++ b/packages/svelte/src/compiler/types/template.d.ts @@ -14,7 +14,8 @@ import type { Pattern, Program, ChainExpression, - SimpleCallExpression + SimpleCallExpression, + SequenceExpression } from 'estree'; import type { Scope } from '../phases/scope'; @@ -187,7 +188,7 @@ export namespace AST { /** The 'x' in `bind:x` */ name: string; /** The y in `bind:x={y}` */ - expression: Identifier | MemberExpression; + expression: Identifier | MemberExpression | SequenceExpression; /** @internal */ metadata: { binding_group_name: Identifier; diff --git a/packages/svelte/tests/runtime-runes/samples/bind-getter-setter-2/Child.svelte b/packages/svelte/tests/runtime-runes/samples/bind-getter-setter-2/Child.svelte new file mode 100644 index 0000000000..0026309d44 --- /dev/null +++ b/packages/svelte/tests/runtime-runes/samples/bind-getter-setter-2/Child.svelte @@ -0,0 +1,11 @@ + + +
div, v => div = v}>123
diff --git a/packages/svelte/tests/runtime-runes/samples/bind-getter-setter-2/_config.js b/packages/svelte/tests/runtime-runes/samples/bind-getter-setter-2/_config.js new file mode 100644 index 0000000000..1d51c8eead --- /dev/null +++ b/packages/svelte/tests/runtime-runes/samples/bind-getter-setter-2/_config.js @@ -0,0 +1,9 @@ +import { test } from '../../test'; + +export default test({ + async test({ assert, target, logs }) { + assert.htmlEqual(target.innerHTML, `
123
`); + + assert.deepEqual(logs, ['123', '123']); + } +}); diff --git a/packages/svelte/tests/runtime-runes/samples/bind-getter-setter-2/main.svelte b/packages/svelte/tests/runtime-runes/samples/bind-getter-setter-2/main.svelte new file mode 100644 index 0000000000..21646e745a --- /dev/null +++ b/packages/svelte/tests/runtime-runes/samples/bind-getter-setter-2/main.svelte @@ -0,0 +1,11 @@ + + + child, v => child = v} /> diff --git a/packages/svelte/tests/runtime-runes/samples/bind-getter-setter/Child.svelte b/packages/svelte/tests/runtime-runes/samples/bind-getter-setter/Child.svelte new file mode 100644 index 0000000000..bea5849ec7 --- /dev/null +++ b/packages/svelte/tests/runtime-runes/samples/bind-getter-setter/Child.svelte @@ -0,0 +1,12 @@ + + + a, + (v) => { + console.log('b', v); + a = v; + }} +/> diff --git a/packages/svelte/tests/runtime-runes/samples/bind-getter-setter/_config.js b/packages/svelte/tests/runtime-runes/samples/bind-getter-setter/_config.js new file mode 100644 index 0000000000..158d1e6f63 --- /dev/null +++ b/packages/svelte/tests/runtime-runes/samples/bind-getter-setter/_config.js @@ -0,0 +1,20 @@ +import { flushSync } from 'svelte'; +import { test } from '../../test'; +import { assert_ok } from '../../../suite'; + +export default test({ + async test({ assert, target, logs }) { + const input = target.querySelector('input'); + + assert_ok(input); + + input.value = '2'; + input.dispatchEvent(new window.Event('input')); + + flushSync(); + + assert.htmlEqual(target.innerHTML, ``); + + assert.deepEqual(logs, ['b', '2', 'a', '2']); + } +}); diff --git a/packages/svelte/tests/runtime-runes/samples/bind-getter-setter/main.svelte b/packages/svelte/tests/runtime-runes/samples/bind-getter-setter/main.svelte new file mode 100644 index 0000000000..191a423476 --- /dev/null +++ b/packages/svelte/tests/runtime-runes/samples/bind-getter-setter/main.svelte @@ -0,0 +1,16 @@ + + + + + a, + (v) => { + console.log('a', v); + a = v; + }} +/> + diff --git a/packages/svelte/tests/validator/samples/bind_group_invalid_expression/errors.json b/packages/svelte/tests/validator/samples/bind_group_invalid_expression/errors.json new file mode 100644 index 0000000000..f85363106b --- /dev/null +++ b/packages/svelte/tests/validator/samples/bind_group_invalid_expression/errors.json @@ -0,0 +1,14 @@ +[ + { + "code": "bind_group_invalid_expression", + "message": "`bind:group` can only bind to an Identifier or MemberExpression", + "start": { + "line": 8, + "column": 38 + }, + "end": { + "line": 8, + "column": 84 + } + } +] diff --git a/packages/svelte/tests/validator/samples/bind_group_invalid_expression/input.svelte b/packages/svelte/tests/validator/samples/bind_group_invalid_expression/input.svelte new file mode 100644 index 0000000000..3f8afe7655 --- /dev/null +++ b/packages/svelte/tests/validator/samples/bind_group_invalid_expression/input.svelte @@ -0,0 +1,12 @@ + + +{#each values as value} + +{/each} + +

{selected.name}

diff --git a/packages/svelte/types/index.d.ts b/packages/svelte/types/index.d.ts index 0d761919a8..61a34dcb8e 100644 --- a/packages/svelte/types/index.d.ts +++ b/packages/svelte/types/index.d.ts @@ -606,7 +606,7 @@ declare module 'svelte/animate' { } declare module 'svelte/compiler' { - import type { Expression, Identifier, ArrayExpression, ArrowFunctionExpression, VariableDeclaration, VariableDeclarator, MemberExpression, ObjectExpression, Pattern, Program, ChainExpression, SimpleCallExpression } from 'estree'; + import type { Expression, Identifier, ArrayExpression, ArrowFunctionExpression, VariableDeclaration, VariableDeclarator, MemberExpression, ObjectExpression, Pattern, Program, ChainExpression, SimpleCallExpression, SequenceExpression } from 'estree'; import type { SourceMap } from 'magic-string'; import type { Location } from 'locate-character'; /** @@ -1047,7 +1047,7 @@ declare module 'svelte/compiler' { /** The 'x' in `bind:x` */ name: string; /** The y in `bind:x={y}` */ - expression: Identifier | MemberExpression; + expression: Identifier | MemberExpression | SequenceExpression; } /** A `class:` directive */