From 28765f846e956c0da0e41e634de8c1066da9e6d5 Mon Sep 17 00:00:00 2001
From: Simon H <5968653+dummdidumm@users.noreply.github.com>
Date: Tue, 14 Oct 2025 17:59:02 +0200
Subject: [PATCH] fix: don't rerun async effects unnecessarily (#16944)
Since #16866, when an async effect runs multiple times, we rebase older batches and rerun those effects. This can have unintended consequences: In a case where an async effect only depends on a single source, and that single source was updated in a later batch, we know that we don't need to / should not rerun the older batch.
This PR makes it so: We collect all the sources of older batches that are not part of the current batch that just committed, and then only mark those async effects as dirty which depend on one of those other sources. Fixes the bug I noticed while working on #16935
---
.changeset/wild-mirrors-take.md | 5 ++
.../src/internal/client/reactivity/batch.js | 67 ++++++++++++++-----
.../internal/client/reactivity/deriveds.js | 7 ++
.../samples/async-resolve-stale/_config.js | 3 +-
4 files changed, 65 insertions(+), 17 deletions(-)
create mode 100644 .changeset/wild-mirrors-take.md
diff --git a/.changeset/wild-mirrors-take.md b/.changeset/wild-mirrors-take.md
new file mode 100644
index 0000000000..faf28e7695
--- /dev/null
+++ b/.changeset/wild-mirrors-take.md
@@ -0,0 +1,5 @@
+---
+'svelte': patch
+---
+
+fix: don't rerun async effects unnecessarily
diff --git a/packages/svelte/src/internal/client/reactivity/batch.js b/packages/svelte/src/internal/client/reactivity/batch.js
index 2edfc1343a..2956e7ed6a 100644
--- a/packages/svelte/src/internal/client/reactivity/batch.js
+++ b/packages/svelte/src/internal/client/reactivity/batch.js
@@ -1,4 +1,4 @@
-/** @import { Derived, Effect, Source, Value } from '#client' */
+/** @import { Derived, Effect, Reaction, Source, Value } from '#client' */
import {
BLOCK_EFFECT,
BRANCH_EFFECT,
@@ -342,31 +342,46 @@ export class Batch {
continue;
}
+ /** @type {Source[]} */
+ const sources = [];
+
for (const [source, value] of this.current) {
if (batch.current.has(source)) {
- if (is_earlier) {
+ if (is_earlier && value !== batch.current.get(source)) {
// bring the value up to date
batch.current.set(source, value);
} else {
- // later batch has more recent value,
+ // same value or later batch has more recent value,
// no need to re-run these effects
continue;
}
}
- mark_effects(source);
+ sources.push(source);
}
- if (queued_root_effects.length > 0) {
- current_batch = batch;
- batch.apply();
+ if (sources.length === 0) {
+ continue;
+ }
- for (const root of queued_root_effects) {
- batch.#traverse_effect_tree(root);
+ // Re-run async/block effects that depend on distinct values changed in both batches
+ const others = [...batch.current.keys()].filter((s) => !this.current.has(s));
+ if (others.length > 0) {
+ for (const source of sources) {
+ mark_effects(source, others);
}
- queued_root_effects = [];
- batch.deactivate();
+ if (queued_root_effects.length > 0) {
+ current_batch = batch;
+ batch.apply();
+
+ for (const root of queued_root_effects) {
+ batch.#traverse_effect_tree(root);
+ }
+
+ queued_root_effects = [];
+ batch.deactivate();
+ }
}
}
@@ -621,17 +636,19 @@ function flush_queued_effects(effects) {
/**
* This is similar to `mark_reactions`, but it only marks async/block effects
- * so that these can re-run after another batch has been committed
+ * depending on `value` and at least one of the other `sources`, so that
+ * these effects can re-run after another batch has been committed
* @param {Value} value
+ * @param {Source[]} sources
*/
-function mark_effects(value) {
+function mark_effects(value, sources) {
if (value.reactions !== null) {
for (const reaction of value.reactions) {
const flags = reaction.f;
if ((flags & DERIVED) !== 0) {
- mark_effects(/** @type {Derived} */ (reaction));
- } else if ((flags & (ASYNC | BLOCK_EFFECT)) !== 0) {
+ mark_effects(/** @type {Derived} */ (reaction), sources);
+ } else if ((flags & (ASYNC | BLOCK_EFFECT)) !== 0 && depends_on(reaction, sources)) {
set_signal_status(reaction, DIRTY);
schedule_effect(/** @type {Effect} */ (reaction));
}
@@ -639,6 +656,26 @@ function mark_effects(value) {
}
}
+/**
+ * @param {Reaction} reaction
+ * @param {Source[]} sources
+ */
+function depends_on(reaction, sources) {
+ if (reaction.deps !== null) {
+ for (const dep of reaction.deps) {
+ if (sources.includes(dep)) {
+ return true;
+ }
+
+ if ((dep.f & DERIVED) !== 0 && depends_on(/** @type {Derived} */ (dep), sources)) {
+ return true;
+ }
+ }
+ }
+
+ return false;
+}
+
/**
* @param {Effect} signal
* @returns {void}
diff --git a/packages/svelte/src/internal/client/reactivity/deriveds.js b/packages/svelte/src/internal/client/reactivity/deriveds.js
index bf8733cfe5..fa780013e1 100644
--- a/packages/svelte/src/internal/client/reactivity/deriveds.js
+++ b/packages/svelte/src/internal/client/reactivity/deriveds.js
@@ -171,6 +171,13 @@ export function async_derived(fn, location) {
internal_set(signal, value);
+ // All prior async derived runs are now stale
+ for (const [b, d] of deferreds) {
+ deferreds.delete(b);
+ if (b === batch) break;
+ d.reject(STALE_REACTION);
+ }
+
if (DEV && location !== undefined) {
recent_async_deriveds.add(signal);
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 bccf12562a..50bb414afc 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
@@ -20,7 +20,6 @@ export default test({
input.value = '12';
input.dispatchEvent(new Event('input', { bubbles: true }));
await macrotask(6);
- // TODO this is wrong (separate bug), this should be 3 | 12
- assert.htmlEqual(target.innerHTML, ' 5 | 12');
+ assert.htmlEqual(target.innerHTML, ' 3 | 12');
}
});