diff --git a/.changeset/old-taxis-relate.md b/.changeset/old-taxis-relate.md new file mode 100644 index 0000000000..f38dc03f5e --- /dev/null +++ b/.changeset/old-taxis-relate.md @@ -0,0 +1,5 @@ +--- +'svelte': patch +--- + +fix: clean up scheduling system diff --git a/packages/svelte/src/internal/client/dom/task.js b/packages/svelte/src/internal/client/dom/task.js index 48a2fbe660..938f3ccda2 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,12 +25,27 @@ 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) { - queueMicrotask(run_micro_tasks); + if (micro_tasks.length === 0 && !is_flushing_sync) { + var tasks = micro_tasks; + queueMicrotask(() => { + // If this is false, a flushSync happened in the meantime. Do _not_ run new scheduled microtasks in that case + // as the ordering of microtasks would be broken at that point - consider this case: + // - queue_micro_task schedules microtask A to flush task X + // - synchronously after, flushSync runs, processing task X + // - synchronously after, some other microtask B is scheduled, but not through queue_micro_task but for example a Promise.resolve() in user code + // - synchronously after, queue_micro_task schedules microtask C to flush task Y + // - one tick later, microtask A now resolves, flushing task Y before microtask B, which is incorrect + // This if check prevents that race condition (that realistically will only happen in tests) + if (tasks === micro_tasks) run_micro_tasks(); + }); } micro_tasks.push(fn); diff --git a/packages/svelte/src/internal/client/reactivity/batch.js b/packages/svelte/src/internal/client/reactivity/batch.js index 5176a4f74b..c28617608e 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 { /** @@ -470,11 +457,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 +488,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()` 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 2f461e96c8..2d8032228a 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/dynamic-component-transition/_config.js b/packages/svelte/tests/runtime-runes/samples/dynamic-component-transition/_config.js index 9c741d2b8c..19f0c38227 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 9c741d2b8c..19f0c38227 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

`); }