From 15588f5fbfe736f65e189e56047ee08678f5509f Mon Sep 17 00:00:00 2001 From: Simon H <5968653+dummdidumm@users.noreply.github.com> Date: Fri, 10 Apr 2026 03:05:26 +0200 Subject: [PATCH] fix: avoid false positives for reactivity loss warning (#18088) Avoids two categories of false positives for reactivity loss warning: 1. if you have synchronously read signals already as part of invoking the async_derived function, then it shouldn't warn when these signals are read after an await if we know they haven't changed (we check the write version for that) 2. `track_reactivity_loss` kept the `reactivity_loss_tracker` around indefinitely, both when invoking the async operation as well as when it's finished. The former is buggy because while the async operation happens unrelated reads as part of other reactivity work can happen, the latter is buggy because if it's the last in a chain of awaits it's kept around until the next async work starts. Fixes https://github.com/sveltejs/kit/issues/15654 --------- Co-authored-by: Rich Harris Co-authored-by: vercel[bot] <35613825+vercel[bot]@users.noreply.github.com> --- .changeset/twenty-snakes-design.md | 5 +++ .../src/internal/client/reactivity/async.js | 18 +++++++++ .../internal/client/reactivity/deriveds.js | 37 ++++++++++++++----- .../svelte/src/internal/client/runtime.js | 7 ++-- .../_config.js | 27 ++++++++++++++ .../main.svelte | 27 ++++++++++++++ .../_config.js | 2 +- .../_config.js | 2 +- .../_config.js | 21 +++++++++++ .../main.svelte | 16 ++++++++ .../_config.js | 19 ++++++++++ .../main.svelte | 14 +++++++ .../_config.js | 19 ++++++++++ .../main.svelte | 14 +++++++ 14 files changed, 213 insertions(+), 15 deletions(-) create mode 100644 .changeset/twenty-snakes-design.md create mode 100644 packages/svelte/tests/runtime-runes/samples/async-reactivity-loss-async-after-sync/_config.js create mode 100644 packages/svelte/tests/runtime-runes/samples/async-reactivity-loss-async-after-sync/main.svelte create mode 100644 packages/svelte/tests/runtime-runes/samples/async-reactivity-loss-no-false-positive-1/_config.js create mode 100644 packages/svelte/tests/runtime-runes/samples/async-reactivity-loss-no-false-positive-1/main.svelte create mode 100644 packages/svelte/tests/runtime-runes/samples/async-reactivity-loss-no-false-positive-2/_config.js create mode 100644 packages/svelte/tests/runtime-runes/samples/async-reactivity-loss-no-false-positive-2/main.svelte create mode 100644 packages/svelte/tests/runtime-runes/samples/async-reactivity-loss-no-false-positive-3/_config.js create mode 100644 packages/svelte/tests/runtime-runes/samples/async-reactivity-loss-no-false-positive-3/main.svelte diff --git a/.changeset/twenty-snakes-design.md b/.changeset/twenty-snakes-design.md new file mode 100644 index 0000000000..ec1ac591f0 --- /dev/null +++ b/.changeset/twenty-snakes-design.md @@ -0,0 +1,5 @@ +--- +'svelte': patch +--- + +fix: avoid false positives for reactivity loss warning diff --git a/packages/svelte/src/internal/client/reactivity/async.js b/packages/svelte/src/internal/client/reactivity/async.js index 18e4f71c88..6aea790c36 100644 --- a/packages/svelte/src/internal/client/reactivity/async.js +++ b/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 diff --git a/packages/svelte/src/internal/client/reactivity/deriveds.js b/packages/svelte/src/internal/client/reactivity/deriveds.js index 12dd7bf672..5af51449ad 100644 --- a/packages/svelte/src/internal/client/reactivity/deriveds.js +++ b/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'; @@ -24,7 +24,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'; @@ -49,11 +51,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, warned: boolean } | null} */ export let reactivity_loss_tracker = null; -/** @param {{ effect: Effect, warned: boolean } | null} v */ +/** @param {{ effect: Effect, effect_deps: Set, warned: boolean } | null} v */ export function set_reactivity_loss_tracker(v) { reactivity_loss_tracker = v; } @@ -125,15 +127,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>} */ var d = deferred(); promise = d.promise; @@ -149,6 +148,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; } diff --git a/packages/svelte/src/internal/client/runtime.js b/packages/svelte/src/internal/client/runtime.js index d9578142eb..033afe98dc 100644 --- a/packages/svelte/src/internal/client/runtime.js +++ b/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, @@ -580,7 +580,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; diff --git a/packages/svelte/tests/runtime-runes/samples/async-reactivity-loss-async-after-sync/_config.js b/packages/svelte/tests/runtime-runes/samples/async-reactivity-loss-async-after-sync/_config.js new file mode 100644 index 0000000000..2bec9e93a3 --- /dev/null +++ b/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`' + } + ]); + } +}); diff --git a/packages/svelte/tests/runtime-runes/samples/async-reactivity-loss-async-after-sync/main.svelte b/packages/svelte/tests/runtime-runes/samples/async-reactivity-loss-async-after-sync/main.svelte new file mode 100644 index 0000000000..d3bcc199a4 --- /dev/null +++ b/packages/svelte/tests/runtime-runes/samples/async-reactivity-loss-async-after-sync/main.svelte @@ -0,0 +1,27 @@ + + + + +{await get()} diff --git a/packages/svelte/tests/runtime-runes/samples/async-reactivity-loss-for-await-break-return/_config.js b/packages/svelte/tests/runtime-runes/samples/async-reactivity-loss-for-await-break-return/_config.js index 3429380800..7fb54d763c 100644 --- a/packages/svelte/tests/runtime-runes/samples/async-reactivity-loss-for-await-break-return/_config.js +++ b/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`' } ]); } diff --git a/packages/svelte/tests/runtime-runes/samples/async-reactivity-loss-for-await/_config.js b/packages/svelte/tests/runtime-runes/samples/async-reactivity-loss-for-await/_config.js index a5dd7fa28a..ddc9cef27d 100644 --- a/packages/svelte/tests/runtime-runes/samples/async-reactivity-loss-for-await/_config.js +++ b/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`' } ]); } diff --git a/packages/svelte/tests/runtime-runes/samples/async-reactivity-loss-no-false-positive-1/_config.js b/packages/svelte/tests/runtime-runes/samples/async-reactivity-loss-no-false-positive-1/_config.js new file mode 100644 index 0000000000..516fa59a97 --- /dev/null +++ b/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); + } +}); diff --git a/packages/svelte/tests/runtime-runes/samples/async-reactivity-loss-no-false-positive-1/main.svelte b/packages/svelte/tests/runtime-runes/samples/async-reactivity-loss-no-false-positive-1/main.svelte new file mode 100644 index 0000000000..9af5206705 --- /dev/null +++ b/packages/svelte/tests/runtime-runes/samples/async-reactivity-loss-no-false-positive-1/main.svelte @@ -0,0 +1,16 @@ + + +{x} {await foo(y)} + + + diff --git a/packages/svelte/tests/runtime-runes/samples/async-reactivity-loss-no-false-positive-2/_config.js b/packages/svelte/tests/runtime-runes/samples/async-reactivity-loss-no-false-positive-2/_config.js new file mode 100644 index 0000000000..6c774609e5 --- /dev/null +++ b/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); + } +}); diff --git a/packages/svelte/tests/runtime-runes/samples/async-reactivity-loss-no-false-positive-2/main.svelte b/packages/svelte/tests/runtime-runes/samples/async-reactivity-loss-no-false-positive-2/main.svelte new file mode 100644 index 0000000000..ea7e9dcc9b --- /dev/null +++ b/packages/svelte/tests/runtime-runes/samples/async-reactivity-loss-no-false-positive-2/main.svelte @@ -0,0 +1,14 @@ + + +{await delay(count)} + + diff --git a/packages/svelte/tests/runtime-runes/samples/async-reactivity-loss-no-false-positive-3/_config.js b/packages/svelte/tests/runtime-runes/samples/async-reactivity-loss-no-false-positive-3/_config.js new file mode 100644 index 0000000000..c9c02336bc --- /dev/null +++ b/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); + } +}); diff --git a/packages/svelte/tests/runtime-runes/samples/async-reactivity-loss-no-false-positive-3/main.svelte b/packages/svelte/tests/runtime-runes/samples/async-reactivity-loss-no-false-positive-3/main.svelte new file mode 100644 index 0000000000..a016655b7c --- /dev/null +++ b/packages/svelte/tests/runtime-runes/samples/async-reactivity-loss-no-false-positive-3/main.svelte @@ -0,0 +1,14 @@ + + + +{await get()}