fix: preserve dirty status of deferred effects (#16487)

* don't mark_reactions inside decrement, it can cause infinite loops

* revert mark_reactions changes

* preserve DIRTY/MAYBE_DIRTY status of deferred effects

* changeset

* tweak

Author: Rich-Harris

.changeset/cool-insects-argue.md

@@ -0,0 +1,5 @@
+---
+'svelte': patch
+---
+
+fix: preserve dirty status of deferred effects

packages/svelte/src/internal/client/reactivity/batch.js

@@ -10,7 +10,8 @@ import {
 	INERT,
 	RENDER_EFFECT,
 	ROOT_EFFECT,
-	USER_EFFECT
+	USER_EFFECT,
+	MAYBE_DIRTY
 } from '#client/constants';
 import { async_mode_flag } from '../../flags/index.js';
 import { deferred, define_property } from '../../shared/utils.js';
@@ -27,7 +28,7 @@ import * as e from '../errors.js';
 import { flush_tasks } from '../dom/task.js';
 import { DEV } from 'esm-env';
 import { invoke_error_boundary } from '../error-handling.js';
-import { mark_reactions, old_values } from './sources.js';
+import { old_values } from './sources.js';
 import { unlink_effect } from './effects.js';
 import { unset_context } from './async.js';
 
@@ -146,6 +147,18 @@ export class Batch {
 	 */
 	#block_effects = [];
 
+	/**
+	 * Deferred effects (which run after async work has completed) that are DIRTY
+	 * @type {Effect[]}
+	 */
+	#dirty_effects = [];
+
+	/**
+	 * Deferred effects that are MAYBE_DIRTY
+	 * @type {Effect[]}
+	 */
+	#maybe_dirty_effects = [];
+
 	/**
 	 * A set of branches that still exist, but will be destroyed when this batch
 	 * is committed — we skip over these during `process`
@@ -221,10 +234,9 @@ export class Batch {
 
 			this.#deferred?.resolve();
 		} else {
-			// otherwise mark effects clean so they get scheduled on the next run
-			for (const e of this.#render_effects) set_signal_status(e, CLEAN);
-			for (const e of this.#effects) set_signal_status(e, CLEAN);
-			for (const e of this.#block_effects) set_signal_status(e, CLEAN);
+			this.#defer_effects(this.#render_effects);
+			this.#defer_effects(this.#effects);
+			this.#defer_effects(this.#block_effects);
 		}
 
 		if (current_values) {
@@ -271,15 +283,15 @@ export class Batch {
 			if (!skip && effect.fn !== null) {
 				if (is_branch) {
 					effect.f ^= CLEAN;
-				} else if ((flags & EFFECT) !== 0) {
-					this.#effects.push(effect);
-				} else if (async_mode_flag && (flags & RENDER_EFFECT) !== 0) {
-					this.#render_effects.push(effect);
-				} else if (is_dirty(effect)) {
-					if ((flags & ASYNC) !== 0) {
+				} else if ((flags & CLEAN) === 0) {
+					if ((flags & EFFECT) !== 0) {
+						this.#effects.push(effect);
+					} else if (async_mode_flag && (flags & RENDER_EFFECT) !== 0) {
+						this.#render_effects.push(effect);
+					} else if ((flags & ASYNC) !== 0) {
 						var effects = effect.b?.pending ? this.#boundary_async_effects : this.#async_effects;
 						effects.push(effect);
-					} else {
+					} else if (is_dirty(effect)) {
 						if ((effect.f & BLOCK_EFFECT) !== 0) this.#block_effects.push(effect);
 						update_effect(effect);
 					}
@@ -303,6 +315,21 @@ export class Batch {
 		}
 	}
 
+	/**
+	 * @param {Effect[]} effects
+	 */
+	#defer_effects(effects) {
+		for (const e of effects) {
+			const target = (e.f & DIRTY) !== 0 ? this.#dirty_effects : this.#maybe_dirty_effects;
+			target.push(e);
+
+			// mark as clean so they get scheduled if they depend on pending async state
+			set_signal_status(e, CLEAN);
+		}
+
+		effects.length = 0;
+	}
+
 	/**
 	 * Associate a change to a given source with the current
 	 * batch, noting its previous and current values
@@ -380,8 +407,14 @@ export class Batch {
 		this.#pending -= 1;
 
 		if (this.#pending === 0) {
-			for (const source of this.current.keys()) {
-				mark_reactions(source, DIRTY, false);
+			for (const e of this.#dirty_effects) {
+				set_signal_status(e, DIRTY);
+				schedule_effect(e);
+			}
+
+			for (const e of this.#maybe_dirty_effects) {
+				set_signal_status(e, MAYBE_DIRTY);
+				schedule_effect(e);
 			}
 
 			this.#render_effects = [];

packages/svelte/src/internal/client/reactivity/sources.js

@@ -301,10 +301,9 @@ export function increment(source) {
 /**
  * @param {Value} signal
  * @param {number} status should be DIRTY or MAYBE_DIRTY
- * @param {boolean} schedule_async
  * @returns {void}
  */
-export function mark_reactions(signal, status, schedule_async = true) {
+function mark_reactions(signal, status) {
 	var reactions = signal.reactions;
 	if (reactions === null) return;
 
@@ -324,16 +323,14 @@ export function mark_reactions(signal, status, schedule_async = true) {
 			continue;
 		}
 
-		var should_schedule = (flags & DIRTY) === 0 && (schedule_async || (flags & ASYNC) === 0);
-
 		// don't set a DIRTY reaction to MAYBE_DIRTY
-		if (should_schedule) {
+		if ((flags & DIRTY) === 0) {
 			set_signal_status(reaction, status);
 		}
 
 		if ((flags & DERIVED) !== 0) {
 			mark_reactions(/** @type {Derived} */ (reaction), MAYBE_DIRTY);
-		} else if (should_schedule) {
+		} else {
 			schedule_effect(/** @type {Effect} */ (reaction));
 		}
 	}

packages/svelte/tests/runtime-runes/samples/async-effect-conservative/_config.js

@@ -0,0 +1,28 @@
+import { tick } from 'svelte';
+import { test } from '../../test';
+
+export default test({
+	async test({ assert, target, logs }) {
+		await tick();
+
+		const [increment] = target.querySelectorAll('button');
+
+		assert.deepEqual(logs, [false]);
+		assert.htmlEqual(target.innerHTML, '<button>increment</button><p>0</p>');
+
+		increment.click();
+		await tick();
+		assert.deepEqual(logs, [false]);
+		assert.htmlEqual(target.innerHTML, '<button>increment</button><p>1</p>');
+
+		increment.click();
+		await tick();
+		assert.deepEqual(logs, [false, true]);
+		assert.htmlEqual(target.innerHTML, '<button>increment</button><p>2</p>');
+
+		increment.click();
+		await tick();
+		assert.deepEqual(logs, [false, true]);
+		assert.htmlEqual(target.innerHTML, '<button>increment</button><p>3</p>');
+	}
+});

packages/svelte/tests/runtime-runes/samples/async-effect-conservative/main.svelte

@@ -0,0 +1,17 @@
+<script lang="ts">
+	let count = $state(0);
+	let two_or_larger = $derived(count >= 2);
+
+	$effect(() => {
+		console.log(two_or_larger);
+	});
+</script>
+
+<svelte:boundary>
+	<button onclick={() => count += 1}>increment</button>
+	<p>{await count}</p>
+
+	{#snippet pending()}
+		<p>loading...</p>
+	{/snippet}
+</svelte:boundary>