From e14561c8293fcfd27456df84327cc4f4700aacfb Mon Sep 17 00:00:00 2001 From: Simon Holthausen Date: Thu, 14 Aug 2025 16:03:27 +0200 Subject: [PATCH 1/9] fix: associate batch with boundary This associates the current batch with the boundary when entering pending mode. That way other async work associated to that boundary also can associate itself with that batch, even if e.g. due to flushing it is no longer the current batch. This solves a null pointer exception that can occur when the batch is flushed before the next top level await or async derived gets a hold of the current batch, which is null then. Fixes #16596 Fixes https://github.com/sveltejs/kit/issues/14124 --- .changeset/spicy-ears-join.md | 5 +++ .../internal/client/dom/blocks/boundary.js | 13 ++++++ .../src/internal/client/reactivity/batch.js | 2 +- .../internal/client/reactivity/deriveds.js | 2 +- .../samples/async-nested-top-level/Bar.svelte | 7 ++++ .../samples/async-nested-top-level/Foo.svelte | 10 +++++ .../samples/async-nested-top-level/_config.js | 42 +++++++++++++++++++ .../async-nested-top-level/main.svelte | 31 ++++++++++++++ .../async-top-level-deriveds/Foo.svelte | 8 ++++ .../async-top-level-deriveds/_config.js | 41 ++++++++++++++++++ .../async-top-level-deriveds/main.svelte | 31 ++++++++++++++ 11 files changed, 190 insertions(+), 2 deletions(-) create mode 100644 .changeset/spicy-ears-join.md create mode 100644 packages/svelte/tests/runtime-runes/samples/async-nested-top-level/Bar.svelte create mode 100644 packages/svelte/tests/runtime-runes/samples/async-nested-top-level/Foo.svelte create mode 100644 packages/svelte/tests/runtime-runes/samples/async-nested-top-level/_config.js create mode 100644 packages/svelte/tests/runtime-runes/samples/async-nested-top-level/main.svelte create mode 100644 packages/svelte/tests/runtime-runes/samples/async-top-level-deriveds/Foo.svelte create mode 100644 packages/svelte/tests/runtime-runes/samples/async-top-level-deriveds/_config.js create mode 100644 packages/svelte/tests/runtime-runes/samples/async-top-level-deriveds/main.svelte diff --git a/.changeset/spicy-ears-join.md b/.changeset/spicy-ears-join.md new file mode 100644 index 0000000000..ee29e77afe --- /dev/null +++ b/.changeset/spicy-ears-join.md @@ -0,0 +1,5 @@ +--- +'svelte': patch +--- + +fix: associate batch with boundary diff --git a/packages/svelte/src/internal/client/dom/blocks/boundary.js b/packages/svelte/src/internal/client/dom/blocks/boundary.js index 1866931ef2..b7fd9a856c 100644 --- a/packages/svelte/src/internal/client/dom/blocks/boundary.js +++ b/packages/svelte/src/internal/client/dom/blocks/boundary.js @@ -59,6 +59,18 @@ export class Boundary { /** @type {Boundary | null} */ parent; + /** + * The associated batch to this boundary while the boundary pending; set by the one interacting with the boundary when entering pending state. + * Will be `null` once the boundary is no longer pending. + * + * Needed because `current_batch` isn't guaranteed to exist: E.g. when component A has top level await, then renders component B + * which also has top level await, `current_batch` can be null when a flush from component A happens before + * suspend() in component B is called. We hence save it on the boundary instead. + * + * @type {Batch | null} + */ + batch = null; + /** @type {TemplateNode} */ #anchor; @@ -231,6 +243,7 @@ export class Boundary { if (this.#pending_count === 0) { this.pending = false; + this.batch = null; if (this.#pending_effect) { pause_effect(this.#pending_effect, () => { diff --git a/packages/svelte/src/internal/client/reactivity/batch.js b/packages/svelte/src/internal/client/reactivity/batch.js index 123bc95d16..b3a3bef731 100644 --- a/packages/svelte/src/internal/client/reactivity/batch.js +++ b/packages/svelte/src/internal/client/reactivity/batch.js @@ -664,7 +664,7 @@ export function schedule_effect(signal) { export function suspend() { var boundary = get_pending_boundary(); - var batch = /** @type {Batch} */ (current_batch); + var batch = (boundary.batch ??= /** @type {Batch} */ (current_batch)); var pending = boundary.pending; boundary.update_pending_count(1); diff --git a/packages/svelte/src/internal/client/reactivity/deriveds.js b/packages/svelte/src/internal/client/reactivity/deriveds.js index 7f730e365e..0ac8582933 100644 --- a/packages/svelte/src/internal/client/reactivity/deriveds.js +++ b/packages/svelte/src/internal/client/reactivity/deriveds.js @@ -131,7 +131,7 @@ export function async_derived(fn, location) { prev = promise; - var batch = /** @type {Batch} */ (current_batch); + var batch = (boundary.batch ??= /** @type {Batch} */ (current_batch)); var pending = boundary.pending; if (should_suspend) { diff --git a/packages/svelte/tests/runtime-runes/samples/async-nested-top-level/Bar.svelte b/packages/svelte/tests/runtime-runes/samples/async-nested-top-level/Bar.svelte new file mode 100644 index 0000000000..f1ac9ab760 --- /dev/null +++ b/packages/svelte/tests/runtime-runes/samples/async-nested-top-level/Bar.svelte @@ -0,0 +1,7 @@ + + +

bar: {bar}

diff --git a/packages/svelte/tests/runtime-runes/samples/async-nested-top-level/Foo.svelte b/packages/svelte/tests/runtime-runes/samples/async-nested-top-level/Foo.svelte new file mode 100644 index 0000000000..e2029a3033 --- /dev/null +++ b/packages/svelte/tests/runtime-runes/samples/async-nested-top-level/Foo.svelte @@ -0,0 +1,10 @@ + + +

foo: {foo}

+ + diff --git a/packages/svelte/tests/runtime-runes/samples/async-nested-top-level/_config.js b/packages/svelte/tests/runtime-runes/samples/async-nested-top-level/_config.js new file mode 100644 index 0000000000..ca7965bf79 --- /dev/null +++ b/packages/svelte/tests/runtime-runes/samples/async-nested-top-level/_config.js @@ -0,0 +1,42 @@ +import { tick } from 'svelte'; +import { test } from '../../test'; + +export default test({ + async test({ assert, target }) { + const [show, resolve] = target.querySelectorAll('button'); + + show.click(); + await tick(); + assert.htmlEqual( + target.innerHTML, + ` + + +

pending...

+ ` + ); + + resolve.click(); + await tick(); + assert.htmlEqual( + target.innerHTML, + ` + + +

pending...

+ ` + ); + + resolve.click(); + await tick(); + assert.htmlEqual( + target.innerHTML, + ` + + +

foo: foo

+

bar: bar

+ ` + ); + } +}); diff --git a/packages/svelte/tests/runtime-runes/samples/async-nested-top-level/main.svelte b/packages/svelte/tests/runtime-runes/samples/async-nested-top-level/main.svelte new file mode 100644 index 0000000000..bd0efaa4f8 --- /dev/null +++ b/packages/svelte/tests/runtime-runes/samples/async-nested-top-level/main.svelte @@ -0,0 +1,31 @@ + + + + + + + + + + {#if show} + + {/if} + + {#if $effect.pending()} +

pending...

+ {/if} + + {#snippet pending()} +

initializing...

+ {/snippet} +
diff --git a/packages/svelte/tests/runtime-runes/samples/async-top-level-deriveds/Foo.svelte b/packages/svelte/tests/runtime-runes/samples/async-top-level-deriveds/Foo.svelte new file mode 100644 index 0000000000..e8a7c84137 --- /dev/null +++ b/packages/svelte/tests/runtime-runes/samples/async-top-level-deriveds/Foo.svelte @@ -0,0 +1,8 @@ + + +

{foo} {bar}

diff --git a/packages/svelte/tests/runtime-runes/samples/async-top-level-deriveds/_config.js b/packages/svelte/tests/runtime-runes/samples/async-top-level-deriveds/_config.js new file mode 100644 index 0000000000..2c7ffd3952 --- /dev/null +++ b/packages/svelte/tests/runtime-runes/samples/async-top-level-deriveds/_config.js @@ -0,0 +1,41 @@ +import { tick } from 'svelte'; +import { test } from '../../test'; + +export default test({ + async test({ assert, target }) { + const [show, resolve] = target.querySelectorAll('button'); + + show.click(); + await tick(); + assert.htmlEqual( + target.innerHTML, + ` + + +

pending...

+ ` + ); + + resolve.click(); + await tick(); + assert.htmlEqual( + target.innerHTML, + ` + + +

pending...

+ ` + ); + + resolve.click(); + await tick(); + assert.htmlEqual( + target.innerHTML, + ` + + +

foo bar

+ ` + ); + } +}); diff --git a/packages/svelte/tests/runtime-runes/samples/async-top-level-deriveds/main.svelte b/packages/svelte/tests/runtime-runes/samples/async-top-level-deriveds/main.svelte new file mode 100644 index 0000000000..bd0efaa4f8 --- /dev/null +++ b/packages/svelte/tests/runtime-runes/samples/async-top-level-deriveds/main.svelte @@ -0,0 +1,31 @@ + + + + + + + + + + {#if show} + + {/if} + + {#if $effect.pending()} +

pending...

+ {/if} + + {#snippet pending()} +

initializing...

+ {/snippet} +
From 0f4d0b101b2d20247bb0a5167b6f6614e83af561 Mon Sep 17 00:00:00 2001 From: Simon Holthausen Date: Fri, 15 Aug 2025 16:46:46 +0200 Subject: [PATCH 2/9] fix --- .../internal/client/dom/blocks/boundary.js | 13 ++++++-- .../src/internal/client/reactivity/batch.js | 31 +++++++++++++++++-- .../internal/client/reactivity/deriveds.js | 5 ++- 3 files changed, 41 insertions(+), 8 deletions(-) diff --git a/packages/svelte/src/internal/client/dom/blocks/boundary.js b/packages/svelte/src/internal/client/dom/blocks/boundary.js index b7fd9a856c..350a417b8e 100644 --- a/packages/svelte/src/internal/client/dom/blocks/boundary.js +++ b/packages/svelte/src/internal/client/dom/blocks/boundary.js @@ -28,7 +28,7 @@ import { queue_micro_task } from '../task.js'; import * as e from '../../errors.js'; import * as w from '../../warnings.js'; import { DEV } from 'esm-env'; -import { Batch, effect_pending_updates } from '../../reactivity/batch.js'; +import { Batch, current_batch, effect_pending_updates } from '../../reactivity/batch.js'; import { internal_set, source } from '../../reactivity/sources.js'; import { tag } from '../../dev/tracing.js'; import { createSubscriber } from '../../../../reactivity/create-subscriber.js'; @@ -69,7 +69,7 @@ export class Boundary { * * @type {Batch | null} */ - batch = null; + #batch = null; /** @type {TemplateNode} */ #anchor; @@ -200,6 +200,13 @@ export class Boundary { return !!this.#props.pending; } + get_batch() { + if (current_batch) { + this.#batch = current_batch; + } + return /** @type {Batch} */ (this.#batch); + } + /** * @param {() => Effect | null} fn */ @@ -243,7 +250,7 @@ export class Boundary { if (this.#pending_count === 0) { this.pending = false; - this.batch = null; + this.#batch = null; if (this.#pending_effect) { pause_effect(this.#pending_effect, () => { diff --git a/packages/svelte/src/internal/client/reactivity/batch.js b/packages/svelte/src/internal/client/reactivity/batch.js index b3a3bef731..d102355071 100644 --- a/packages/svelte/src/internal/client/reactivity/batch.js +++ b/packages/svelte/src/internal/client/reactivity/batch.js @@ -184,6 +184,14 @@ export class Batch { /** @type {Map | null} */ var current_values = null; + /** + * A batch is superseeded if all of its sources are also in the current batch. + * If the current batch commits, we don't need the old batch anymore. + * This also prevents memory leaks since the old batch will never be committed. + * @type {Batch[]} + */ + var superseeded_batches = []; + // if there are multiple batches, we are 'time travelling' — // we need to undo the changes belonging to any batch // other than the current one @@ -196,15 +204,25 @@ export class Batch { source.v = current; } + let is_prior_batch = true; + for (const batch of batches) { - if (batch === this) continue; + if (batch === this) { + is_prior_batch = false; + continue; + } + + let superseeded = is_prior_batch; for (const [source, previous] of batch.#previous) { if (!current_values.has(source)) { + superseeded = false; current_values.set(source, { v: source.v, wv: source.wv }); source.v = previous; } } + + if (superseeded) superseeded_batches.push(batch); } } @@ -215,6 +233,10 @@ export class Batch { // if we didn't start any new async work, and no async work // is outstanding from a previous flush, commit if (this.#async_effects.length === 0 && this.#pending === 0) { + for (const batch of superseeded_batches) { + batch.remove(); + } + this.#commit(); var render_effects = this.#render_effects; @@ -376,6 +398,11 @@ export class Batch { this.#neutered = true; } + remove() { + this.neuter(); + batches.delete(this); + } + flush() { if (queued_root_effects.length > 0) { flush_effects(); @@ -664,7 +691,7 @@ export function schedule_effect(signal) { export function suspend() { var boundary = get_pending_boundary(); - var batch = (boundary.batch ??= /** @type {Batch} */ (current_batch)); + var batch = boundary.get_batch(); var pending = boundary.pending; boundary.update_pending_count(1); diff --git a/packages/svelte/src/internal/client/reactivity/deriveds.js b/packages/svelte/src/internal/client/reactivity/deriveds.js index 0ac8582933..8e5d75443d 100644 --- a/packages/svelte/src/internal/client/reactivity/deriveds.js +++ b/packages/svelte/src/internal/client/reactivity/deriveds.js @@ -1,5 +1,4 @@ /** @import { Derived, Effect, Source } from '#client' */ -/** @import { Batch } from './batch.js'; */ import { DEV } from 'esm-env'; import { ERROR_VALUE, @@ -33,7 +32,7 @@ import { tracing_mode_flag } from '../../flags/index.js'; import { Boundary } from '../dom/blocks/boundary.js'; import { component_context } from '../context.js'; import { UNINITIALIZED } from '../../../constants.js'; -import { batch_deriveds, current_batch } from './batch.js'; +import { batch_deriveds } from './batch.js'; import { unset_context } from './async.js'; /** @type {Effect | null} */ @@ -131,7 +130,7 @@ export function async_derived(fn, location) { prev = promise; - var batch = (boundary.batch ??= /** @type {Batch} */ (current_batch)); + var batch = boundary.get_batch(); var pending = boundary.pending; if (should_suspend) { From 0ee1d5663506c05e21c8f6300b6a965ef4379578 Mon Sep 17 00:00:00 2001 From: Simon Holthausen Date: Fri, 15 Aug 2025 22:51:50 +0200 Subject: [PATCH 3/9] fix memory leak, fix branch commit bug --- .changeset/tame-ears-invite.md | 5 ++ .../src/internal/client/dom/blocks/each.js | 4 +- .../src/internal/client/dom/blocks/if.js | 4 +- .../src/internal/client/dom/blocks/key.js | 4 +- .../client/dom/blocks/svelte-component.js | 4 +- .../src/internal/client/reactivity/batch.js | 56 +++++++++++-------- .../internal/client/reactivity/deriveds.js | 6 -- .../samples/async-redirect/_config.js | 2 + .../samples/async-redirect/main.svelte | 4 ++ 9 files changed, 52 insertions(+), 37 deletions(-) create mode 100644 .changeset/tame-ears-invite.md diff --git a/.changeset/tame-ears-invite.md b/.changeset/tame-ears-invite.md new file mode 100644 index 0000000000..05670a0add --- /dev/null +++ b/.changeset/tame-ears-invite.md @@ -0,0 +1,5 @@ +--- +'svelte': patch +--- + +fix: ensure obsolete batches are removed and its necessary dom changes committed diff --git a/packages/svelte/src/internal/client/dom/blocks/each.js b/packages/svelte/src/internal/client/dom/blocks/each.js index 006bf09257..28c706134a 100644 --- a/packages/svelte/src/internal/client/dom/blocks/each.js +++ b/packages/svelte/src/internal/client/dom/blocks/each.js @@ -187,7 +187,7 @@ export function each(node, flags, get_collection, get_key, render_fn, fallback_f } } - block(() => { + var b = block(() => { // store a reference to the effect so that we can update the start/end nodes in reconciliation each_effect ??= /** @type {Effect} */ (active_effect); @@ -310,7 +310,7 @@ export function each(node, flags, get_collection, get_key, render_fn, fallback_f } } - batch.add_callback(commit); + batch.add_callback(() => b, commit); } else { commit(); } diff --git a/packages/svelte/src/internal/client/dom/blocks/if.js b/packages/svelte/src/internal/client/dom/blocks/if.js index f418d46538..1a69d59b87 100644 --- a/packages/svelte/src/internal/client/dom/blocks/if.js +++ b/packages/svelte/src/internal/client/dom/blocks/if.js @@ -124,7 +124,7 @@ export function if_block(node, fn, elseif = false) { if (active) batch.skipped_effects.delete(active); if (inactive) batch.skipped_effects.add(inactive); - batch.add_callback(commit); + batch.add_callback(() => b, commit); } else { commit(); } @@ -135,7 +135,7 @@ export function if_block(node, fn, elseif = false) { } }; - block(() => { + var b = block(() => { has_branch = false; fn(set_branch); if (!has_branch) { diff --git a/packages/svelte/src/internal/client/dom/blocks/key.js b/packages/svelte/src/internal/client/dom/blocks/key.js index 5e3c42019f..c0b12f4017 100644 --- a/packages/svelte/src/internal/client/dom/blocks/key.js +++ b/packages/svelte/src/internal/client/dom/blocks/key.js @@ -52,7 +52,7 @@ export function key(node, get_key, render_fn) { effect = pending_effect; } - block(() => { + var b = block(() => { if (changed(key, (key = get_key()))) { var target = anchor; @@ -66,7 +66,7 @@ export function key(node, get_key, render_fn) { pending_effect = branch(() => render_fn(target)); if (defer) { - /** @type {Batch} */ (current_batch).add_callback(commit); + /** @type {Batch} */ (current_batch).add_callback(() => b, commit); } else { commit(); } diff --git a/packages/svelte/src/internal/client/dom/blocks/svelte-component.js b/packages/svelte/src/internal/client/dom/blocks/svelte-component.js index 2697722b39..fa7356eae6 100644 --- a/packages/svelte/src/internal/client/dom/blocks/svelte-component.js +++ b/packages/svelte/src/internal/client/dom/blocks/svelte-component.js @@ -51,7 +51,7 @@ export function component(node, get_component, render_fn) { pending_effect = null; } - block(() => { + var b = block(() => { if (component === (component = get_component())) return; var defer = should_defer_append(); @@ -70,7 +70,7 @@ export function component(node, get_component, render_fn) { } if (defer) { - /** @type {Batch} */ (current_batch).add_callback(commit); + /** @type {Batch} */ (current_batch).add_callback(() => b, commit); } else { commit(); } diff --git a/packages/svelte/src/internal/client/reactivity/batch.js b/packages/svelte/src/internal/client/reactivity/batch.js index d102355071..f69b2401d0 100644 --- a/packages/svelte/src/internal/client/reactivity/batch.js +++ b/packages/svelte/src/internal/client/reactivity/batch.js @@ -95,10 +95,12 @@ export class Batch { /** * When the batch is committed (and the DOM is updated), we need to remove old branches - * and append new ones by calling the functions added inside (if/each/key/etc) blocks - * @type {Set<() => void>} + * and append new ones by calling the functions added inside (if/each/key/etc) blocks. + * Key is a function that returns the block effect because #callbacks will be called before + * the block effect reference exists, so we need to capture it in a closure. + * @type {Map<() => Effect, () => void>} */ - #callbacks = new Set(); + #callbacks = new Map(); /** * The number of async effects that are currently in flight @@ -112,12 +114,6 @@ export class Batch { */ #deferred = null; - /** - * True if an async effect inside this batch resolved and - * its parent branch was already deleted - */ - #neutered = false; - /** * Async effects (created inside `async_derived`) encountered during processing. * These run after the rest of the batch has updated, since they should @@ -233,8 +229,20 @@ export class Batch { // if we didn't start any new async work, and no async work // is outstanding from a previous flush, commit if (this.#async_effects.length === 0 && this.#pending === 0) { - for (const batch of superseeded_batches) { - batch.remove(); + if (superseeded_batches.length > 0) { + const own = [...this.#callbacks.keys()].map((c) => c()); + for (const batch of superseeded_batches) { + // A superseeded batch could have callbacks for e.g. destroying if blocks + // that are not part of the current batch because it already happened in the prior one, + // and the corresponding block effect therefore returning early because nothing was changed from its + // point of view, therefore not adding a callback to the current batch, so we gotta call them here. + for (const [effect, cb] of batch.#callbacks) { + if (!own.includes(effect())) { + cb(); + } + } + batch.remove(); + } } this.#commit(); @@ -394,12 +402,13 @@ export class Batch { } } - neuter() { - this.#neutered = true; - } - remove() { - this.neuter(); + this.#callbacks.clear(); + this.#maybe_dirty_effects = + this.#dirty_effects = + this.#boundary_async_effects = + this.#async_effects = + []; batches.delete(this); } @@ -427,10 +436,8 @@ export class Batch { * Append and remove branches to/from the DOM */ #commit() { - if (!this.#neutered) { - for (const fn of this.#callbacks) { - fn(); - } + for (const fn of this.#callbacks.values()) { + fn(); } this.#callbacks.clear(); @@ -463,9 +470,12 @@ export class Batch { } } - /** @param {() => void} fn */ - add_callback(fn) { - this.#callbacks.add(fn); + /** + * @param {() => Effect} effect + * @param {() => void} fn + */ + add_callback(effect, fn) { + this.#callbacks.set(effect, fn); } settled() { diff --git a/packages/svelte/src/internal/client/reactivity/deriveds.js b/packages/svelte/src/internal/client/reactivity/deriveds.js index 8e5d75443d..a03f234300 100644 --- a/packages/svelte/src/internal/client/reactivity/deriveds.js +++ b/packages/svelte/src/internal/client/reactivity/deriveds.js @@ -184,12 +184,6 @@ export function async_derived(fn, location) { }; promise.then(handler, (e) => handler(null, e || 'unknown')); - - if (batch) { - return () => { - queueMicrotask(() => batch.neuter()); - }; - } }); if (DEV) { diff --git a/packages/svelte/tests/runtime-runes/samples/async-redirect/_config.js b/packages/svelte/tests/runtime-runes/samples/async-redirect/_config.js index ebbe642860..fe92977c21 100644 --- a/packages/svelte/tests/runtime-runes/samples/async-redirect/_config.js +++ b/packages/svelte/tests/runtime-runes/samples/async-redirect/_config.js @@ -29,6 +29,7 @@ export default test({

c

+

b or c

` ); @@ -46,6 +47,7 @@ export default test({

b

+

b or c

` ); } diff --git a/packages/svelte/tests/runtime-runes/samples/async-redirect/main.svelte b/packages/svelte/tests/runtime-runes/samples/async-redirect/main.svelte index bf5fdf9ed3..aead1b00e5 100644 --- a/packages/svelte/tests/runtime-runes/samples/async-redirect/main.svelte +++ b/packages/svelte/tests/runtime-runes/samples/async-redirect/main.svelte @@ -33,6 +33,10 @@

c

{/if} + {#if route === 'b' || route === 'c'} +

b or c

+ {/if} + {#snippet pending()}

pending...

{/snippet} From 5d6b755e68b67170090dec7ddab1a98c624970fd Mon Sep 17 00:00:00 2001 From: Simon Holthausen Date: Fri, 15 Aug 2025 23:02:55 +0200 Subject: [PATCH 4/9] belts and braces --- .../svelte/src/internal/client/reactivity/batch.js | 12 +++++++----- 1 file changed, 7 insertions(+), 5 deletions(-) diff --git a/packages/svelte/src/internal/client/reactivity/batch.js b/packages/svelte/src/internal/client/reactivity/batch.js index f69b2401d0..bf9a63c076 100644 --- a/packages/svelte/src/internal/client/reactivity/batch.js +++ b/packages/svelte/src/internal/client/reactivity/batch.js @@ -231,14 +231,16 @@ export class Batch { if (this.#async_effects.length === 0 && this.#pending === 0) { if (superseeded_batches.length > 0) { const own = [...this.#callbacks.keys()].map((c) => c()); - for (const batch of superseeded_batches) { - // A superseeded batch could have callbacks for e.g. destroying if blocks - // that are not part of the current batch because it already happened in the prior one, - // and the corresponding block effect therefore returning early because nothing was changed from its - // point of view, therefore not adding a callback to the current batch, so we gotta call them here. + // A superseeded batch could have callbacks for e.g. destroying if blocks + // that are not part of the current batch because it already happened in the prior one, + // and the corresponding block effect therefore returning early because nothing was changed from its + // point of view, therefore not adding a callback to the current batch, so we gotta call them here. + // We do it from newest to oldest to ensure the correct callback is applied. + for (const batch of superseeded_batches.reverse()) { for (const [effect, cb] of batch.#callbacks) { if (!own.includes(effect())) { cb(); + own.push(effect()); } } batch.remove(); From bde51cde893ed22381e473dbc6776d283a184254 Mon Sep 17 00:00:00 2001 From: Rich Harris Date: Mon, 18 Aug 2025 09:19:59 -0400 Subject: [PATCH 5/9] typo --- .../src/internal/client/reactivity/batch.js | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/packages/svelte/src/internal/client/reactivity/batch.js b/packages/svelte/src/internal/client/reactivity/batch.js index bf9a63c076..357669d870 100644 --- a/packages/svelte/src/internal/client/reactivity/batch.js +++ b/packages/svelte/src/internal/client/reactivity/batch.js @@ -181,12 +181,12 @@ export class Batch { var current_values = null; /** - * A batch is superseeded if all of its sources are also in the current batch. + * A batch is superseded if all of its sources are also in the current batch. * If the current batch commits, we don't need the old batch anymore. * This also prevents memory leaks since the old batch will never be committed. * @type {Batch[]} */ - var superseeded_batches = []; + var superseded_batches = []; // if there are multiple batches, we are 'time travelling' — // we need to undo the changes belonging to any batch @@ -208,17 +208,17 @@ export class Batch { continue; } - let superseeded = is_prior_batch; + let superseded = is_prior_batch; for (const [source, previous] of batch.#previous) { if (!current_values.has(source)) { - superseeded = false; + superseded = false; current_values.set(source, { v: source.v, wv: source.wv }); source.v = previous; } } - if (superseeded) superseeded_batches.push(batch); + if (superseded) superseded_batches.push(batch); } } @@ -229,14 +229,14 @@ export class Batch { // if we didn't start any new async work, and no async work // is outstanding from a previous flush, commit if (this.#async_effects.length === 0 && this.#pending === 0) { - if (superseeded_batches.length > 0) { + if (superseded_batches.length > 0) { const own = [...this.#callbacks.keys()].map((c) => c()); - // A superseeded batch could have callbacks for e.g. destroying if blocks + // A superseded batch could have callbacks for e.g. destroying if blocks // that are not part of the current batch because it already happened in the prior one, // and the corresponding block effect therefore returning early because nothing was changed from its // point of view, therefore not adding a callback to the current batch, so we gotta call them here. // We do it from newest to oldest to ensure the correct callback is applied. - for (const batch of superseeded_batches.reverse()) { + for (const batch of superseded_batches.reverse()) { for (const [effect, cb] of batch.#callbacks) { if (!own.includes(effect())) { cb(); From 3ee6ea47c88dc52290896f81a9fec3084b30f158 Mon Sep 17 00:00:00 2001 From: Simon Holthausen Date: Mon, 25 Aug 2025 20:57:59 +0200 Subject: [PATCH 6/9] adjust, add test that fails without remove clearing arrays and sets --- .changeset/spicy-ears-join.md | 5 --- .../internal/client/dom/blocks/boundary.js | 20 --------- .../src/internal/client/reactivity/batch.js | 6 ++- .../internal/client/reactivity/deriveds.js | 5 ++- .../samples/async-redirect-2/_config.js | 36 ++++++++++++++++ .../samples/async-redirect-2/main.svelte | 41 +++++++++++++++++++ 6 files changed, 84 insertions(+), 29 deletions(-) delete mode 100644 .changeset/spicy-ears-join.md create mode 100644 packages/svelte/tests/runtime-runes/samples/async-redirect-2/_config.js create mode 100644 packages/svelte/tests/runtime-runes/samples/async-redirect-2/main.svelte diff --git a/.changeset/spicy-ears-join.md b/.changeset/spicy-ears-join.md deleted file mode 100644 index ee29e77afe..0000000000 --- a/.changeset/spicy-ears-join.md +++ /dev/null @@ -1,5 +0,0 @@ ---- -'svelte': patch ---- - -fix: associate batch with boundary diff --git a/packages/svelte/src/internal/client/dom/blocks/boundary.js b/packages/svelte/src/internal/client/dom/blocks/boundary.js index 350a417b8e..3dc3cb456b 100644 --- a/packages/svelte/src/internal/client/dom/blocks/boundary.js +++ b/packages/svelte/src/internal/client/dom/blocks/boundary.js @@ -59,18 +59,6 @@ export class Boundary { /** @type {Boundary | null} */ parent; - /** - * The associated batch to this boundary while the boundary pending; set by the one interacting with the boundary when entering pending state. - * Will be `null` once the boundary is no longer pending. - * - * Needed because `current_batch` isn't guaranteed to exist: E.g. when component A has top level await, then renders component B - * which also has top level await, `current_batch` can be null when a flush from component A happens before - * suspend() in component B is called. We hence save it on the boundary instead. - * - * @type {Batch | null} - */ - #batch = null; - /** @type {TemplateNode} */ #anchor; @@ -200,13 +188,6 @@ export class Boundary { return !!this.#props.pending; } - get_batch() { - if (current_batch) { - this.#batch = current_batch; - } - return /** @type {Batch} */ (this.#batch); - } - /** * @param {() => Effect | null} fn */ @@ -250,7 +231,6 @@ export class Boundary { if (this.#pending_count === 0) { this.pending = false; - this.#batch = null; if (this.#pending_effect) { pause_effect(this.#pending_effect, () => { diff --git a/packages/svelte/src/internal/client/reactivity/batch.js b/packages/svelte/src/internal/client/reactivity/batch.js index 8f81321246..7933401496 100644 --- a/packages/svelte/src/internal/client/reactivity/batch.js +++ b/packages/svelte/src/internal/client/reactivity/batch.js @@ -10,7 +10,6 @@ import { INERT, RENDER_EFFECT, ROOT_EFFECT, - USER_EFFECT, MAYBE_DIRTY } from '#client/constants'; import { async_mode_flag } from '../../flags/index.js'; @@ -405,6 +404,9 @@ export class Batch { } remove() { + // Cleanup to + // - prevent memory leaks which could happen if a batch is tied to a never-ending promise + // - prevent effects from rerunning for outdated-and-now-no-longer-pending batches this.#callbacks.clear(); this.#maybe_dirty_effects = this.#dirty_effects = @@ -705,7 +707,7 @@ export function schedule_effect(signal) { export function suspend() { var boundary = get_pending_boundary(); - var batch = boundary.get_batch(); + var batch = /** @type {Batch} */ (current_batch); var pending = boundary.pending; boundary.update_pending_count(1); diff --git a/packages/svelte/src/internal/client/reactivity/deriveds.js b/packages/svelte/src/internal/client/reactivity/deriveds.js index a03f234300..be70e358a2 100644 --- a/packages/svelte/src/internal/client/reactivity/deriveds.js +++ b/packages/svelte/src/internal/client/reactivity/deriveds.js @@ -1,4 +1,5 @@ /** @import { Derived, Effect, Source } from '#client' */ +/** @import { Batch } from './batch.js'; */ import { DEV } from 'esm-env'; import { ERROR_VALUE, @@ -32,7 +33,7 @@ import { tracing_mode_flag } from '../../flags/index.js'; import { Boundary } from '../dom/blocks/boundary.js'; import { component_context } from '../context.js'; import { UNINITIALIZED } from '../../../constants.js'; -import { batch_deriveds } from './batch.js'; +import { batch_deriveds, current_batch } from './batch.js'; import { unset_context } from './async.js'; /** @type {Effect | null} */ @@ -130,7 +131,7 @@ export function async_derived(fn, location) { prev = promise; - var batch = boundary.get_batch(); + var batch = /** @type {Batch} */ (current_batch); var pending = boundary.pending; if (should_suspend) { diff --git a/packages/svelte/tests/runtime-runes/samples/async-redirect-2/_config.js b/packages/svelte/tests/runtime-runes/samples/async-redirect-2/_config.js new file mode 100644 index 0000000000..de9b948cd3 --- /dev/null +++ b/packages/svelte/tests/runtime-runes/samples/async-redirect-2/_config.js @@ -0,0 +1,36 @@ +import { tick } from 'svelte'; +import { test } from '../../test'; + +export default test({ + async test({ assert, logs, target }) { + assert.htmlEqual( + target.innerHTML, + ` +

a

+ + + +

a

+ ` + ); + + const [a, b] = target.querySelectorAll('button'); + + b.click(); + await tick(); + + assert.htmlEqual( + target.innerHTML, + ` +

c

+ + + +

c

+

b or c

+ ` + ); + + assert.deepEqual(logs, ['route a', 'route c']); + } +}); diff --git a/packages/svelte/tests/runtime-runes/samples/async-redirect-2/main.svelte b/packages/svelte/tests/runtime-runes/samples/async-redirect-2/main.svelte new file mode 100644 index 0000000000..eb1e90cd30 --- /dev/null +++ b/packages/svelte/tests/runtime-runes/samples/async-redirect-2/main.svelte @@ -0,0 +1,41 @@ + + +

{route}

+ + + + + + {#if route === 'a'} +

a

+ {/if} + + {#if route === 'b'} + {await goto('c')} + {/if} + + {#if route === 'c'} +

c

+ {/if} + + {#if route === 'b' || route === 'c'} +

b or c

+ {/if} + + {#snippet pending()} +

pending...

+ {/snippet} +
From 28be8c94e747fb53a154dc92654b769cc515664a Mon Sep 17 00:00:00 2001 From: Simon Holthausen Date: Mon, 25 Aug 2025 20:59:51 +0200 Subject: [PATCH 7/9] unused --- .../svelte/src/internal/client/dom/blocks/boundary.js | 9 ++------- 1 file changed, 2 insertions(+), 7 deletions(-) diff --git a/packages/svelte/src/internal/client/dom/blocks/boundary.js b/packages/svelte/src/internal/client/dom/blocks/boundary.js index 3dc3cb456b..60fe2b7d3c 100644 --- a/packages/svelte/src/internal/client/dom/blocks/boundary.js +++ b/packages/svelte/src/internal/client/dom/blocks/boundary.js @@ -1,10 +1,5 @@ /** @import { Effect, Source, TemplateNode, } from '#client' */ -import { - BOUNDARY_EFFECT, - EFFECT_PRESERVED, - EFFECT_RAN, - EFFECT_TRANSPARENT -} from '#client/constants'; +import { BOUNDARY_EFFECT, EFFECT_PRESERVED, EFFECT_TRANSPARENT } from '#client/constants'; import { component_context, set_component_context } from '../../context.js'; import { handle_error, invoke_error_boundary } from '../../error-handling.js'; import { block, branch, destroy_effect, pause_effect } from '../../reactivity/effects.js'; @@ -28,7 +23,7 @@ import { queue_micro_task } from '../task.js'; import * as e from '../../errors.js'; import * as w from '../../warnings.js'; import { DEV } from 'esm-env'; -import { Batch, current_batch, effect_pending_updates } from '../../reactivity/batch.js'; +import { Batch, effect_pending_updates } from '../../reactivity/batch.js'; import { internal_set, source } from '../../reactivity/sources.js'; import { tag } from '../../dev/tracing.js'; import { createSubscriber } from '../../../../reactivity/create-subscriber.js'; From 18a790cb64baf47ac8f70ae652b8af39a3b97962 Mon Sep 17 00:00:00 2001 From: Rich Harris Date: Sat, 30 Aug 2025 09:10:49 -0400 Subject: [PATCH 8/9] simplify --- .../svelte/src/internal/client/dom/blocks/each.js | 4 ++-- .../svelte/src/internal/client/dom/blocks/if.js | 4 ++-- .../svelte/src/internal/client/dom/blocks/key.js | 4 ++-- .../internal/client/dom/blocks/svelte-component.js | 4 ++-- .../svelte/src/internal/client/reactivity/batch.js | 13 ++++++------- 5 files changed, 14 insertions(+), 15 deletions(-) diff --git a/packages/svelte/src/internal/client/dom/blocks/each.js b/packages/svelte/src/internal/client/dom/blocks/each.js index 28c706134a..006bf09257 100644 --- a/packages/svelte/src/internal/client/dom/blocks/each.js +++ b/packages/svelte/src/internal/client/dom/blocks/each.js @@ -187,7 +187,7 @@ export function each(node, flags, get_collection, get_key, render_fn, fallback_f } } - var b = block(() => { + block(() => { // store a reference to the effect so that we can update the start/end nodes in reconciliation each_effect ??= /** @type {Effect} */ (active_effect); @@ -310,7 +310,7 @@ export function each(node, flags, get_collection, get_key, render_fn, fallback_f } } - batch.add_callback(() => b, commit); + batch.add_callback(commit); } else { commit(); } diff --git a/packages/svelte/src/internal/client/dom/blocks/if.js b/packages/svelte/src/internal/client/dom/blocks/if.js index 1a69d59b87..f418d46538 100644 --- a/packages/svelte/src/internal/client/dom/blocks/if.js +++ b/packages/svelte/src/internal/client/dom/blocks/if.js @@ -124,7 +124,7 @@ export function if_block(node, fn, elseif = false) { if (active) batch.skipped_effects.delete(active); if (inactive) batch.skipped_effects.add(inactive); - batch.add_callback(() => b, commit); + batch.add_callback(commit); } else { commit(); } @@ -135,7 +135,7 @@ export function if_block(node, fn, elseif = false) { } }; - var b = block(() => { + block(() => { has_branch = false; fn(set_branch); if (!has_branch) { diff --git a/packages/svelte/src/internal/client/dom/blocks/key.js b/packages/svelte/src/internal/client/dom/blocks/key.js index c0b12f4017..5e3c42019f 100644 --- a/packages/svelte/src/internal/client/dom/blocks/key.js +++ b/packages/svelte/src/internal/client/dom/blocks/key.js @@ -52,7 +52,7 @@ export function key(node, get_key, render_fn) { effect = pending_effect; } - var b = block(() => { + block(() => { if (changed(key, (key = get_key()))) { var target = anchor; @@ -66,7 +66,7 @@ export function key(node, get_key, render_fn) { pending_effect = branch(() => render_fn(target)); if (defer) { - /** @type {Batch} */ (current_batch).add_callback(() => b, commit); + /** @type {Batch} */ (current_batch).add_callback(commit); } else { commit(); } diff --git a/packages/svelte/src/internal/client/dom/blocks/svelte-component.js b/packages/svelte/src/internal/client/dom/blocks/svelte-component.js index fa7356eae6..2697722b39 100644 --- a/packages/svelte/src/internal/client/dom/blocks/svelte-component.js +++ b/packages/svelte/src/internal/client/dom/blocks/svelte-component.js @@ -51,7 +51,7 @@ export function component(node, get_component, render_fn) { pending_effect = null; } - var b = block(() => { + block(() => { if (component === (component = get_component())) return; var defer = should_defer_append(); @@ -70,7 +70,7 @@ export function component(node, get_component, render_fn) { } if (defer) { - /** @type {Batch} */ (current_batch).add_callback(() => b, commit); + /** @type {Batch} */ (current_batch).add_callback(commit); } else { commit(); } diff --git a/packages/svelte/src/internal/client/reactivity/batch.js b/packages/svelte/src/internal/client/reactivity/batch.js index 9ac9987f21..b720c71a03 100644 --- a/packages/svelte/src/internal/client/reactivity/batch.js +++ b/packages/svelte/src/internal/client/reactivity/batch.js @@ -97,7 +97,7 @@ export class Batch { * and append new ones by calling the functions added inside (if/each/key/etc) blocks. * Key is a function that returns the block effect because #callbacks will be called before * the block effect reference exists, so we need to capture it in a closure. - * @type {Map<() => Effect, () => void>} + * @type {Map void>} */ #callbacks = new Map(); @@ -229,7 +229,7 @@ export class Batch { // is outstanding from a previous flush, commit if (this.#async_effects.length === 0 && this.#pending === 0) { if (superseded_batches.length > 0) { - const own = [...this.#callbacks.keys()].map((c) => c()); + const own = [...this.#callbacks.keys()]; // A superseded batch could have callbacks for e.g. destroying if blocks // that are not part of the current batch because it already happened in the prior one, // and the corresponding block effect therefore returning early because nothing was changed from its @@ -237,9 +237,9 @@ export class Batch { // We do it from newest to oldest to ensure the correct callback is applied. for (const batch of superseded_batches.reverse()) { for (const [effect, cb] of batch.#callbacks) { - if (!own.includes(effect())) { + if (!own.includes(effect)) { cb(); - own.push(effect()); + own.push(effect); } } batch.remove(); @@ -475,11 +475,10 @@ export class Batch { } /** - * @param {() => Effect} effect * @param {() => void} fn */ - add_callback(effect, fn) { - this.#callbacks.set(effect, fn); + add_callback(fn) { + this.#callbacks.set(/** @type {Effect} */ (active_effect), fn); } settled() { From b88b4e8ef82f2886798a4514a49e34ff713b7493 Mon Sep 17 00:00:00 2001 From: Rich Harris Date: Sat, 30 Aug 2025 09:33:37 -0400 Subject: [PATCH 9/9] comment is no longer true --- packages/svelte/src/internal/client/reactivity/batch.js | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/packages/svelte/src/internal/client/reactivity/batch.js b/packages/svelte/src/internal/client/reactivity/batch.js index b720c71a03..d8d8286b9b 100644 --- a/packages/svelte/src/internal/client/reactivity/batch.js +++ b/packages/svelte/src/internal/client/reactivity/batch.js @@ -94,9 +94,7 @@ export class Batch { /** * When the batch is committed (and the DOM is updated), we need to remove old branches - * and append new ones by calling the functions added inside (if/each/key/etc) blocks. - * Key is a function that returns the block effect because #callbacks will be called before - * the block effect reference exists, so we need to capture it in a closure. + * and append new ones by calling the functions added inside (if/each/key/etc) blocks * @type {Map void>} */ #callbacks = new Map();