fix: resolve effect_update_depth_exceeded with select bind:value in legacy mode (#17645)

## Summary

Fixes #13768

`<select bind:value={derived.prop}>` in legacy (non-runes) components
throws `effect_update_depth_exceeded` when the bound value comes from a
`$:` reactive statement.

**Root cause:** `setup_select_synchronization` created a
`template_effect` that called `invalidate_inner_signals`, which reads
and writes the same signals on every change — creating an infinite
update loop when those signals feed back into derived state.

**Fix:** Remove the effect-based synchronization entirely. Instead,
populate `legacy_indirect_bindings` during the analyze phase for
`<select bind:value>` elements, and call `invalidate_inner_signals`
inline at the mutation point in `AssignmentExpression` — only when the
binding is actually mutated, avoiding the read-write cycle.

Based on the approach outlined in #16200.

## Changes

- **`scope.js`**: Add `legacy_indirect_bindings` field to `Binding`
class
- **`RegularElement.js` (analyze)**: For `<select bind:value={foo}>`,
collect scope references as indirect bindings on the bound variable
- **`RegularElement.js` (transform)**: Remove
`setup_select_synchronization` function and its call site
- **`AssignmentExpression.js` (transform)**: When mutating a binding
with indirect bindings, append `invalidate_inner_signals` call after the
mutation

## Test plan

- Added `binding-select-reactive-derived` test that reproduces the exact
scenario from #13768
- All 3291 runtime-legacy tests pass (0 regressions)
- All 2312 runtime-runes tests pass
- All snapshot and compiler tests pass

---------

Co-authored-by: Rich Harris <rich.harris@vercel.com>
pull/17668/head
Artyom Alekseevich 3 days ago committed by GitHub
parent 3c6bb6faba
commit 684cdba253
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194

@ -0,0 +1,5 @@
---
'svelte': patch
---
fix: resolve `effect_update_depth_exceeded` when using `bind:value` on `<select>` with derived state in legacy mode

@ -16,6 +16,8 @@ import { regex_starts_with_newline } from '../../patterns.js';
import { check_element } from './shared/a11y/index.js';
import { validate_element } from './shared/element.js';
import { mark_subtree_dynamic } from './shared/fragment.js';
import { object } from '../../../utils/ast.js';
import { runes } from '../../../state.js';
/**
* @param {AST.RegularElement} node
@ -64,6 +66,34 @@ export function RegularElement(node, context) {
}
}
// Special case: `<select bind:value={foo}><option>{bar}</option>`
// 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 (

@ -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

@ -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

@ -120,6 +120,12 @@ export class Binding {
*/
legacy_dependencies = [];
/**
* Bindings that should be invalidated when this binding is invalidated
* @type {Set<Binding>}
*/
legacy_indirect_bindings = new Set();
/**
* Legacy props: the `class` in `{ export klass as class}`. $props(): The `class` in { class: klass } = $props()
* @type {string | null}

@ -0,0 +1,37 @@
import { test } from '../../test';
export default test({
ssrHtml: `
<select>
<option selected="" value="">Select</option>
<option value="us">US</option>
<option value="uk">UK</option>
</select>
`,
async test({ assert, target, window, variant }) {
assert.htmlEqual(
target.innerHTML,
`
<select>
<option${variant === 'hydrate' ? ' selected=""' : ''} value="">Select</option>
<option value="us">US</option>
<option value="uk">UK</option>
</select>
`
);
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');
}
});

@ -0,0 +1,21 @@
<script>
const default_details = {
country: '',
}
$: data = {
locked: false,
details: null,
}
$: details = data.details ?? default_details
</script>
<select
bind:value={details.country}
disabled={data.locked}
>
<option value="">Select</option>
<option value="us">US</option>
<option value="uk">UK</option>
</select>
Loading…
Cancel
Save