From ed0a39345b5e453abfdd5b4d921f3c5675dddea7 Mon Sep 17 00:00:00 2001 From: Rich Harris Date: Thu, 25 Sep 2025 14:36:47 -0400 Subject: [PATCH] chore: simplify batch logic (#16838) * always delete a batch once committed * activate inside flush * centralise logic * chore: simplify batch logic a bit more (#16845) * more * remove unnecessary flushSync * simplify --- .../src/internal/client/dom/blocks/await.js | 4 +- .../svelte/src/internal/client/dom/task.js | 6 +-- .../src/internal/client/reactivity/async.js | 1 - .../src/internal/client/reactivity/batch.js | 37 +++++++------------ 4 files changed, 17 insertions(+), 31 deletions(-) diff --git a/packages/svelte/src/internal/client/dom/blocks/await.js b/packages/svelte/src/internal/client/dom/blocks/await.js index 8605812f1e..e7917fbd9e 100644 --- a/packages/svelte/src/internal/client/dom/blocks/await.js +++ b/packages/svelte/src/internal/client/dom/blocks/await.js @@ -22,7 +22,7 @@ import { set_dev_current_component_function, set_dev_stack } from '../../context.js'; -import { flushSync } from '../../reactivity/batch.js'; +import { flushSync, is_flushing_sync } from '../../reactivity/batch.js'; const PENDING = 0; const THEN = 1; @@ -126,7 +126,7 @@ export function await_block(node, get_input, pending_fn, then_fn, catch_fn) { // without this, the DOM does not update until two ticks after the promise // resolves, which is unexpected behaviour (and somewhat irksome to test) - flushSync(); + if (!is_flushing_sync) flushSync(); } } } diff --git a/packages/svelte/src/internal/client/dom/task.js b/packages/svelte/src/internal/client/dom/task.js index 1a47eee6cf..8c51361467 100644 --- a/packages/svelte/src/internal/client/dom/task.js +++ b/packages/svelte/src/internal/client/dom/task.js @@ -10,10 +10,6 @@ function run_micro_tasks() { run_all(tasks); } -export function has_pending_tasks() { - return micro_tasks.length > 0; -} - /** * @param {() => void} fn */ @@ -40,7 +36,7 @@ export function queue_micro_task(fn) { * Synchronously run any queued tasks. */ export function flush_tasks() { - if (micro_tasks.length > 0) { + while (micro_tasks.length > 0) { run_micro_tasks(); } } diff --git a/packages/svelte/src/internal/client/reactivity/async.js b/packages/svelte/src/internal/client/reactivity/async.js index a9126f1a5c..4d572281b7 100644 --- a/packages/svelte/src/internal/client/reactivity/async.js +++ b/packages/svelte/src/internal/client/reactivity/async.js @@ -244,7 +244,6 @@ export async function async_body(fn) { if (pending) { batch.flush(); } else { - batch.activate(); batch.decrement(); } diff --git a/packages/svelte/src/internal/client/reactivity/batch.js b/packages/svelte/src/internal/client/reactivity/batch.js index 35aff7d4c9..8cf0e4bd9d 100644 --- a/packages/svelte/src/internal/client/reactivity/batch.js +++ b/packages/svelte/src/internal/client/reactivity/batch.js @@ -23,7 +23,7 @@ import { update_effect } from '../runtime.js'; import * as e from '../errors.js'; -import { flush_tasks, has_pending_tasks, queue_micro_task } from '../dom/task.js'; +import { flush_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'; @@ -216,15 +216,6 @@ export class Batch { flush_queued_effects(render_effects); flush_queued_effects(effects); - // Reinstate the current batch if there was no new one created, as `process()` runs in a loop in `flush_effects()`. - // That method expects `current_batch` to be set, and could run the loop again if effects result in new effects - // being scheduled but without writes happening in which case no new batch is created. - if (current_batch === null) { - current_batch = this; - } else { - batches.delete(this); - } - this.#deferred?.resolve(); } else { this.#defer_effects(this.#render_effects); @@ -365,19 +356,15 @@ export class Batch { flush() { if (queued_root_effects.length > 0) { + this.activate(); flush_effects(); - } else { - this.#commit(); - } - - if (current_batch !== this) { - // this can happen if a `flushSync` occurred during `flush_effects()`, - // which is permitted in legacy mode despite being a terrible idea - return; - } - if (this.#pending === 0) { - batches.delete(this); + if (current_batch !== null && current_batch !== this) { + // this can happen if a new batch was created during `flush_effects()` + return; + } + } else if (this.#pending === 0) { + this.#commit(); } this.deactivate(); @@ -394,6 +381,7 @@ export class Batch { } this.#callbacks.clear(); + batches.delete(this); } increment() { @@ -478,14 +466,17 @@ export function flushSync(fn) { var result; if (fn) { - flush_effects(); + if (current_batch !== null) { + flush_effects(); + } + result = fn(); } while (true) { flush_tasks(); - if (queued_root_effects.length === 0 && !has_pending_tasks()) { + if (queued_root_effects.length === 0) { current_batch?.flush(); // we need to check again, in case we just updated an `$effect.pending()`