diff --git a/.changeset/big-webs-sing.md b/.changeset/big-webs-sing.md new file mode 100644 index 0000000000..946a41d881 --- /dev/null +++ b/.changeset/big-webs-sing.md @@ -0,0 +1,5 @@ +--- +'svelte': patch +--- + +fix: leave stale promises to wait for a later resolution, instead of rejecting diff --git a/packages/svelte/src/internal/client/reactivity/async.js b/packages/svelte/src/internal/client/reactivity/async.js index 5e418d81a1..c1d4cbcd67 100644 --- a/packages/svelte/src/internal/client/reactivity/async.js +++ b/packages/svelte/src/internal/client/reactivity/async.js @@ -70,20 +70,23 @@ export function flatten(blockers, sync, async, fn) { unset_context(); } + var decrement_pending = increment_pending(); + // Fast path: blockers but no async expressions if (async.length === 0) { - /** @type {Promise} */ (blocker_promise).then(() => finish(sync.map(d))); + /** @type {Promise} */ (blocker_promise) + .then(() => finish(sync.map(d))) + .finally(decrement_pending); + return; } - var decrement_pending = increment_pending(); - // Full path: has async expressions function run() { Promise.all(async.map((expression) => async_derived(expression))) .then((result) => finish([...sync.map(d), ...result])) .catch((error) => invoke_error_boundary(error, parent)) - .finally(() => decrement_pending()); + .finally(decrement_pending); } if (blocker_promise) { @@ -325,7 +328,7 @@ export function run(thunks) { // wait one more tick, so that template effects are // guaranteed to run before `$effect(...)` .then(() => Promise.resolve()) - .finally(() => decrement_pending()); + .finally(decrement_pending); return blockers; } @@ -349,8 +352,8 @@ export function increment_pending() { boundary.update_pending_count(1, batch); batch.increment(blocking, effect); - return (skip = false) => { + return () => { boundary.update_pending_count(-1, batch); - batch.decrement(blocking, effect, skip); + batch.decrement(blocking, effect); }; } diff --git a/packages/svelte/src/internal/client/reactivity/batch.js b/packages/svelte/src/internal/client/reactivity/batch.js index 2d555bb34d..a106806721 100644 --- a/packages/svelte/src/internal/client/reactivity/batch.js +++ b/packages/svelte/src/internal/client/reactivity/batch.js @@ -110,6 +110,13 @@ export class Batch { */ previous = new Map(); + /** + * Async effects which this batch doesn't take into account anymore when calculating blockers, + * as it has a value for it already. + * @type {Set} + */ + unblocked = new Set(); + /** * 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 @@ -200,6 +207,8 @@ export class Batch { #is_blocked() { for (const batch of this.#blockers) { for (const effect of batch.#blocking_pending.keys()) { + if (this.unblocked.has(effect)) continue; + var skipped = false; var e = effect; @@ -647,9 +656,8 @@ export class Batch { /** * @param {boolean} blocking * @param {Effect} effect - * @param {boolean} skip - whether to skip updates (because this is triggered by a stale reaction) */ - decrement(blocking, effect, skip) { + decrement(blocking, effect) { this.#pending -= 1; if (blocking) { @@ -662,12 +670,15 @@ export class Batch { } } - if (this.#decrement_queued || skip) return; + if (this.#decrement_queued) return; this.#decrement_queued = true; queue_micro_task(() => { this.#decrement_queued = false; - this.flush(); + + if (batches.has(this)) { + this.flush(); + } }); } diff --git a/packages/svelte/src/internal/client/reactivity/deriveds.js b/packages/svelte/src/internal/client/reactivity/deriveds.js index 070dfc8ff3..794da4f2b0 100644 --- a/packages/svelte/src/internal/client/reactivity/deriveds.js +++ b/packages/svelte/src/internal/client/reactivity/deriveds.js @@ -100,6 +100,8 @@ export function derived(fn) { return signal; } +const OBSOLETE = {}; + /** * @template V * @param {() => V | Promise} fn @@ -118,7 +120,7 @@ export function async_derived(fn, label, location) { var promise = /** @type {Promise} */ (/** @type {unknown} */ (undefined)); var signal = source(/** @type {V} */ (UNINITIALIZED)); - if (DEV) signal.label = label; + if (DEV) signal.label = label ?? fn.toString(); // only suspend in async deriveds created on initialisation var should_suspend = !active_reaction; @@ -141,7 +143,13 @@ export function async_derived(fn, label, 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).finally(unset_context); + Promise.resolve(fn()) + .then(d.resolve, (e) => { + // if the promise was rejected by the user, via `getAbortSignal`, then + // wait for a subsequent resolution instead of flushing the batch + if (e !== STALE_REACTION) d.reject(e); + }) + .finally(unset_context); } catch (error) { d.reject(error); unset_context(); @@ -180,15 +188,13 @@ export function async_derived(fn, label, location) { } if (/** @type {Boundary} */ (parent.b).is_rendered()) { - deferreds.get(batch)?.reject(STALE_REACTION); - deferreds.delete(batch); // delete to ensure correct order in Map iteration below + deferreds.get(batch)?.reject(OBSOLETE); } else { // While the boundary is still showing pending, a new run supersedes all older in-flight runs // for this async expression. Cancel eagerly so resolution cannot commit stale values. for (const d of deferreds.values()) { - d.reject(STALE_REACTION); + d.reject(OBSOLETE); } - deferreds.clear(); } deferreds.set(batch, d); @@ -203,16 +209,10 @@ export function async_derived(fn, label, location) { reactivity_loss_tracker = null; } - if (decrement_pending) { - // don't trigger an update if we're only here because - // the promise was superseded before it could resolve - var skip = error === STALE_REACTION; - decrement_pending(skip); - } + decrement_pending?.(); + deferreds.delete(batch); - if (error === STALE_REACTION || (effect.f & DESTROYED) !== 0) { - return; - } + if (error === OBSOLETE) return; batch.activate(); @@ -230,9 +230,14 @@ export function async_derived(fn, label, location) { // All prior async derived runs are now stale for (const [b, d] of deferreds) { - deferreds.delete(b); - if (b === batch) break; - d.resolve(value); + if (b.id < batch.id) { + // Don't delete + resolve directly, instead only do that once + // the current batch commits. This way we avoid tearing when + // `b` is rendering through the early resolve while `batch` is + // still pending. + batch.unblocked.add(effect); + batch.oncommit(() => d.resolve(value)); + } } if (DEV && location !== undefined) { @@ -255,7 +260,7 @@ export function async_derived(fn, label, location) { teardown(() => { for (const d of deferreds.values()) { - d.reject(STALE_REACTION); + d.reject(OBSOLETE); } }); diff --git a/packages/svelte/tests/runtime-runes/samples/async-batch-order/_config.js b/packages/svelte/tests/runtime-runes/samples/async-batch-order/_config.js new file mode 100644 index 0000000000..53cceb9d54 --- /dev/null +++ b/packages/svelte/tests/runtime-runes/samples/async-batch-order/_config.js @@ -0,0 +1,30 @@ +import { tick } from 'svelte'; +import { test } from '../../test'; + +export default test({ + async test({ assert, target }) { + await tick(); + const [increment, shift, middle] = target.querySelectorAll('button'); + const [div] = target.querySelectorAll('div'); + + increment.click(); + await tick(); + increment.click(); + await tick(); + increment.click(); + await tick(); + middle.click(); // resolve the second increment which will make the if block go away and the first batch discarded + await tick(); + assert.htmlEqual(div.innerHTML, '2 2'); + + shift.click(); + await tick(); + shift.click(); + await tick(); + shift.click(); + await tick(); + shift.click(); + await tick(); + assert.htmlEqual(div.innerHTML, '3 3'); + } +}); diff --git a/packages/svelte/tests/runtime-runes/samples/async-batch-order/main.svelte b/packages/svelte/tests/runtime-runes/samples/async-batch-order/main.svelte new file mode 100644 index 0000000000..0289380d78 --- /dev/null +++ b/packages/svelte/tests/runtime-runes/samples/async-batch-order/main.svelte @@ -0,0 +1,21 @@ + + +
+ {a} {await delay(a)} + {#if a < 2} + {await delay(a)} + {/if} +
+ + + + diff --git a/packages/svelte/tests/runtime-runes/samples/async-stale-derived-4/_config.js b/packages/svelte/tests/runtime-runes/samples/async-stale-derived-4/_config.js new file mode 100644 index 0000000000..e1555c0062 --- /dev/null +++ b/packages/svelte/tests/runtime-runes/samples/async-stale-derived-4/_config.js @@ -0,0 +1,28 @@ +import { tick } from 'svelte'; +import { test } from '../../test'; + +export default test({ + async test({ assert, target }) { + await tick(); + + const [increment, hide, pop] = target.querySelectorAll('button'); + + increment.click(); + await tick(); + pop.click(); + await tick(); + hide.click(); // hides the if block, which cancels the pending async inside, which means the batch can complete + await tick(); + assert.htmlEqual( + target.innerHTML, + ` 1` + ); + + pop.click(); + await tick(); + assert.htmlEqual( + target.innerHTML, + ` 1` + ); + } +}); diff --git a/packages/svelte/tests/runtime-runes/samples/async-stale-derived-4/main.svelte b/packages/svelte/tests/runtime-runes/samples/async-stale-derived-4/main.svelte new file mode 100644 index 0000000000..5ff3263d39 --- /dev/null +++ b/packages/svelte/tests/runtime-runes/samples/async-stale-derived-4/main.svelte @@ -0,0 +1,21 @@ + + + + + + + +{await push(count)} +{#if show} + {await push(count)} +{/if} diff --git a/packages/svelte/tests/runtime-runes/samples/async-stale-derived-5/_config.js b/packages/svelte/tests/runtime-runes/samples/async-stale-derived-5/_config.js new file mode 100644 index 0000000000..05d92e9df2 --- /dev/null +++ b/packages/svelte/tests/runtime-runes/samples/async-stale-derived-5/_config.js @@ -0,0 +1,33 @@ +import { tick } from 'svelte'; +import { test } from '../../test'; + +export default test({ + async test({ assert, target }) { + await tick(); + + const [increment, shift] = target.querySelectorAll('button'); + + increment.click(); + await tick(); + increment.click(); + await tick(); + assert.htmlEqual( + target.innerHTML, + `

1 = 1

fizz: true

buzz: true

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

1 = 1

fizz: true

buzz: true

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

3 = 3

fizz: true

buzz: false

` + ); + } +}); diff --git a/packages/svelte/tests/runtime-runes/samples/async-stale-derived-5/main.svelte b/packages/svelte/tests/runtime-runes/samples/async-stale-derived-5/main.svelte new file mode 100644 index 0000000000..b2a40e65b3 --- /dev/null +++ b/packages/svelte/tests/runtime-runes/samples/async-stale-derived-5/main.svelte @@ -0,0 +1,44 @@ + + + + + + +

{n} = {await push(n)}

+ +{#if true} +

fizz: {fizz}

+{/if} + +{#if true} +

buzz: {buzz}

+{/if} diff --git a/packages/svelte/tests/runtime-runes/samples/async-stale-derived-6/_config.js b/packages/svelte/tests/runtime-runes/samples/async-stale-derived-6/_config.js new file mode 100644 index 0000000000..5b951d6c49 --- /dev/null +++ b/packages/svelte/tests/runtime-runes/samples/async-stale-derived-6/_config.js @@ -0,0 +1,34 @@ +import { tick } from 'svelte'; +import { test } from '../../test'; + +export default test({ + async test({ assert, target }) { + await tick(); + const [button1, button2, pop, shift] = target.querySelectorAll('button'); + const [p] = target.querySelectorAll('p'); + + button1.click(); + await tick(); + button2.click(); + await tick(); + assert.htmlEqual(p.innerHTML, `0 + 0 = 0 | 0 0`); + + shift.click(); + await tick(); + assert.htmlEqual(p.innerHTML, `0 + 0 = 0 | 0 0`); + + pop.click(); + await tick(); + assert.htmlEqual(p.innerHTML, `0 + 0 = 0 | 0 0`); + + pop.click(); + await tick(); + assert.htmlEqual(p.innerHTML, `1 + 0 = 1 | 1 0`); + + shift.click(); + await tick(); + pop.click(); + await tick(); + assert.htmlEqual(p.innerHTML, `1 + 2 = 3 | 1 1`); + } +}); diff --git a/packages/svelte/tests/runtime-runes/samples/async-stale-derived-6/main.svelte b/packages/svelte/tests/runtime-runes/samples/async-stale-derived-6/main.svelte new file mode 100644 index 0000000000..4515e7a488 --- /dev/null +++ b/packages/svelte/tests/runtime-runes/samples/async-stale-derived-6/main.svelte @@ -0,0 +1,21 @@ + + + + + + + +

{a} + {b} = {await push(a + b)} | {await push(c, 2)} {await push(d, 2)}

diff --git a/packages/svelte/tests/runtime-runes/samples/async-stale-derived-7/_config.js b/packages/svelte/tests/runtime-runes/samples/async-stale-derived-7/_config.js new file mode 100644 index 0000000000..a6f1833d67 --- /dev/null +++ b/packages/svelte/tests/runtime-runes/samples/async-stale-derived-7/_config.js @@ -0,0 +1,34 @@ +import { tick } from 'svelte'; +import { test } from '../../test'; + +export default test({ + async test({ assert, target }) { + await tick(); + const [button1, button2, shift_1, pop_1, shift_2] = target.querySelectorAll('button'); + const [p] = target.querySelectorAll('p'); + + button1.click(); + await tick(); + button2.click(); + await tick(); + assert.htmlEqual(p.innerHTML, `0 + 0 = 0 | 0 0`); + + pop_1.click(); + await tick(); + shift_2.click(); + await tick(); + assert.htmlEqual(p.innerHTML, `0 + 0 = 0 | 0 0`); + + // Check that the first batch can still resolve before the second even if one of its async values + // is already superseeded (but the subsequent batch as a whole is still pending). + shift_1.click(); + await tick(); + assert.htmlEqual(p.innerHTML, `1 + 0 = 1 | 1 0`); + + shift_1.click(); + await tick(); + shift_2.click(); + await tick(); + assert.htmlEqual(p.innerHTML, `1 + 2 = 3 | 1 1`); + } +}); diff --git a/packages/svelte/tests/runtime-runes/samples/async-stale-derived-7/main.svelte b/packages/svelte/tests/runtime-runes/samples/async-stale-derived-7/main.svelte new file mode 100644 index 0000000000..fec684c257 --- /dev/null +++ b/packages/svelte/tests/runtime-runes/samples/async-stale-derived-7/main.svelte @@ -0,0 +1,22 @@ + + + + + + + + +

{a} + {b} = {await push(a + b)} | {await push(c, 2)} {await push(d, 2)}