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/reactivity/batch.js b/packages/svelte/src/internal/client/reactivity/batch.js index 82f1de67a9..d8d8286b9b 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'; @@ -96,9 +95,9 @@ 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>} + * @type {Map void>} */ - #callbacks = new Set(); + #callbacks = new Map(); /** * The number of async effects that are currently in flight @@ -112,12 +111,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 @@ -184,6 +177,14 @@ export class Batch { /** @type {Map | null} */ var current_values = null; + /** + * 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 superseded_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 +197,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 superseded = is_prior_batch; for (const [source, previous] of batch.#previous) { if (!current_values.has(source)) { + superseded = false; current_values.set(source, { v: source.v, wv: source.wv }); source.v = previous; } } + + if (superseded) superseded_batches.push(batch); } } @@ -215,6 +226,24 @@ 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 (superseded_batches.length > 0) { + 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 + // 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 superseded_batches.reverse()) { + for (const [effect, cb] of batch.#callbacks) { + if (!own.includes(effect)) { + cb(); + own.push(effect); + } + } + batch.remove(); + } + } + this.#commit(); var render_effects = this.#render_effects; @@ -372,8 +401,17 @@ export class Batch { } } - neuter() { - this.#neutered = true; + 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 = + this.#boundary_async_effects = + this.#async_effects = + []; + batches.delete(this); } flush() { @@ -400,10 +438,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(); @@ -436,9 +472,11 @@ export class Batch { } } - /** @param {() => void} fn */ + /** + * @param {() => void} fn + */ add_callback(fn) { - this.#callbacks.add(fn); + this.#callbacks.set(/** @type {Effect} */ (active_effect), fn); } settled() { diff --git a/packages/svelte/src/internal/client/reactivity/deriveds.js b/packages/svelte/src/internal/client/reactivity/deriveds.js index 31dc267960..f0037f6405 100644 --- a/packages/svelte/src/internal/client/reactivity/deriveds.js +++ b/packages/svelte/src/internal/client/reactivity/deriveds.js @@ -188,12 +188,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-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} +