svelte
svelte copied to clipboard
Prevent effect_update_depth_exceeded when using bind:value on a select with deriveds state in legacy components
Fixes https://github.com/sveltejs/svelte/issues/13768
The bug
In legacy mode Svelte generates extra code for every
<select bind:value={…}>.
That helper reads the bound value and immediately calls
invalidate_inner_signals, which writes back to the signals it just
read so that indirect updates propagate.
When the bound value itself is produced by a reactive statement
($:) this write-back creates a tight loop:
<script>
const default_details = { country: '' };
$: data = { locked: false, details: null };
$: details = data.details ?? default_details; // reactive
// variant that fails the same way
/* let details;
$:{ details = data.details ?? default_details } */
</script>
<select bind:value={details.country} disabled={data.locked}>
<option value="1">1</option>
</select>
- Helper reads
details.country invalidate_inner_signalsdoesinternal_set(data, data)$:re-runs, re-assignsdetails- Helper fires again → infinite loop →
runtime throwseffect_update_depth_exceeded.
What this PR does
Stops emitting the helper when it’s not needed.
In setup_select_synchronization we now check the bound identifier:
// skip helper if the variable is already managed by a `$:` block
if (bound.type === 'Identifier') {
const binding = context.state.scope.get(bound.name);
// 1. declared directly inside `$:`
if (binding?.kind === 'legacy_reactive') return;
// 2. declared elsewhere but *assigned* inside any `$:` block
for (const [, rs] of context.state.analysis.reactive_statements) {
if (rs.assignments.has(binding)) return;
}
}
If either condition is true the synchronisation helper is omitted, breaking the cycle. For all other bindings (plain state, props, store-subs, etc.) the helper is still generated, so existing behaviour should be unchanged.
Before submitting the PR, please make sure you do the following
- [x] It's really useful if your PR references an issue where it is discussed ahead of time. In many cases, features are absent for a reason. For large changes, please create an RFC: https://github.com/sveltejs/rfcs
- [x] Prefix your PR title with
feat:,fix:,chore:, ordocs:. - [x] This message body should clearly illustrate what problems it solves.
- [x] Ideally, include a test that fails without this PR but passes with it.
- [x] If this PR changes code within
packages/svelte/src, add a changeset (npx changeset).
Tests and linting
- [x] Run the tests with
pnpm testand lint the project withpnpm lint
🦋 Changeset detected
Latest commit: 0ceac964255a9b471f108be9d54bbd9192e44e59
The changes in this PR will be included in the next version bump.
This PR includes changesets to release 1 package
| Name | Type |
|---|---|
| svelte | Patch |
Not sure what this means? Click here to learn what changesets are.
Click here if you're a maintainer who wants to add another changeset to this PR
Preview: https://svelte-dev-git-preview-svelte-16165-svelte.vercel.app/
I wonder why invalidate_inner_signals is placed in a separate effect instead of calling it inside the selector's setter, similarly to how it was done in Svelte 4:
function select_change_handler() {
details.country = select_value(this);
($$invalidate(0, details), $$invalidate(1, data));
}
Mmh interesting, I can try to change my solution to do that and see if it works ^^.
So to mimic the svelte 4 behavior we should call it inside the mutate callback of the value binding right?
setup_select_synchronization is such, basically, since the very beginning of Svelte 5 and I'm not sure why. But I'd explore moving the logic inside the setter in hope we can recreate the Svelte 4 behavior.
Thank you! I think @7nik is right — the issue here is that we're using effects at all for this. I worry that tweaking the logic will fix the test case but still be buggy. Opened #16200 to start looking into this