From e14561c8293fcfd27456df84327cc4f4700aacfb Mon Sep 17 00:00:00 2001 From: Simon Holthausen Date: Thu, 14 Aug 2025 16:03:27 +0200 Subject: [PATCH 1/7] 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 000000000000..ee29e77afec2 --- /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 1866931ef2fd..b7fd9a856c53 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 123bc95d163a..b3a3bef73121 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 7f730e365e5a..0ac85829335b 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 000000000000..f1ac9ab760cf --- /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 000000000000..e2029a3033ed --- /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 000000000000..ca7965bf79e0 --- /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 000000000000..bd0efaa4f85c --- /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 000000000000..e8a7c8413748 --- /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 000000000000..2c7ffd395265 --- /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 000000000000..bd0efaa4f85c --- /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/7] 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 b7fd9a856c53..350a417b8eb3 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 b3a3bef73121..d10235507193 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 0ac85829335b..8e5d75443dbb 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/7] 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 000000000000..05670a0add38 --- /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 006bf09257d1..28c706134ab4 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 f418d465388e..1a69d59b8731 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 5e3c42019f33..c0b12f4017f5 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 2697722b3953..fa7356eae69e 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 d10235507193..f69b2401d0f7 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 8e5d75443dbb..a03f234300a5 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 ebbe642860d0..fe92977c21f4 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 bf5fdf9ed395..aead1b00e597 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/7] 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 f69b2401d0f7..bf9a63c0766d 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/7] 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 bf9a63c0766d..357669d870c7 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/7] 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 ee29e77afec2..000000000000 --- 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 350a417b8eb3..3dc3cb456bc7 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 8f813212469e..7933401496c7 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 a03f234300a5..be70e358a22a 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 000000000000..de9b948cd32d --- /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 000000000000..eb1e90cd307b --- /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/7] 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 3dc3cb456bc7..60fe2b7d3cc1 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';