diff --git a/.changeset/old-taxis-relate.md b/.changeset/old-taxis-relate.md new file mode 100644 index 000000000000..ea1b696ca850 --- /dev/null +++ b/.changeset/old-taxis-relate.md @@ -0,0 +1,5 @@ +--- +'svelte': patch +--- + +fix: run effects created in the template after async work has resolved diff --git a/packages/svelte/src/internal/client/dom/task.js b/packages/svelte/src/internal/client/dom/task.js index 48a2fbe660eb..b639555cd0ca 100644 --- a/packages/svelte/src/internal/client/dom/task.js +++ b/packages/svelte/src/internal/client/dom/task.js @@ -1,4 +1,5 @@ import { run_all } from '../../shared/utils.js'; +import { is_flushing_sync } from '../reactivity/batch.js'; // Fallback for when requestIdleCallback is not available const request_idle_callback = @@ -24,11 +25,15 @@ function run_idle_tasks() { run_all(tasks); } +export function has_pending_tasks() { + return micro_tasks.length > 0 || idle_tasks.length > 0; +} + /** * @param {() => void} fn */ export function queue_micro_task(fn) { - if (micro_tasks.length === 0) { + if (micro_tasks.length === 0 && !is_flushing_sync) { queueMicrotask(run_micro_tasks); } diff --git a/packages/svelte/src/internal/client/reactivity/batch.js b/packages/svelte/src/internal/client/reactivity/batch.js index 5176a4f74b0d..d06080dc5091 100644 --- a/packages/svelte/src/internal/client/reactivity/batch.js +++ b/packages/svelte/src/internal/client/reactivity/batch.js @@ -25,7 +25,7 @@ import { update_effect } from '../runtime.js'; import * as e from '../errors.js'; -import { flush_tasks } from '../dom/task.js'; +import { flush_tasks, has_pending_tasks, queue_micro_task } from '../dom/task.js'; import { DEV } from 'esm-env'; import { invoke_error_boundary } from '../error-handling.js'; import { old_values } from './sources.js'; @@ -56,19 +56,6 @@ export let batch_deriveds = null; /** @type {Set<() => void>} */ export let effect_pending_updates = new Set(); -/** @type {Array<() => void>} */ -let tasks = []; - -function dequeue() { - const task = /** @type {() => void} */ (tasks.shift()); - - if (tasks.length > 0) { - queueMicrotask(dequeue); - } - - task(); -} - /** @type {Effect[]} */ let queued_root_effects = []; @@ -76,7 +63,7 @@ let queued_root_effects = []; let last_scheduled_effect = null; let is_flushing = false; -let is_flushing_sync = false; +export let is_flushing_sync = false; export class Batch { /** @@ -360,6 +347,48 @@ export class Batch { current_batch = this; } + /** + * Check if the branch this effect is in is obsolete in a later batch. + * That is, if the branch exists in this batch but will be destroyed in a later batch. + * @param {Effect} effect + */ + branch_obsolete(effect) { + /** @type {Effect[]} */ + let alive = []; + /** @type {Effect[]} */ + let skipped = []; + /** @type {Effect | null} */ + let current = effect; + + while (current !== null) { + if ((current.f & (BRANCH_EFFECT | ROOT_EFFECT)) !== 0) { + alive.push(current); + if (this.skipped_effects.has(current)) { + skipped.push(...alive); + alive = []; + } + } + current = current.parent; + } + + let check = false; + for (const b of batches) { + if (b === this) { + check = true; + } else if (check) { + if ( + alive.some((branch) => b.skipped_effects.has(branch)) || + // TODO do we even have to check skipped here? how would an async_derived run for a branch that was already skipped? + (skipped.length > 0 && !skipped.some((branch) => b.skipped_effects.has(branch))) + ) { + return true; + } + } + } + + return false; + } + deactivate() { current_batch = null; previous_batch = null; @@ -382,7 +411,7 @@ export class Batch { flush() { if (queued_root_effects.length > 0) { flush_effects(); - } else { + } else if (this.#pending === 0) { this.#commit(); } @@ -420,20 +449,24 @@ export class Batch { this.#pending -= 1; if (this.#pending === 0) { - for (const e of this.#dirty_effects) { - set_signal_status(e, DIRTY); - schedule_effect(e); - } + Batch.enqueue(() => { + this.activate(); - for (const e of this.#maybe_dirty_effects) { - set_signal_status(e, MAYBE_DIRTY); - schedule_effect(e); - } + for (const e of this.#dirty_effects) { + set_signal_status(e, DIRTY); + schedule_effect(e); + } - this.#render_effects = []; - this.#effects = []; + for (const e of this.#maybe_dirty_effects) { + set_signal_status(e, MAYBE_DIRTY); + schedule_effect(e); + } + + this.#render_effects = []; + this.#effects = []; - this.flush(); + this.flush(); + }); } else { this.deactivate(); } @@ -470,11 +503,7 @@ export class Batch { /** @param {() => void} task */ static enqueue(task) { - if (tasks.length === 0) { - queueMicrotask(dequeue); - } - - tasks.unshift(task); + queue_micro_task(task); } } @@ -505,7 +534,7 @@ export function flushSync(fn) { while (true) { flush_tasks(); - if (queued_root_effects.length === 0) { + if (queued_root_effects.length === 0 && !has_pending_tasks()) { current_batch?.flush(); // we need to check again, in case we just updated an `$effect.pending()` @@ -673,19 +702,24 @@ export function schedule_effect(signal) { export function suspend() { var boundary = get_boundary(); var batch = /** @type {Batch} */ (current_batch); - var pending = boundary.is_pending(); + // In case the pending snippet is shown, we want to update the UI immediately + // and not have the batch be blocked on async work, + // since the async work is happening "hidden" behind the pending snippet. + var ignore_async = boundary.is_pending(); boundary.update_pending_count(1); - if (!pending) batch.increment(); + if (!ignore_async) batch.increment(); return function unsuspend() { boundary.update_pending_count(-1); - - if (!pending) { - batch.activate(); - batch.decrement(); + batch.activate(); + if (ignore_async) { + // Template could have created new effects (for example via attachments) which need to be flushed + Batch.enqueue(() => { + batch.flush(); + }); } else { - batch.deactivate(); + batch.decrement(); } unset_context(); diff --git a/packages/svelte/src/internal/client/reactivity/deriveds.js b/packages/svelte/src/internal/client/reactivity/deriveds.js index 299251a2dcdd..0f697a8a04ca 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, batch_deriveds, current_batch } from './batch.js'; import { unset_context } from './async.js'; /** @type {Effect | null} */ @@ -115,7 +114,7 @@ export function async_derived(fn, location) { // only suspend in async deriveds created on initialisation var should_suspend = !active_reaction; - async_effect(() => { + var effect = async_effect(() => { if (DEV) current_async_effect = active_effect; try { @@ -135,11 +134,14 @@ export function async_derived(fn, location) { prev = promise; var batch = /** @type {Batch} */ (current_batch); - var pending = boundary.is_pending(); + // In case the pending snippet is shown, we want to update the UI immediately + // and not have the batch be blocked on async work, + // since the async work is happening "hidden" behind the pending snippet. + var ignore_async = boundary.is_pending(); if (should_suspend) { boundary.update_pending_count(1); - if (!pending) batch.increment(); + if (!ignore_async) batch.increment(); } /** @@ -151,10 +153,10 @@ export function async_derived(fn, location) { current_async_effect = null; - if (!pending) batch.activate(); + batch.activate(); if (error) { - if (error !== STALE_REACTION) { + if (error !== STALE_REACTION && !batch.branch_obsolete(effect)) { signal.f |= ERROR_VALUE; // @ts-expect-error the error is the wrong type, but we don't care @@ -181,7 +183,14 @@ export function async_derived(fn, location) { if (should_suspend) { boundary.update_pending_count(-1); - if (!pending) batch.decrement(); + if (ignore_async) { + // Template could have created new effects (for example via attachments) which need to be flushed + Batch.enqueue(() => { + batch.flush(); + }); + } else { + batch.decrement(); + } } unset_context(); diff --git a/packages/svelte/tests/runtime-runes/samples/async-attachment/Inner.svelte b/packages/svelte/tests/runtime-runes/samples/async-attachment/Inner.svelte new file mode 100644 index 000000000000..b9b9d7a3d08b --- /dev/null +++ b/packages/svelte/tests/runtime-runes/samples/async-attachment/Inner.svelte @@ -0,0 +1,10 @@ + + +

{test}

+
diff --git a/packages/svelte/tests/runtime-runes/samples/async-attachment/_config.js b/packages/svelte/tests/runtime-runes/samples/async-attachment/_config.js new file mode 100644 index 000000000000..f6b48b38b14a --- /dev/null +++ b/packages/svelte/tests/runtime-runes/samples/async-attachment/_config.js @@ -0,0 +1,18 @@ +import { tick } from 'svelte'; +import { test } from '../../test'; + +export default test({ + async test({ assert, target }) { + await tick(); + assert.htmlEqual(target.innerHTML, '

foo

foo
'); + + const [toggle] = target.querySelectorAll('button'); + toggle.click(); + await tick(); + assert.htmlEqual(target.innerHTML, ''); + + toggle.click(); + await tick(); + assert.htmlEqual(target.innerHTML, '

foo

foo
'); + } +}); diff --git a/packages/svelte/tests/runtime-runes/samples/async-attachment/main.svelte b/packages/svelte/tests/runtime-runes/samples/async-attachment/main.svelte new file mode 100644 index 000000000000..6cef6e8f5c8a --- /dev/null +++ b/packages/svelte/tests/runtime-runes/samples/async-attachment/main.svelte @@ -0,0 +1,16 @@ + + + + + {#if show} + + {/if} + + {#snippet pending()} +

pending

+ {/snippet} +
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..18f59ce3c8b3 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 @@ -8,9 +8,11 @@ export default test({ increment.click(); await tick(); + reject.click(); reject.click(); await tick(); + resolve.click(); resolve.click(); await tick(); @@ -22,6 +24,35 @@ export default test({

false

1

+

false

+

1

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

false

+

3

+

false

+

3

` ); } diff --git a/packages/svelte/tests/runtime-runes/samples/async-block-reject-each-during-init/main.svelte b/packages/svelte/tests/runtime-runes/samples/async-block-reject-each-during-init/main.svelte index 1ad6cb84decc..f17f492292d9 100644 --- a/packages/svelte/tests/runtime-runes/samples/async-block-reject-each-during-init/main.svelte +++ b/packages/svelte/tests/runtime-runes/samples/async-block-reject-each-during-init/main.svelte @@ -3,7 +3,7 @@ let deferreds = []; - function push() { + function push(_just_so_that_template_is_reactive_) { const deferred = Promise.withResolvers(); deferreds.push(deferred); return deferred.promise; @@ -17,10 +17,19 @@ {#if count % 2 === 0}

true

- {#each await push() as count}

{count}

{/each} + {#each await push(count) as count}

{count}

{/each} {:else}

false

- {#each await push() as count}

{count}

{/each} + {#each await push(count) as count}

{count}

{/each} + {/if} + + {#if count % 2 === 0} +

true

+ {#each await push(count) as count}

{count}

{/each} + {/if} + {#if count % 2 === 1} +

false

+ {#each await push(count) as count}

{count}

{/each} {/if} {#snippet pending()} diff --git a/packages/svelte/tests/runtime-runes/samples/async-pending-flushed-immediately/_config.js b/packages/svelte/tests/runtime-runes/samples/async-pending-flushed-immediately/_config.js new file mode 100644 index 000000000000..7c16efc8b748 --- /dev/null +++ b/packages/svelte/tests/runtime-runes/samples/async-pending-flushed-immediately/_config.js @@ -0,0 +1,75 @@ +import { tick } from 'svelte'; +import { test } from '../../test'; + +export default test({ + async test({ assert, target }) { + const [increment, resolve] = target.querySelectorAll('button'); + + assert.htmlEqual( + target.innerHTML, + ` + + + ` + ); + + increment.click(); + await tick(); + + assert.htmlEqual( + target.innerHTML, + ` + + + ` + ); + + increment.click(); + await tick(); + + assert.htmlEqual( + target.innerHTML, + ` + + +

loading...

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

2

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

2

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

3

+ ` + ); + } +}); diff --git a/packages/svelte/tests/runtime-runes/samples/async-pending-flushed-immediately/main.svelte b/packages/svelte/tests/runtime-runes/samples/async-pending-flushed-immediately/main.svelte new file mode 100644 index 000000000000..9c3700c78eaa --- /dev/null +++ b/packages/svelte/tests/runtime-runes/samples/async-pending-flushed-immediately/main.svelte @@ -0,0 +1,24 @@ + + + + + +{#if count > 1} + +

{await push(count)}

+ + {#snippet pending()} +

loading...

+ {/snippet} +
+{/if} diff --git a/packages/svelte/tests/runtime-runes/samples/async-top-level-error-nested-obsolete/main.svelte b/packages/svelte/tests/runtime-runes/samples/async-top-level-error-nested-obsolete/main.svelte index 2f461e96c856..2d8032228afb 100644 --- a/packages/svelte/tests/runtime-runes/samples/async-top-level-error-nested-obsolete/main.svelte +++ b/packages/svelte/tests/runtime-runes/samples/async-top-level-error-nested-obsolete/main.svelte @@ -3,6 +3,11 @@ export let route = $state({ current: 'home' }); + + diff --git a/packages/svelte/tests/runtime-runes/samples/await-pending-wait/_config.js b/packages/svelte/tests/runtime-runes/samples/await-pending-wait/_config.js index 7ad653433cab..f1ca4eadbbca 100644 --- a/packages/svelte/tests/runtime-runes/samples/await-pending-wait/_config.js +++ b/packages/svelte/tests/runtime-runes/samples/await-pending-wait/_config.js @@ -5,6 +5,8 @@ export default test({ async test({ assert, target }) { const [b1, b2, b3] = target.querySelectorAll('button'); + await Promise.resolve(); + // not flushing means we wait a tick before showing the pending state ... b2.click(); await Promise.resolve(); @@ -45,6 +47,7 @@ export default test({ ); // when not flushing ... + await Promise.resolve(); b3.click(); await Promise.resolve(); assert.htmlEqual( diff --git a/packages/svelte/tests/runtime-runes/samples/await-resolve-2/_config.js b/packages/svelte/tests/runtime-runes/samples/await-resolve-2/_config.js index 88f1b1e96124..d8da208599bc 100644 --- a/packages/svelte/tests/runtime-runes/samples/await-resolve-2/_config.js +++ b/packages/svelte/tests/runtime-runes/samples/await-resolve-2/_config.js @@ -3,6 +3,7 @@ import { test } from '../../test'; export default test({ async test({ assert, target, logs }) { const [b1, b2, b3, b4] = target.querySelectorAll('button'); + await Promise.resolve(); b1.click(); await Promise.resolve(); b2.click(); diff --git a/packages/svelte/tests/runtime-runes/samples/await-resolve/_config.js b/packages/svelte/tests/runtime-runes/samples/await-resolve/_config.js index 11a645673a18..e8574c3295cc 100644 --- a/packages/svelte/tests/runtime-runes/samples/await-resolve/_config.js +++ b/packages/svelte/tests/runtime-runes/samples/await-resolve/_config.js @@ -3,6 +3,7 @@ import { test } from '../../test'; export default test({ async test({ assert, target, logs }) { const [b1, b2] = target.querySelectorAll('button'); + await Promise.resolve(); b1.click(); await Promise.resolve(); assert.htmlEqual( diff --git a/packages/svelte/tests/runtime-runes/samples/dynamic-component-transition/_config.js b/packages/svelte/tests/runtime-runes/samples/dynamic-component-transition/_config.js index 9c741d2b8c85..19f0c38227aa 100644 --- a/packages/svelte/tests/runtime-runes/samples/dynamic-component-transition/_config.js +++ b/packages/svelte/tests/runtime-runes/samples/dynamic-component-transition/_config.js @@ -5,7 +5,8 @@ export default test({ async test({ assert, target, raf }) { const btn = target.querySelector('button'); - raf.tick(0); + // one tick to not be at 0. Else the flushSync would revert the in-transition which hasn't started, and directly remove the button + raf.tick(1); flushSync(() => { btn?.click(); @@ -13,7 +14,7 @@ export default test({ assert.htmlEqual(target.innerHTML, `

Outside

`); - raf.tick(100); + raf.tick(101); assert.htmlEqual(target.innerHTML, `

Outside

`); } diff --git a/packages/svelte/tests/runtime-runes/samples/dynamic-if-component-transition/_config.js b/packages/svelte/tests/runtime-runes/samples/dynamic-if-component-transition/_config.js index 9c741d2b8c85..19f0c38227aa 100644 --- a/packages/svelte/tests/runtime-runes/samples/dynamic-if-component-transition/_config.js +++ b/packages/svelte/tests/runtime-runes/samples/dynamic-if-component-transition/_config.js @@ -5,7 +5,8 @@ export default test({ async test({ assert, target, raf }) { const btn = target.querySelector('button'); - raf.tick(0); + // one tick to not be at 0. Else the flushSync would revert the in-transition which hasn't started, and directly remove the button + raf.tick(1); flushSync(() => { btn?.click(); @@ -13,7 +14,7 @@ export default test({ assert.htmlEqual(target.innerHTML, `

Outside

`); - raf.tick(100); + raf.tick(101); assert.htmlEqual(target.innerHTML, `

Outside

`); }