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()}