diff --git a/.changeset/fix-select-bind-legacy-sync.md b/.changeset/fix-select-bind-legacy-sync.md new file mode 100644 index 0000000000..bf363f0fc4 --- /dev/null +++ b/.changeset/fix-select-bind-legacy-sync.md @@ -0,0 +1,5 @@ +--- +'svelte': patch +--- + +fix: resolve `effect_update_depth_exceeded` when using `bind:value` on `` + // means we need to invalidate `bar` whenever `foo` is mutated + if (node.name === 'select' && !runes) { + for (const attribute of node.attributes) { + if ( + attribute.type === 'BindDirective' && + attribute.name === 'value' && + attribute.expression.type !== 'SequenceExpression' + ) { + const identifier = object(attribute.expression); + const binding = identifier && context.state.scope.get(identifier.name); + + if (binding) { + for (const name of context.state.scope.references.keys()) { + if (name === binding.node.name) continue; + const indirect = context.state.scope.get(name); + + if (indirect) { + binding.legacy_indirect_bindings.add(indirect); + } + } + } + + break; + } + } + } + // Special case: single expression tag child of option element -> add "fake" attribute // to ensure that value types are the same (else for example numbers would be strings) if ( diff --git a/packages/svelte/src/compiler/phases/3-transform/client/visitors/AssignmentExpression.js b/packages/svelte/src/compiler/phases/3-transform/client/visitors/AssignmentExpression.js index 0f6a619357..1379669e77 100644 --- a/packages/svelte/src/compiler/phases/3-transform/client/visitors/AssignmentExpression.js +++ b/packages/svelte/src/compiler/phases/3-transform/client/visitors/AssignmentExpression.js @@ -8,7 +8,7 @@ import { is_event_attribute } from '../../../../utils/ast.js'; import { dev, locate_node } from '../../../../state.js'; -import { should_proxy } from '../utils.js'; +import { build_getter, should_proxy } from '../utils.js'; import { visit_assignment_expression } from '../../shared/assignments.js'; import { validate_mutation } from './shared/utils.js'; import { get_rune } from '../../../scope.js'; @@ -147,7 +147,7 @@ function build_assignment(operator, left, right, context) { // mutation if (transform?.mutate) { - return transform.mutate( + let mutation = transform.mutate( object, b.assignment( operator, @@ -155,6 +155,25 @@ function build_assignment(operator, left, right, context) { /** @type {Expression} */ (context.visit(right)) ) ); + + if (binding.legacy_indirect_bindings.size > 0) { + mutation = b.sequence([ + mutation, + b.call( + '$.invalidate_inner_signals', + b.arrow( + [], + b.block( + Array.from(binding.legacy_indirect_bindings).map((binding) => + b.stmt(build_getter({ ...binding.node }, context.state)) + ) + ) + ) + ) + ]); + } + + return mutation; } // in cases like `(object.items ??= []).push(value)`, we may need to warn 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 f651586dd8..7469dcda5b 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 @@ -199,10 +199,6 @@ export function RegularElement(node, context) { } } - if (node.name === 'select' && bindings.has('value')) { - setup_select_synchronization(/** @type {AST.BindDirective} */ (bindings.get('value')), context); - } - // Let bindings first, they can be used on attributes context.state.init.push(...lets); @@ -501,62 +497,6 @@ export function RegularElement(node, context) { context.state.template.pop_element(); } -/** - * Special case: if we have a value binding on a select element, we need to set up synchronization - * between the value binding and inner signals, for indirect updates - * @param {AST.BindDirective} value_binding - * @param {ComponentContext} context - */ -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); - } - - /** @type {string[]} */ - const names = []; - - for (const [name, refs] of context.state.scope.references) { - if ( - refs.length > 0 && - // prevent infinite loop - name !== bound.name - ) { - names.push(name); - } - } - - const invalidator = b.call( - '$.invalidate_inner_signals', - b.thunk( - b.block( - names.map((name) => { - const serialized = build_getter(b.id(name), context.state); - return b.stmt(serialized); - }) - ) - ) - ); - - context.state.init.push( - b.stmt( - b.call( - '$.template_effect', - b.thunk( - b.block([b.stmt(/** @type {Expression} */ (context.visit(bound))), b.stmt(invalidator)]) - ) - ) - ) - ); -} - /** * @param {AST.ClassDirective[]} class_directives * @param {ComponentContext} context diff --git a/packages/svelte/src/compiler/phases/scope.js b/packages/svelte/src/compiler/phases/scope.js index 52efd93210..a320718f58 100644 --- a/packages/svelte/src/compiler/phases/scope.js +++ b/packages/svelte/src/compiler/phases/scope.js @@ -120,6 +120,12 @@ export class Binding { */ legacy_dependencies = []; + /** + * Bindings that should be invalidated when this binding is invalidated + * @type {Set} + */ + legacy_indirect_bindings = new Set(); + /** * Legacy props: the `class` in `{ export klass as class}`. $props(): The `class` in { class: klass } = $props() * @type {string | null} diff --git a/packages/svelte/tests/runtime-legacy/samples/binding-select-reactive-derived/_config.js b/packages/svelte/tests/runtime-legacy/samples/binding-select-reactive-derived/_config.js new file mode 100644 index 0000000000..54f1fea7d0 --- /dev/null +++ b/packages/svelte/tests/runtime-legacy/samples/binding-select-reactive-derived/_config.js @@ -0,0 +1,37 @@ +import { test } from '../../test'; + +export default test({ + ssrHtml: ` + + `, + + async test({ assert, target, window, variant }) { + assert.htmlEqual( + target.innerHTML, + ` + + ` + ); + + const [select] = target.querySelectorAll('select'); + const options = target.querySelectorAll('option'); + + assert.equal(select.value, ''); + + const change = new window.Event('change'); + + // Select "UK" + options[2].selected = true; + await select.dispatchEvent(change); + + assert.equal(select.value, 'uk'); + } +}); diff --git a/packages/svelte/tests/runtime-legacy/samples/binding-select-reactive-derived/main.svelte b/packages/svelte/tests/runtime-legacy/samples/binding-select-reactive-derived/main.svelte new file mode 100644 index 0000000000..57342347f5 --- /dev/null +++ b/packages/svelte/tests/runtime-legacy/samples/binding-select-reactive-derived/main.svelte @@ -0,0 +1,21 @@ + + +