From de3757adf26e05aba8a9d27a044506303dda4449 Mon Sep 17 00:00:00 2001 From: Mattias Granlund Date: Thu, 5 Mar 2026 00:19:33 +0100 Subject: [PATCH 1/5] fix: skip derived re-evaluation inside destroyed branch effects MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit When a {#if} or {#each} branch is destroyed, derived values inside it must not be re-evaluated — the closure captures state that is now gone, and calling methods on undefined values throws. The crash path: - A parent $derived outside the block reads a child's exported reactive value via bind:this, keeping the inner derived connected in the reactive graph after the branch is torn down - On the next flush, is_dirty() walks the dep chain and calls execute_derived() on the destroyed branch's derived — which crashes The fix checks whether the nearest non-derived ancestor has both DESTROYED and BRANCH_EFFECT flags set, and returns the cached value instead of re-evaluating. This also simplifies get_derived_parent_effect() — it now returns the raw ancestor unconditionally, and execute_derived handles the destroyed case by setting parent_effect to null to avoid parenting new effects into a dead scope. --- .changeset/rare-ducks-sip.md | 5 +++++ .../internal/client/reactivity/deriveds.js | 21 +++++++++++++++---- 2 files changed, 22 insertions(+), 4 deletions(-) create mode 100644 .changeset/rare-ducks-sip.md diff --git a/.changeset/rare-ducks-sip.md b/.changeset/rare-ducks-sip.md new file mode 100644 index 0000000000..3575e9f726 --- /dev/null +++ b/.changeset/rare-ducks-sip.md @@ -0,0 +1,5 @@ +--- +'svelte': patch +--- + +fix: skip derived re-evaluation inside destroyed branch effects diff --git a/packages/svelte/src/internal/client/reactivity/deriveds.js b/packages/svelte/src/internal/client/reactivity/deriveds.js index 036256284c..1eb9a5d7e4 100644 --- a/packages/svelte/src/internal/client/reactivity/deriveds.js +++ b/packages/svelte/src/internal/client/reactivity/deriveds.js @@ -11,7 +11,8 @@ import { WAS_MARKED, DESTROYED, CLEAN, - REACTION_RAN + REACTION_RAN, + BRANCH_EFFECT } from '#client/constants'; import { active_reaction, @@ -312,9 +313,7 @@ function get_derived_parent_effect(derived) { var parent = derived.parent; while (parent !== null) { if ((parent.f & DERIVED) === 0) { - // The original parent effect might've been destroyed but the derived - // is used elsewhere now - do not return the destroyed effect in that case - return (parent.f & DESTROYED) === 0 ? /** @type {Effect} */ (parent) : null; + return /** @type {Effect} */ (parent); } parent = parent.parent; } @@ -327,6 +326,20 @@ function get_derived_parent_effect(derived) { * @returns {T} */ export function execute_derived(derived) { + var raw_parent = get_derived_parent_effect(derived); + var parent_effect = raw_parent !== null && (raw_parent.f & DESTROYED) !== 0 ? null : raw_parent; + + // don't update deriveds inside a destroyed branch (e.g. {#if} or {#each}) — + // the branch scope is invalid and evaluating could trigger side effects + // with stale values. + if ( + !is_destroying_effect && + raw_parent !== null && + (raw_parent.f & (DESTROYED | BRANCH_EFFECT)) === (DESTROYED | BRANCH_EFFECT) + ) { + return derived.v; + } + var value; var prev_active_effect = active_effect; From ba44bddc9d528054b72f77a743608a8d16995f7e Mon Sep 17 00:00:00 2001 From: Mattias Granlund Date: Sat, 7 Mar 2026 10:38:03 +0100 Subject: [PATCH 2/5] fix: guard derived re-evaluation inside INERT (outroing) branches MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit When a branch is transitioning out (INERT), an external $derived can still pull inner deriveds via bind:this, causing re-evaluation with stale dependency values (e.g. value.toUpperCase() where value is now undefined). This violates the {#if} contract — code inside {#if value} should never run with a falsy value. The fix adds a unified guard in update_derived that returns the cached value instead of re-evaluating: - Non-async mode: blocks unconditionally (INERT branches are never walked by the scheduler, so any read is external) - Async mode: blocks only during effect flushing (detected via collected_effects === null && active_effect === null), allowing branch-internal traversal to still update deriveds for transitions This replaces a narrower guard that was previously in execute_derived (non-async only) and extends coverage to async mode where the same crash occurs but was previously unguarded. Includes a component test (if-block-derived-inert-external-reader) that reproduces the crash: an Inner component inside {#if value} with out:fade exposes a $derived via bind:this; clearing value starts the out-transition and triggers the external read path. --- .../internal/client/reactivity/deriveds.js | 30 ++++++++++++++-- .../Inner.svelte | 9 +++++ .../_config.js | 36 +++++++++++++++++++ .../main.svelte | 21 +++++++++++ 4 files changed, 94 insertions(+), 2 deletions(-) create mode 100644 packages/svelte/tests/runtime-runes/samples/if-block-derived-inert-external-reader/Inner.svelte create mode 100644 packages/svelte/tests/runtime-runes/samples/if-block-derived-inert-external-reader/_config.js create mode 100644 packages/svelte/tests/runtime-runes/samples/if-block-derived-inert-external-reader/main.svelte diff --git a/packages/svelte/src/internal/client/reactivity/deriveds.js b/packages/svelte/src/internal/client/reactivity/deriveds.js index 1eb9a5d7e4..af46ae01ac 100644 --- a/packages/svelte/src/internal/client/reactivity/deriveds.js +++ b/packages/svelte/src/internal/client/reactivity/deriveds.js @@ -40,7 +40,7 @@ import { get_error } from '../../shared/dev.js'; import { async_mode_flag, tracing_mode_flag } from '../../flags/index.js'; import { component_context } from '../context.js'; import { UNINITIALIZED } from '../../../constants.js'; -import { batch_values, current_batch } from './batch.js'; +import { batch_values, collected_effects, current_batch } from './batch.js'; import { increment_pending, unset_context } from './async.js'; import { deferred, includes, noop } from '../../shared/utils.js'; import { set_signal_status, update_derived_status } from './status.js'; @@ -343,7 +343,7 @@ export function execute_derived(derived) { var value; var prev_active_effect = active_effect; - set_active_effect(get_derived_parent_effect(derived)); + set_active_effect(parent_effect); if (DEV) { let prev_eager_effects = eager_effects; @@ -381,6 +381,32 @@ export function execute_derived(derived) { * @returns {void} */ export function update_derived(derived) { + // Don't re-evaluate deriveds inside INERT (outroing) branches when the + // read originates from outside the branch. Re-evaluating would use stale + // dependency values (e.g. a prop that became `undefined` when the branch + // condition changed), violating the `{#if}` contract. + // + // In non-async mode, INERT branches are never walked by the scheduler, + // so any read is necessarily external — block unconditionally. + // + // In async mode, INERT branches ARE walked (to keep transitions alive), + // so we only block reads during effect flushing (collected_effects === null + // and active_effect === null), which indicates the reader is an external + // effect, not the branch's own traversal. + if (!is_destroying_effect) { + var dominated_by_inert = async_mode_flag + ? collected_effects === null && active_effect === null + : true; + + if (dominated_by_inert) { + var parent = get_derived_parent_effect(derived); + + if (parent !== null && (parent.f & INERT) !== 0 && (parent.f & DESTROYED) === 0) { + return; + } + } + } + var value = execute_derived(derived); if (!derived.equals(value)) { diff --git a/packages/svelte/tests/runtime-runes/samples/if-block-derived-inert-external-reader/Inner.svelte b/packages/svelte/tests/runtime-runes/samples/if-block-derived-inert-external-reader/Inner.svelte new file mode 100644 index 0000000000..e9e6e0d18d --- /dev/null +++ b/packages/svelte/tests/runtime-runes/samples/if-block-derived-inert-external-reader/Inner.svelte @@ -0,0 +1,9 @@ + diff --git a/packages/svelte/tests/runtime-runes/samples/if-block-derived-inert-external-reader/_config.js b/packages/svelte/tests/runtime-runes/samples/if-block-derived-inert-external-reader/_config.js new file mode 100644 index 0000000000..ee98fbfcff --- /dev/null +++ b/packages/svelte/tests/runtime-runes/samples/if-block-derived-inert-external-reader/_config.js @@ -0,0 +1,36 @@ +import { flushSync } from 'svelte'; +import { test } from '../../test'; + +// Covers the INERT (outroing) counterpart to if-block-const-destroyed-external-reader. +// An external $derived reads a child component's $derived via bind:this, keeping it +// connected in the reactive graph while the branch is outroing. Without a guard, +// the inner derived re-evaluates with stale values mid-transition and crashes. +// The fix returns the cached value and keeps the derived dirty so it re-evaluates +// correctly if the branch reverses (INERT cleared) rather than being stuck with +// a stale clean value. +export default test({ + ssrHtml: '

', + html: '

HELLO

', + + async test({ assert, raf, target }) { + const [button] = target.querySelectorAll('button'); + + // Clearing value starts the out-transition (branch becomes INERT). + // Without the guard this crashes with a TypeError in async mode. + flushSync(() => button.click()); + + assert.htmlEqual( + target.innerHTML, + '

HELLO

' + ); + + // Complete the transition — branch is now destroyed and div is removed. + raf.tick(100); + + // Flush the bind:this teardown microtask and resulting effect updates. + await Promise.resolve(); + flushSync(); + + assert.htmlEqual(target.innerHTML, '

'); + } +}); diff --git a/packages/svelte/tests/runtime-runes/samples/if-block-derived-inert-external-reader/main.svelte b/packages/svelte/tests/runtime-runes/samples/if-block-derived-inert-external-reader/main.svelte new file mode 100644 index 0000000000..f66ddb7015 --- /dev/null +++ b/packages/svelte/tests/runtime-runes/samples/if-block-derived-inert-external-reader/main.svelte @@ -0,0 +1,21 @@ + + +{#if value} +
+ +
+{/if} + + +

{externalView}

From afd35cf2c735436b576af1fd0c8249a05575dab0 Mon Sep 17 00:00:00 2001 From: Mattias Granlund Date: Sat, 7 Mar 2026 17:33:27 +0100 Subject: [PATCH 3/5] fix: capture computed bind:this keys to prevent stale reads on teardown MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit While investigating the INERT branch guard from the previous commit, a different trigger for the same class of bug was found: bind:this with a dynamic key (e.g. `bind:this={elements[key]}`) inside an {#if} block. When the branch is destroyed, bind:this queues a microtask to null out the binding. That teardown calls get_value() which re-reads the dynamic key expression — but the reactive context is gone, so it crashes (e.g. value.toUpperCase() on undefined). The fix is in the compiler (build_bind_this): computed member expression keys and block-scoped variables used in bind:this expressions are now captured as "parts" via get_parts, which the runtime evaluates during the render_effect (while values are still valid) and reuses during teardown. This extends an existing mechanism that previously only captured {#each} context variables. Two changes in build_bind_this: - Broadened the identifier capture to include all block-scoped variables (not just {#each} context), so {@const} variables are captured too - Added extraction of computed MemberExpression properties into parts, so inline expressions like bind:this={obj[expr]} are also safe Test: if-block-bind-this-dynamic-key-destroyed covers the inline case (bind:this={elements[value.toUpperCase()]}) which is only fixable via the compiler change. The {#if} + transition variant (if-block-bind-this-dynamic-key-inert) is covered by either this fix or the runtime guard, so it was removed to avoid redundant coverage. --- .../client/visitors/shared/utils.js | 27 +++++++++++++++---- .../internal/client/reactivity/deriveds.js | 2 ++ .../_config.js | 24 +++++++++++++++++ .../main.svelte | 10 +++++++ 4 files changed, 58 insertions(+), 5 deletions(-) create mode 100644 packages/svelte/tests/runtime-runes/samples/if-block-bind-this-dynamic-key-destroyed/_config.js create mode 100644 packages/svelte/tests/runtime-runes/samples/if-block-bind-this-dynamic-key-destroyed/main.svelte 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 7256073096..8b244820c9 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 @@ -253,10 +253,27 @@ export function build_bind_this(expression, value, { state, visit }) { const transform = { ...state.transform }; - // Pass in each context variables to the get/set functions, so that we can null out old values on teardown. - // Note that we only do this for each context variables, the consequence is that the value might be stale in - // some scenarios where the value is a member expression with changing computed parts or using a combination of multiple - // variables, but that was the same case in Svelte 4, too. Once legacy mode is gone completely, we can revisit this. + // Extract computed member expression keys from the bind:this target so that teardown + // uses captured values instead of re-reading reactive state from a potentially destroyed scope. + // For `bind:this={obj[expr]}`, we capture `expr` as a part and replace it with a parameter. + const target = getter ?? expression; + if (target.type === 'MemberExpression' && target.computed && target.property.type !== 'Literal') { + const key_id = b.id('$$key'); + ids.push(key_id); + values.push(/** @type {Expression} */ (visit(target.property))); + + // Replace the computed property with the parameter in the expression. + // We mutate a clone so the original AST is not affected. + if (!getter) { + expression = /** @type {typeof expression} */ (JSON.parse(JSON.stringify(expression))); + /** @type {MemberExpression} */ (expression).property = key_id; + /** @type {MemberExpression} */ (expression).computed = true; + } + } + + // Pass in context variables to the get/set functions, so that we can null out old values on + // teardown using the captured (non-stale) parts. Without this, bind:this teardown re-reads + // reactive state from a potentially destroyed scope, causing crashes. walk(getter ?? expression, null, { Identifier(node, { path }) { if (seen.includes(node.name)) return; @@ -269,7 +286,7 @@ export function build_bind_this(expression, value, { state, visit }) { if (!binding) return; for (const [owner, scope] of state.scopes) { - if (owner.type === 'EachBlock' && scope === binding.scope) { + if (scope === binding.scope) { ids.push(node); values.push(/** @type {Expression} */ (visit(node))); diff --git a/packages/svelte/src/internal/client/reactivity/deriveds.js b/packages/svelte/src/internal/client/reactivity/deriveds.js index af46ae01ac..9e32849c9f 100644 --- a/packages/svelte/src/internal/client/reactivity/deriveds.js +++ b/packages/svelte/src/internal/client/reactivity/deriveds.js @@ -11,6 +11,7 @@ import { WAS_MARKED, DESTROYED, CLEAN, + INERT, REACTION_RAN, BRANCH_EFFECT } from '#client/constants'; @@ -44,6 +45,7 @@ import { batch_values, collected_effects, current_batch } from './batch.js'; import { increment_pending, unset_context } from './async.js'; import { deferred, includes, noop } from '../../shared/utils.js'; import { set_signal_status, update_derived_status } from './status.js'; +import { Boundary } from '../dom/blocks/boundary.js'; /** @type {Effect | null} */ export let current_async_effect = null; diff --git a/packages/svelte/tests/runtime-runes/samples/if-block-bind-this-dynamic-key-destroyed/_config.js b/packages/svelte/tests/runtime-runes/samples/if-block-bind-this-dynamic-key-destroyed/_config.js new file mode 100644 index 0000000000..891ac103d6 --- /dev/null +++ b/packages/svelte/tests/runtime-runes/samples/if-block-bind-this-dynamic-key-destroyed/_config.js @@ -0,0 +1,24 @@ +import { flushSync } from 'svelte'; +import { test } from '../../test'; + +// When an {#if} branch with bind:this={obj[expr]} is destroyed, bind:this +// queues a microtask to null out the binding. That teardown re-reads the +// dynamic key expression — which crashes if the reactive context is stale. +// The compiler fix captures the computed key in get_parts during the +// render_effect, so teardown uses the captured value instead. +export default test({ + html: 'hello', + + async test({ assert, target }) { + const [button] = target.querySelectorAll('button'); + + flushSync(() => button.click()); + + // Branch immediately destroyed (no transition). + // bind:this teardown microtask reads the dynamic key — must not crash. + await new Promise((resolve) => setTimeout(resolve, 0)); + flushSync(); + + assert.htmlEqual(target.innerHTML, ''); + } +}); diff --git a/packages/svelte/tests/runtime-runes/samples/if-block-bind-this-dynamic-key-destroyed/main.svelte b/packages/svelte/tests/runtime-runes/samples/if-block-bind-this-dynamic-key-destroyed/main.svelte new file mode 100644 index 0000000000..8b60d693eb --- /dev/null +++ b/packages/svelte/tests/runtime-runes/samples/if-block-bind-this-dynamic-key-destroyed/main.svelte @@ -0,0 +1,10 @@ + + +{#if value} + {value} +{/if} + + From 8e0cfc8a8d59f063e657b5be7044730e61f1c56a Mon Sep 17 00:00:00 2001 From: Rich Harris Date: Mon, 9 Mar 2026 11:54:23 -0400 Subject: [PATCH 4/5] typecheck --- packages/svelte/src/internal/client/reactivity/deriveds.js | 1 - 1 file changed, 1 deletion(-) diff --git a/packages/svelte/src/internal/client/reactivity/deriveds.js b/packages/svelte/src/internal/client/reactivity/deriveds.js index 076ed615c3..fc3444bc30 100644 --- a/packages/svelte/src/internal/client/reactivity/deriveds.js +++ b/packages/svelte/src/internal/client/reactivity/deriveds.js @@ -46,7 +46,6 @@ import { batch_values, collected_effects, current_batch } from './batch.js'; import { increment_pending, unset_context } from './async.js'; import { deferred, includes, noop } from '../../shared/utils.js'; import { set_signal_status, update_derived_status } from './status.js'; -import { Boundary } from '../dom/blocks/boundary.js'; /** @type {Effect | null} */ export let current_async_effect = null; From 1bb8442f16ea3eac339c17fb52b131ee1f354721 Mon Sep 17 00:00:00 2001 From: Rich Harris Date: Mon, 9 Mar 2026 17:19:54 -0400 Subject: [PATCH 5/5] remove the bind:this stuff --- .../client/visitors/shared/utils.js | 27 ++++--------------- .../_config.js | 24 ----------------- .../main.svelte | 10 ------- 3 files changed, 5 insertions(+), 56 deletions(-) delete mode 100644 packages/svelte/tests/runtime-runes/samples/if-block-bind-this-dynamic-key-destroyed/_config.js delete mode 100644 packages/svelte/tests/runtime-runes/samples/if-block-bind-this-dynamic-key-destroyed/main.svelte 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 8b244820c9..7256073096 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 @@ -253,27 +253,10 @@ export function build_bind_this(expression, value, { state, visit }) { const transform = { ...state.transform }; - // Extract computed member expression keys from the bind:this target so that teardown - // uses captured values instead of re-reading reactive state from a potentially destroyed scope. - // For `bind:this={obj[expr]}`, we capture `expr` as a part and replace it with a parameter. - const target = getter ?? expression; - if (target.type === 'MemberExpression' && target.computed && target.property.type !== 'Literal') { - const key_id = b.id('$$key'); - ids.push(key_id); - values.push(/** @type {Expression} */ (visit(target.property))); - - // Replace the computed property with the parameter in the expression. - // We mutate a clone so the original AST is not affected. - if (!getter) { - expression = /** @type {typeof expression} */ (JSON.parse(JSON.stringify(expression))); - /** @type {MemberExpression} */ (expression).property = key_id; - /** @type {MemberExpression} */ (expression).computed = true; - } - } - - // Pass in context variables to the get/set functions, so that we can null out old values on - // teardown using the captured (non-stale) parts. Without this, bind:this teardown re-reads - // reactive state from a potentially destroyed scope, causing crashes. + // Pass in each context variables to the get/set functions, so that we can null out old values on teardown. + // Note that we only do this for each context variables, the consequence is that the value might be stale in + // some scenarios where the value is a member expression with changing computed parts or using a combination of multiple + // variables, but that was the same case in Svelte 4, too. Once legacy mode is gone completely, we can revisit this. walk(getter ?? expression, null, { Identifier(node, { path }) { if (seen.includes(node.name)) return; @@ -286,7 +269,7 @@ export function build_bind_this(expression, value, { state, visit }) { if (!binding) return; for (const [owner, scope] of state.scopes) { - if (scope === binding.scope) { + if (owner.type === 'EachBlock' && scope === binding.scope) { ids.push(node); values.push(/** @type {Expression} */ (visit(node))); diff --git a/packages/svelte/tests/runtime-runes/samples/if-block-bind-this-dynamic-key-destroyed/_config.js b/packages/svelte/tests/runtime-runes/samples/if-block-bind-this-dynamic-key-destroyed/_config.js deleted file mode 100644 index 891ac103d6..0000000000 --- a/packages/svelte/tests/runtime-runes/samples/if-block-bind-this-dynamic-key-destroyed/_config.js +++ /dev/null @@ -1,24 +0,0 @@ -import { flushSync } from 'svelte'; -import { test } from '../../test'; - -// When an {#if} branch with bind:this={obj[expr]} is destroyed, bind:this -// queues a microtask to null out the binding. That teardown re-reads the -// dynamic key expression — which crashes if the reactive context is stale. -// The compiler fix captures the computed key in get_parts during the -// render_effect, so teardown uses the captured value instead. -export default test({ - html: 'hello', - - async test({ assert, target }) { - const [button] = target.querySelectorAll('button'); - - flushSync(() => button.click()); - - // Branch immediately destroyed (no transition). - // bind:this teardown microtask reads the dynamic key — must not crash. - await new Promise((resolve) => setTimeout(resolve, 0)); - flushSync(); - - assert.htmlEqual(target.innerHTML, ''); - } -}); diff --git a/packages/svelte/tests/runtime-runes/samples/if-block-bind-this-dynamic-key-destroyed/main.svelte b/packages/svelte/tests/runtime-runes/samples/if-block-bind-this-dynamic-key-destroyed/main.svelte deleted file mode 100644 index 8b60d693eb..0000000000 --- a/packages/svelte/tests/runtime-runes/samples/if-block-bind-this-dynamic-key-destroyed/main.svelte +++ /dev/null @@ -1,10 +0,0 @@ - - -{#if value} - {value} -{/if} - -