diff --git a/.changeset/mighty-mice-call.md b/.changeset/mighty-mice-call.md new file mode 100644 index 000000000000..340b33bd4b93 --- /dev/null +++ b/.changeset/mighty-mice-call.md @@ -0,0 +1,5 @@ +--- +'svelte': patch +--- + +fix: keep batches alive until all async work is complete diff --git a/packages/svelte/src/internal/client/dom/blocks/async.js b/packages/svelte/src/internal/client/dom/blocks/async.js index 6df0739918ed..5ee9d25bce75 100644 --- a/packages/svelte/src/internal/client/dom/blocks/async.js +++ b/packages/svelte/src/internal/client/dom/blocks/async.js @@ -1,5 +1,6 @@ /** @import { TemplateNode, Value } from '#client' */ import { flatten } from '../../reactivity/async.js'; +import { Batch, current_batch } from '../../reactivity/batch.js'; import { get } from '../../runtime.js'; import { hydrate_next, @@ -18,8 +19,11 @@ import { get_boundary } from './boundary.js'; */ export function async(node, expressions, fn) { var boundary = get_boundary(); + var batch = /** @type {Batch} */ (current_batch); + var blocking = !boundary.is_pending(); boundary.update_pending_count(1); + batch.increment(blocking); var was_hydrating = hydrating; @@ -44,6 +48,7 @@ export function async(node, expressions, fn) { fn(node, ...values); } finally { boundary.update_pending_count(-1); + batch.decrement(blocking); } if (was_hydrating) { diff --git a/packages/svelte/src/internal/client/dom/blocks/boundary.js b/packages/svelte/src/internal/client/dom/blocks/boundary.js index 026ffb36fc51..3da920457170 100644 --- a/packages/svelte/src/internal/client/dom/blocks/boundary.js +++ b/packages/svelte/src/internal/client/dom/blocks/boundary.js @@ -291,13 +291,6 @@ export class Boundary { this.#anchor.before(this.#offscreen_fragment); this.#offscreen_fragment = null; } - - // TODO this feels like a little bit of a kludge, but until we - // overhaul the boundary/batch relationship it's probably - // the most pragmatic solution available to us - queue_micro_task(() => { - Batch.ensure().flush(); - }); } } diff --git a/packages/svelte/src/internal/client/reactivity/async.js b/packages/svelte/src/internal/client/reactivity/async.js index 1d408744fdfc..fb836df98914 100644 --- a/packages/svelte/src/internal/client/reactivity/async.js +++ b/packages/svelte/src/internal/client/reactivity/async.js @@ -218,10 +218,10 @@ export function unset_context() { export async function async_body(anchor, fn) { var boundary = get_boundary(); var batch = /** @type {Batch} */ (current_batch); - var pending = boundary.is_pending(); + var blocking = !boundary.is_pending(); boundary.update_pending_count(1); - if (!pending) batch.increment(); + batch.increment(blocking); var active = /** @type {Effect} */ (active_effect); @@ -254,12 +254,7 @@ export async function async_body(anchor, fn) { } boundary.update_pending_count(-1); - - if (pending) { - batch.flush(); - } else { - batch.decrement(); - } + batch.decrement(blocking); unset_context(); } diff --git a/packages/svelte/src/internal/client/reactivity/batch.js b/packages/svelte/src/internal/client/reactivity/batch.js index 302d48d03fe2..91635bd5d2b4 100644 --- a/packages/svelte/src/internal/client/reactivity/batch.js +++ b/packages/svelte/src/internal/client/reactivity/batch.js @@ -11,7 +11,8 @@ import { RENDER_EFFECT, ROOT_EFFECT, MAYBE_DIRTY, - DERIVED + DERIVED, + BOUNDARY_EFFECT } from '#client/constants'; import { async_mode_flag } from '../../flags/index.js'; import { deferred, define_property } from '../../shared/utils.js'; @@ -31,6 +32,16 @@ import { invoke_error_boundary } from '../error-handling.js'; import { old_values, source, update } from './sources.js'; import { inspect_effect, unlink_effect } from './effects.js'; +/** + * @typedef {{ + * parent: EffectTarget | null; + * effect: Effect | null; + * effects: Effect[]; + * render_effects: Effect[]; + * block_effects: Effect[]; + * }} EffectTarget + */ + /** @type {Set} */ const batches = new Set(); @@ -65,6 +76,8 @@ let is_flushing = false; export let is_flushing_sync = false; export class Batch { + committed = false; + /** * The current values of any sources that are updated in this batch * They keys of this map are identical to `this.#previous` @@ -91,6 +104,11 @@ export class Batch { */ #pending = 0; + /** + * The number of async effects that are currently in flight, _not_ inside a pending boundary + */ + #blocking_pending = 0; + /** * A deferred that resolves when the batch is committed, used with `settled()` * TODO replace with Promise.withResolvers once supported widely enough @@ -98,26 +116,6 @@ export class Batch { */ #deferred = null; - /** - * Template effects and `$effect.pre` effects, which run when - * a batch is committed - * @type {Effect[]} - */ - #render_effects = []; - - /** - * The same as `#render_effects`, but for `$effect` (which runs after) - * @type {Effect[]} - */ - #effects = []; - - /** - * Block effects, which may need to re-run on subsequent flushes - * in order to update internal sources (e.g. each block items) - * @type {Effect[]} - */ - #block_effects = []; - /** * Deferred effects (which run after async work has completed) that are DIRTY * @type {Effect[]} @@ -148,41 +146,37 @@ export class Batch { this.apply(); + /** @type {EffectTarget} */ + var target = { + parent: null, + effect: null, + effects: [], + render_effects: [], + block_effects: [] + }; + for (const root of root_effects) { - this.#traverse_effect_tree(root); + this.#traverse_effect_tree(root, target); } - // if there is no outstanding async work, commit - if (this.#pending === 0) { - // TODO we need this because we commit _then_ flush effects... - // maybe there's a way we can reverse the order? - var previous_batch_sources = batch_values; + this.#resolve(); - this.#commit(); - - var render_effects = this.#render_effects; - var effects = this.#effects; - - this.#render_effects = []; - this.#effects = []; - this.#block_effects = []; + if (this.#blocking_pending > 0) { + this.#defer_effects(target.effects); + this.#defer_effects(target.render_effects); + this.#defer_effects(target.block_effects); + } else { + // TODO append/detach blocks here, not in #commit // If sources are written to, then work needs to happen in a separate batch, else prior sources would be mixed with // newly updated sources, which could lead to infinite loops when effects run over and over again. previous_batch = this; current_batch = null; - batch_values = previous_batch_sources; - flush_queued_effects(render_effects); - flush_queued_effects(effects); + flush_queued_effects(target.render_effects); + flush_queued_effects(target.effects); previous_batch = null; - - this.#deferred?.resolve(); - } else { - this.#defer_effects(this.#render_effects); - this.#defer_effects(this.#effects); - this.#defer_effects(this.#block_effects); } batch_values = null; @@ -192,8 +186,9 @@ export class Batch { * Traverse the effect tree, executing effects or stashing * them for later execution as appropriate * @param {Effect} root + * @param {EffectTarget} target */ - #traverse_effect_tree(root) { + #traverse_effect_tree(root, target) { root.f ^= CLEAN; var effect = root.first; @@ -205,15 +200,25 @@ export class Batch { var skip = is_skippable_branch || (flags & INERT) !== 0 || this.skipped_effects.has(effect); + if ((effect.f & BOUNDARY_EFFECT) !== 0 && effect.b?.is_pending()) { + target = { + parent: target, + effect, + effects: [], + render_effects: [], + block_effects: [] + }; + } + if (!skip && effect.fn !== null) { if (is_branch) { effect.f ^= CLEAN; } else if ((flags & EFFECT) !== 0) { - this.#effects.push(effect); + target.effects.push(effect); } else if (async_mode_flag && (flags & RENDER_EFFECT) !== 0) { - this.#render_effects.push(effect); + target.render_effects.push(effect); } else if (is_dirty(effect)) { - if ((effect.f & BLOCK_EFFECT) !== 0) this.#block_effects.push(effect); + if ((effect.f & BLOCK_EFFECT) !== 0) target.block_effects.push(effect); update_effect(effect); } @@ -229,6 +234,17 @@ export class Batch { effect = effect.next; while (effect === null && parent !== null) { + if (parent === target.effect) { + // TODO rather than traversing into pending boundaries and deferring the effects, + // could we just attach the effects _to_ the pending boundary and schedule them + // once the boundary is ready? + this.#defer_effects(target.effects); + this.#defer_effects(target.render_effects); + this.#defer_effects(target.block_effects); + + target = /** @type {EffectTarget} */ (target.parent); + } + effect = parent.next; parent = parent.parent; } @@ -246,8 +262,6 @@ export class Batch { // mark as clean so they get scheduled if they depend on pending async state set_signal_status(e, CLEAN); } - - effects.length = 0; } /** @@ -283,8 +297,8 @@ export class Batch { // this can happen if a new batch was created during `flush_effects()` return; } - } else if (this.#pending === 0) { - this.#commit(); + } else { + this.#resolve(); } this.deactivate(); @@ -300,16 +314,19 @@ export class Batch { } } - /** - * Append and remove branches to/from the DOM - */ - #commit() { - for (const fn of this.#callbacks) { - fn(); + #resolve() { + if (this.#blocking_pending === 0) { + // append/remove branches + for (const fn of this.#callbacks) fn(); + this.#callbacks.clear(); } - this.#callbacks.clear(); + if (this.#pending === 0) { + this.#commit(); + } + } + #commit() { // If there are other pending batches, they now need to be 'rebased' — // in other words, we re-run block/async effects with the newly // committed state, unless the batch in question has a more @@ -317,7 +334,17 @@ export class Batch { if (batches.size > 1) { this.#previous.clear(); - let is_earlier = true; + var previous_batch_values = batch_values; + var is_earlier = true; + + /** @type {EffectTarget} */ + var dummy_target = { + parent: null, + effect: null, + effects: [], + render_effects: [], + block_effects: [] + }; for (const batch of batches) { if (batch === this) { @@ -359,9 +386,11 @@ export class Batch { batch.apply(); for (const root of queued_root_effects) { - batch.#traverse_effect_tree(root); + batch.#traverse_effect_tree(root, dummy_target); } + // TODO do we need to do anything with `target`? defer block effects? + queued_root_effects = []; batch.deactivate(); } @@ -369,17 +398,31 @@ export class Batch { } current_batch = null; + batch_values = previous_batch_values; } + this.committed = true; batches.delete(this); + + this.#deferred?.resolve(); } - increment() { + /** + * + * @param {boolean} blocking + */ + increment(blocking) { this.#pending += 1; + if (blocking) this.#blocking_pending += 1; } - decrement() { + /** + * + * @param {boolean} blocking + */ + decrement(blocking) { this.#pending -= 1; + if (blocking) this.#blocking_pending -= 1; for (const e of this.#dirty_effects) { set_signal_status(e, DIRTY); @@ -391,6 +434,9 @@ export class Batch { schedule_effect(e); } + this.#dirty_effects = []; + this.#maybe_dirty_effects = []; + this.flush(); } diff --git a/packages/svelte/src/internal/client/reactivity/deriveds.js b/packages/svelte/src/internal/client/reactivity/deriveds.js index 6aa9a1d9d920..06ae0f6d7a2e 100644 --- a/packages/svelte/src/internal/client/reactivity/deriveds.js +++ b/packages/svelte/src/internal/client/reactivity/deriveds.js @@ -127,7 +127,17 @@ export function async_derived(fn, location) { // If this code is changed at some point, make sure to still access the then property // of fn() to read any signals it might access, so that we track them as dependencies. // We call `unset_context` to undo any `save` calls that happen inside `fn()` - Promise.resolve(fn()).then(d.resolve, d.reject).then(unset_context); + Promise.resolve(fn()) + .then(d.resolve, d.reject) + .then(() => { + if (batch === current_batch && batch.committed) { + // if the batch was rejected as stale, we need to cleanup + // after any `$.save(...)` calls inside `fn()` + batch.deactivate(); + } + + unset_context(); + }); } catch (error) { d.reject(error); unset_context(); @@ -136,17 +146,16 @@ export function async_derived(fn, location) { if (DEV) current_async_effect = null; var batch = /** @type {Batch} */ (current_batch); - var pending = boundary.is_pending(); if (should_suspend) { + var blocking = !boundary.is_pending(); + boundary.update_pending_count(1); - if (!pending) { - batch.increment(); + batch.increment(blocking); - deferreds.get(batch)?.reject(STALE_REACTION); - deferreds.delete(batch); // delete to ensure correct order in Map iteration below - deferreds.set(batch, d); - } + deferreds.get(batch)?.reject(STALE_REACTION); + deferreds.delete(batch); // delete to ensure correct order in Map iteration below + deferreds.set(batch, d); } /** @@ -156,7 +165,7 @@ export function async_derived(fn, location) { const handler = (value, error = undefined) => { current_async_effect = null; - if (!pending) batch.activate(); + batch.activate(); if (error) { if (error !== STALE_REACTION) { @@ -193,7 +202,7 @@ export function async_derived(fn, location) { if (should_suspend) { boundary.update_pending_count(-1); - if (!pending) batch.decrement(); + batch.decrement(blocking); } }; diff --git a/packages/svelte/tests/runtime-runes/samples/async-abort-signal/_config.js b/packages/svelte/tests/runtime-runes/samples/async-abort-signal/_config.js index a947a91ab881..af49b1779c01 100644 --- a/packages/svelte/tests/runtime-runes/samples/async-abort-signal/_config.js +++ b/packages/svelte/tests/runtime-runes/samples/async-abort-signal/_config.js @@ -6,7 +6,7 @@ export default test({ const [reset, resolve] = target.querySelectorAll('button'); reset.click(); - await settled(); + await tick(); assert.deepEqual(logs, ['aborted']); resolve.click(); diff --git a/packages/svelte/tests/runtime-runes/samples/async-binding-update-while-focused-2/main.svelte b/packages/svelte/tests/runtime-runes/samples/async-binding-update-while-focused-2/main.svelte index e2f01a66c892..c0e4d862a8e5 100644 --- a/packages/svelte/tests/runtime-runes/samples/async-binding-update-while-focused-2/main.svelte +++ b/packages/svelte/tests/runtime-runes/samples/async-binding-update-while-focused-2/main.svelte @@ -12,7 +12,7 @@ diff --git a/packages/svelte/tests/runtime-runes/samples/async-binding-update-while-focused-3/main.svelte b/packages/svelte/tests/runtime-runes/samples/async-binding-update-while-focused-3/main.svelte index 566ea60ec5cf..8f5e2862eb05 100644 --- a/packages/svelte/tests/runtime-runes/samples/async-binding-update-while-focused-3/main.svelte +++ b/packages/svelte/tests/runtime-runes/samples/async-binding-update-while-focused-3/main.svelte @@ -12,7 +12,7 @@ diff --git a/packages/svelte/tests/runtime-runes/samples/async-block-reject-each-during-init/_config.js b/packages/svelte/tests/runtime-runes/samples/async-block-reject-each-during-init/_config.js index c5dae7fee294..ca5fd9ca8974 100644 --- a/packages/svelte/tests/runtime-runes/samples/async-block-reject-each-during-init/_config.js +++ b/packages/svelte/tests/runtime-runes/samples/async-block-reject-each-during-init/_config.js @@ -24,5 +24,7 @@ export default test({

1

` ); - } + }, + + expect_unhandled_rejections: true }); diff --git a/packages/svelte/tests/runtime-runes/samples/async-block-resolve/_config.js b/packages/svelte/tests/runtime-runes/samples/async-block-resolve/_config.js new file mode 100644 index 000000000000..ee403290bc6a --- /dev/null +++ b/packages/svelte/tests/runtime-runes/samples/async-block-resolve/_config.js @@ -0,0 +1,63 @@ +import { tick } from 'svelte'; +import { test } from '../../test'; + +export default test({ + async test({ assert, target }) { + const [increment, shift] = target.querySelectorAll('button'); + + shift.click(); + await tick(); + + shift.click(); + await tick(); + + assert.htmlEqual( + target.innerHTML, + ` + + +

even

+

0

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

even

+

0

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

odd

+

loading...

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

odd

+

1

+ ` + ); + } +}); diff --git a/packages/svelte/tests/runtime-runes/samples/async-block-resolve/main.svelte b/packages/svelte/tests/runtime-runes/samples/async-block-resolve/main.svelte new file mode 100644 index 000000000000..73fe83889a87 --- /dev/null +++ b/packages/svelte/tests/runtime-runes/samples/async-block-resolve/main.svelte @@ -0,0 +1,36 @@ + + + + + + + {#if await push(count) % 2 === 0} +

even

+ {:else} +

odd

+ {/if} + + {#key count} + +

{await push(count)}

+ + {#snippet pending()} +

loading...

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

loading...

+ {/snippet} +
diff --git a/packages/svelte/tests/runtime-runes/samples/async-effect-after-await/Child.svelte b/packages/svelte/tests/runtime-runes/samples/async-effect-after-await/Child.svelte index 682f7a063179..758ebc00040c 100644 --- a/packages/svelte/tests/runtime-runes/samples/async-effect-after-await/Child.svelte +++ b/packages/svelte/tests/runtime-runes/samples/async-effect-after-await/Child.svelte @@ -1,7 +1,11 @@ diff --git a/packages/svelte/tests/runtime-runes/samples/async-effect-after-await/_config.js b/packages/svelte/tests/runtime-runes/samples/async-effect-after-await/_config.js index 81548a25ea67..0908b6a9feb9 100644 --- a/packages/svelte/tests/runtime-runes/samples/async-effect-after-await/_config.js +++ b/packages/svelte/tests/runtime-runes/samples/async-effect-after-await/_config.js @@ -3,7 +3,8 @@ import { test } from '../../test'; export default test({ async test({ assert, logs }) { + assert.deepEqual(logs, []); await tick(); - assert.deepEqual(logs, ['hello']); + assert.deepEqual(logs, ['before', 'after']); } }); diff --git a/packages/svelte/tests/runtime-runes/samples/async-effect-after-boundary/Child.svelte b/packages/svelte/tests/runtime-runes/samples/async-effect-after-boundary/Child.svelte new file mode 100644 index 000000000000..65a225431b18 --- /dev/null +++ b/packages/svelte/tests/runtime-runes/samples/async-effect-after-boundary/Child.svelte @@ -0,0 +1,5 @@ + diff --git a/packages/svelte/tests/runtime-runes/samples/async-effect-after-boundary/_config.js b/packages/svelte/tests/runtime-runes/samples/async-effect-after-boundary/_config.js new file mode 100644 index 000000000000..f7b6c513d4cc --- /dev/null +++ b/packages/svelte/tests/runtime-runes/samples/async-effect-after-boundary/_config.js @@ -0,0 +1,16 @@ +import { tick } from 'svelte'; +import { test } from '../../test'; + +export default test({ + async test({ assert, target, logs }) { + const [shift] = target.querySelectorAll('button'); + + await tick(); + assert.deepEqual(logs, []); + + shift.click(); + await tick(); + + assert.deepEqual(logs, ['in effect']); + } +}); diff --git a/packages/svelte/tests/runtime-runes/samples/async-effect-after-boundary/main.svelte b/packages/svelte/tests/runtime-runes/samples/async-effect-after-boundary/main.svelte new file mode 100644 index 000000000000..edfd3c4d10ac --- /dev/null +++ b/packages/svelte/tests/runtime-runes/samples/async-effect-after-boundary/main.svelte @@ -0,0 +1,22 @@ + + + + + +

{await push('hello')}

+ + + {#snippet pending()} +

loading...

+ {/snippet} +
diff --git a/packages/svelte/tests/runtime-runes/samples/async-resolve-stale/_config.js b/packages/svelte/tests/runtime-runes/samples/async-resolve-stale/_config.js index 50bb414afc8b..7fb49c473ec3 100644 --- a/packages/svelte/tests/runtime-runes/samples/async-resolve-stale/_config.js +++ b/packages/svelte/tests/runtime-runes/samples/async-resolve-stale/_config.js @@ -3,23 +3,28 @@ import { test } from '../../test'; export default test({ async test({ assert, target }) { // We gotta wait a bit more in this test because of the macrotasks in App.svelte - function macrotask(t = 3) { + function sleep(t = 50) { return new Promise((r) => setTimeout(r, t)); } - await macrotask(); + await sleep(); assert.htmlEqual(target.innerHTML, ' 1 | '); const [input] = target.querySelectorAll('input'); input.value = '1'; input.dispatchEvent(new Event('input', { bubbles: true })); - await macrotask(); + await sleep(); assert.htmlEqual(target.innerHTML, ' 1 | '); input.value = '12'; input.dispatchEvent(new Event('input', { bubbles: true })); - await macrotask(6); + await sleep(); assert.htmlEqual(target.innerHTML, ' 3 | 12'); + + input.value = ''; + input.dispatchEvent(new Event('input', { bubbles: true })); + await sleep(); + assert.htmlEqual(target.innerHTML, ' 4 | '); } }); diff --git a/packages/svelte/tests/runtime-runes/samples/async-resolve-stale/main.svelte b/packages/svelte/tests/runtime-runes/samples/async-resolve-stale/main.svelte index dc4a157928a3..dec5a558999d 100644 --- a/packages/svelte/tests/runtime-runes/samples/async-resolve-stale/main.svelte +++ b/packages/svelte/tests/runtime-runes/samples/async-resolve-stale/main.svelte @@ -1,26 +1,31 @@