fix: avoid false positives for reactivity loss warning

.changeset/twenty-snakes-design.md

@@ -0,0 +1,5 @@
+---
+'svelte': patch
+---
+
+fix: avoid false positives for reactivity loss warning

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

@@ -164,10 +164,26 @@ export async function save(promise) {
  */
 export async function track_reactivity_loss(promise) {
 	var previous_async_effect = reactivity_loss_tracker;
+	// Ensure that unrelated reads after an async operation is kicked off don't cause false positives
+	queueMicrotask(() => {
+		if (reactivity_loss_tracker === previous_async_effect) {
+			set_reactivity_loss_tracker(null);
+		}
+	});
+
 	var value = await promise;
 
 	return () => {
 		set_reactivity_loss_tracker(previous_async_effect);
+		// While this can result in false negatives it also guards against the more important
+		// false positives that would occur if this is the last in a chain of async operations,
+		// and the reactivity_loss_tracker would then stay around until the next async operation happens.
+		queueMicrotask(() => {
+			if (reactivity_loss_tracker === previous_async_effect) {
+				set_reactivity_loss_tracker(null);
+			}
+		});
+
 		return value;
 	};
 }
@@ -206,7 +222,9 @@ export async function* for_await_track_reactivity_loss(iterable) {
 				normal_completion = true;
 				break;
 			}
+			var prev = reactivity_loss_tracker;
 			yield value;
+			set_reactivity_loss_tracker(prev);
 		}
 	} finally {
 		// If the iterator had an abrupt completion and `return` is defined on the iterator, call it and return the value

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

@@ -1,4 +1,4 @@
-/** @import { Derived, Effect, Source } from '#client' */
+/** @import { Derived, Effect, Reaction, Source, Value } from '#client' */
 /** @import { Batch } from './batch.js'; */
 /** @import { Boundary } from '../dom/blocks/boundary.js'; */
 import { DEV } from 'esm-env';
@@ -23,7 +23,9 @@ import {
 	push_reaction_value,
 	is_destroying_effect,
 	update_effect,
-	remove_reactions
+	remove_reactions,
+	skipped_deps,
+	new_deps
 } from '../runtime.js';
 import { equals, safe_equals } from './equality.js';
 import * as e from '../errors.js';
@@ -48,11 +50,11 @@ import { set_signal_status, update_derived_status } from './status.js';
 /**
  * This allows us to track 'reactivity loss' that occurs when signals
  * are read after a non-context-restoring `await`. Dev-only
- * @type {{ effect: Effect, warned: boolean } | null}
+ * @type {{ effect: Effect, effect_deps: Set<Value>, warned: boolean } | null}
  */
 export let reactivity_loss_tracker = null;
 
-/** @param {{ effect: Effect, warned: boolean } | null} v */
+/** @param {{ effect: Effect, effect_deps: Set<Value>, warned: boolean } | null} v */
 export function set_reactivity_loss_tracker(v) {
 	reactivity_loss_tracker = v;
 }
@@ -128,15 +130,12 @@ export function async_derived(fn, label, location) {
 	var deferreds = new Map();
 
 	async_effect(() => {
+		var effect = /** @type {Effect} */ (active_effect);
+
 		if (DEV) {
-			reactivity_loss_tracker = {
-				effect: /** @type {Effect} */ (active_effect),
-				warned: false
-			};
+			reactivity_loss_tracker = { effect, effect_deps: new Set(), warned: false };
 		}
 
-		var effect = /** @type {Effect} */ (active_effect);
-
 		/** @type {ReturnType<typeof deferred<V>>} */
 		var d = deferred();
 		promise = d.promise;
@@ -152,6 +151,24 @@ export function async_derived(fn, label, location) {
 		}
 
 		if (DEV) {
+			if (reactivity_loss_tracker) {
+				// Reused deps from previous run (indices 0 to skipped_deps-1)
+				// We deliberately only track direct dependencies of the async expression to encourage
+				// dependencies being directly visible at the point of the expression
+				if (effect.deps !== null) {
+					for (let i = 0; i < skipped_deps; i += 1) {
+						reactivity_loss_tracker.effect_deps.add(effect.deps[i]);
+					}
+				}
+
+				// New deps discovered this run
+				if (new_deps !== null) {
+					for (let i = 0; i < new_deps.length; i += 1) {
+						reactivity_loss_tracker.effect_deps.add(new_deps[i]);
+					}
+				}
+			}
+
 			reactivity_loss_tracker = null;
 		}
 

packages/svelte/src/internal/client/runtime.js

@@ -111,9 +111,9 @@ export function push_reaction_value(value) {
  * and until a new dependency is accessed — we track this via `skipped_deps`
  * @type {null | Value[]}
  */
-let new_deps = null;
+export let new_deps = null;
 
-let skipped_deps = 0;
+export let skipped_deps = 0;
 
 /**
  * Tracks writes that the effect it's executed in doesn't listen to yet,
@@ -573,7 +573,8 @@ export function get(signal) {
 			!untracking &&
 			reactivity_loss_tracker &&
 			!reactivity_loss_tracker.warned &&
-			(reactivity_loss_tracker.effect.f & REACTION_IS_UPDATING) === 0
+			(reactivity_loss_tracker.effect.f & REACTION_IS_UPDATING) === 0 &&
+			!reactivity_loss_tracker.effect_deps.has(signal)
 		) {
 			reactivity_loss_tracker.warned = true;
 

packages/svelte/tests/runtime-runes/samples/async-reactivity-loss-async-after-sync/_config.js

@@ -0,0 +1,27 @@
+import { tick } from 'svelte';
+import { test } from '../../test';
+import { normalise_trace_logs } from '../../../helpers.js';
+
+export default test({
+	compileOptions: {
+		dev: true
+	},
+
+	async test({ assert, target, warnings }) {
+		await new Promise((r) => setTimeout(r, 25));
+
+		const [count] = target.querySelectorAll('button');
+
+		count.click();
+		await new Promise((r) => setTimeout(r, 25));
+
+		assert.deepEqual(normalise_trace_logs(warnings), [
+			{
+				log: 'Detected reactivity loss when reading `other`. This happens when state is read in an async function after an earlier `await`'
+			},
+			{
+				log: 'Detected reactivity loss when reading `other`. This happens when state is read in an async function after an earlier `await`'
+			}
+		]);
+	}
+});

packages/svelte/tests/runtime-runes/samples/async-reactivity-loss-async-after-sync/main.svelte

@@ -0,0 +1,27 @@
+<script>
+	let count = $state(0);
+	let other = $state(0);
+
+	function delayed(value, ms = 1000) {
+		return new Promise((f) => setTimeout(() => f(value), ms))
+	}
+
+	async function foo() {
+		await new Promise(r => setTimeout(r, 10));
+	}
+
+	async function bar() {
+		const value = await delayed(count, 10);
+		other; // should trigger warning
+		return value;
+	}
+
+	async function get() {
+		foo();
+		return await bar();
+	}
+</script>
+
+<button onclick={() => count++}>{count}</button>
+<button onclick={() => other++}>{other}</button>
+{await get()}

packages/svelte/tests/runtime-runes/samples/async-reactivity-loss-for-await-break-return/_config.js

@@ -14,7 +14,7 @@ export default test({
 
 		assert.deepEqual(normalise_trace_logs(warnings), [
 			{
-				log: 'Detected reactivity loss when reading `values.length`. This happens when state is read in an async function after an earlier `await`'
+				log: 'Detected reactivity loss when reading `values[1]`. This happens when state is read in an async function after an earlier `await`'
 			}
 		]);
 	}

packages/svelte/tests/runtime-runes/samples/async-reactivity-loss-for-await/_config.js

@@ -15,7 +15,7 @@ export default test({
 
 		assert.deepEqual(normalise_trace_logs(warnings), [
 			{
-				log: 'Detected reactivity loss when reading `values.length`. This happens when state is read in an async function after an earlier `await`'
+				log: 'Detected reactivity loss when reading `values[1]`. This happens when state is read in an async function after an earlier `await`'
 			}
 		]);
 	}

packages/svelte/tests/runtime-runes/samples/async-reactivity-loss-no-false-positive-1/_config.js

@@ -0,0 +1,21 @@
+import { tick } from 'svelte';
+import { test } from '../../test';
+
+export default test({
+	compileOptions: {
+		dev: true
+	},
+
+	async test({ assert, target, warnings }) {
+		await tick();
+
+		const [x, y] = target.querySelectorAll('button');
+
+		y.click();
+		await tick();
+		x.click();
+		await new Promise((r) => setTimeout(r, 15));
+
+		assert.equal(warnings.length, 0);
+	}
+});

packages/svelte/tests/runtime-runes/samples/async-reactivity-loss-no-false-positive-1/main.svelte

@@ -0,0 +1,16 @@
+<script>
+	let x = $state(0);
+	let y = $state(0);
+	async function foo(x) {
+		if (x) {
+			await 1; // restores reactivity loss warning context
+			await new Promise(r => setTimeout(r, 10)) // saves reactivity loss warning context; should not keep it while running
+		}
+		return x
+	}
+</script>
+
+{x} {await foo(y)}
+
+<button onclick={() => x += 1}>x++</button>
+<button onclick={() => y += 1}>y++</button>

packages/svelte/tests/runtime-runes/samples/async-reactivity-loss-no-false-positive-2/_config.js

@@ -0,0 +1,19 @@
+import { tick } from 'svelte';
+import { test } from '../../test';
+
+export default test({
+	compileOptions: {
+		dev: true
+	},
+
+	async test({ assert, target, warnings }) {
+		await tick();
+
+		const [count] = target.querySelectorAll('button');
+
+		count.click();
+		await tick();
+
+		assert.equal(warnings.length, 0);
+	}
+});

packages/svelte/tests/runtime-runes/samples/async-reactivity-loss-no-false-positive-2/main.svelte

@@ -0,0 +1,14 @@
+<script>
+	let count = $state(0);
+	async function delay(c) {
+		if (c) {
+			await new Promise(r => setTimeout(r));
+			count; // count already read synchronously; should not result in reacitive loss warning
+		}
+		return c;
+	}
+</script>
+
+{await delay(count)}
+
+<button onclick={() => count += 1}>count++</button>

packages/svelte/tests/runtime-runes/samples/async-reactivity-loss-no-false-positive-3/_config.js

@@ -0,0 +1,19 @@
+import { tick } from 'svelte';
+import { test } from '../../test';
+
+export default test({
+	compileOptions: {
+		dev: true
+	},
+
+	async test({ assert, target, warnings }) {
+		await new Promise((r) => setTimeout(r, 5));
+
+		const [count] = target.querySelectorAll('button');
+
+		count.click();
+		await tick();
+
+		assert.equal(warnings.length, 0);
+	}
+});

packages/svelte/tests/runtime-runes/samples/async-reactivity-loss-no-false-positive-3/main.svelte

@@ -0,0 +1,14 @@
+<script>
+	let count = $state(0);
+
+	async function run() {
+		await new Promise(r => setTimeout(r));
+	}
+	async function get() { 
+		run();
+		return 1;
+	}
+</script>
+
+<button onclick={() => count++}>{count}</button>
+{await get()}