From 3ee6ea47c88dc52290896f81a9fec3084b30f158 Mon Sep 17 00:00:00 2001 From: Simon Holthausen Date: Mon, 25 Aug 2025 20:57:59 +0200 Subject: [PATCH] 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} +