From 9f12fdd61f076983b5c0f6e399bfb95287a90fa6 Mon Sep 17 00:00:00 2001 From: Simon Holthausen Date: Tue, 13 Oct 2026 22:27:04 +0200 Subject: [PATCH] we need the "has pending batches" check to keep deriveds (maybe)dirty in more places after all --- .../src/internal/client/reactivity/batch.js | 17 +++++++ .../internal/client/reactivity/deriveds.js | 8 ++++ .../src/internal/client/reactivity/sources.js | 7 ++- .../svelte/src/internal/client/runtime.js | 6 ++- .../_config.js | 30 +++++++++++++ .../main.svelte | 44 +++++++++++++++++++ .../_config.js | 31 +++++++++++++ .../main.svelte | 43 ++++++++++++++++++ 8 files changed, 183 insertions(+), 3 deletions(-) create mode 100644 packages/svelte/tests/runtime-runes/samples/async-derived-read-keeps-maybe-dirty-direct/_config.js create mode 100644 packages/svelte/tests/runtime-runes/samples/async-derived-read-keeps-maybe-dirty-direct/main.svelte create mode 100644 packages/svelte/tests/runtime-runes/samples/async-derived-read-keeps-maybe-dirty/_config.js create mode 100644 packages/svelte/tests/runtime-runes/samples/async-derived-read-keeps-maybe-dirty/main.svelte diff --git a/packages/svelte/src/internal/client/reactivity/batch.js b/packages/svelte/src/internal/client/reactivity/batch.js index f99cb64d66..f5ebecd3d6 100644 --- a/packages/svelte/src/internal/client/reactivity/batch.js +++ b/packages/svelte/src/internal/client/reactivity/batch.js @@ -132,6 +132,23 @@ export function own_value(signal) { : signal; } +/** + * Whether some batch's view of the world (may) differ from the real one: there's a batch that is going to be + * processed (again) in its own view, and another batch, or it's a fork. The batch that is being committed and + * the one that hasn't been processed yet don't count, their view is the real one. Deriveds that are computed + * from the real values (e.g. in an event handler) must not be marked clean then, as those batches wouldn't + * re-validate them in their view (see `update_derived`) + */ +export function has_pending_batches() { + for (var batch = first_batch; batch !== null; batch = batch.next) { + if (batch !== previous_batch && batch !== current_batch) { + return batch.is_fork || /** @type {Batch} */ (first_batch).next !== null; + } + } + + return false; +} + /** @type {Effect | null} */ let last_scheduled_effect = null; diff --git a/packages/svelte/src/internal/client/reactivity/deriveds.js b/packages/svelte/src/internal/client/reactivity/deriveds.js index df475312f3..826326d71b 100644 --- a/packages/svelte/src/internal/client/reactivity/deriveds.js +++ b/packages/svelte/src/internal/client/reactivity/deriveds.js @@ -48,6 +48,7 @@ import { batch_values, current_batch, first_batch, + has_pending_batches, own_value, previous_batch, read_batch_local_value @@ -467,6 +468,13 @@ export function update_derived(derived) { } else if (derived.v !== UNINITIALIZED) { set_signal_status(derived, MAYBE_DIRTY); } + } else if (has_pending_batches()) { + // Computed from the real values (e.g. in an event handler) while other batches are in flight. + // Don't mark it clean: those batches see other values for its dependencies, and unless they + // stashed a reaction that reads it, they wouldn't re-validate it and would use whatever value + // they hold for it — which can be stale, e.g. if the batch that dirtied it never evaluated it + // (see `async-derived-read-keeps-maybe-dirty`) + set_signal_status(derived, MAYBE_DIRTY); } else { update_derived_status(derived); } diff --git a/packages/svelte/src/internal/client/reactivity/sources.js b/packages/svelte/src/internal/client/reactivity/sources.js index 7485062d8b..e823fcb266 100644 --- a/packages/svelte/src/internal/client/reactivity/sources.js +++ b/packages/svelte/src/internal/client/reactivity/sources.js @@ -38,6 +38,7 @@ import { schedule_effect, legacy_updates, current_batch, + has_pending_batches, own_value } from './batch.js'; import { proxy } from '../proxy.js'; @@ -208,7 +209,11 @@ export function internal_set(source, value, updated_during_traversal = null) { // During time traveling we don't want to reset the status so that // traversal of the graph in the other batches still happens - if (batch_values === null) { + if ( + batch_values === null && + // could also be "read outside of reactivity", e.g. in an event handler, while other batches are in flight + !has_pending_batches() + ) { update_derived_status(derived); } } diff --git a/packages/svelte/src/internal/client/runtime.js b/packages/svelte/src/internal/client/runtime.js index ca531ac737..3e4973861f 100644 --- a/packages/svelte/src/internal/client/runtime.js +++ b/packages/svelte/src/internal/client/runtime.js @@ -53,6 +53,7 @@ import { current_batch, first_batch, flushSync, + has_pending_batches, held_sources, previous_batch, read_batch_local_value, @@ -188,8 +189,9 @@ export function is_dirty(reaction) { // During time traveling we don't want to reset the status so that // traversal of the graph in the other batches still happens. Effects // can be reset because block/async effects execute right away and others - // are deferred and re-dirtied as needed. - (batch_values === null || (flags & DERIVED) === 0) + // are deferred and re-dirtied as needed. Deriveds also mustn't be reset + // outside of a batch's view while other batches are in flight (see `update_derived`) + ((flags & DERIVED) === 0 || (batch_values === null && !has_pending_batches())) ) { set_signal_status(reaction, CLEAN); } diff --git a/packages/svelte/tests/runtime-runes/samples/async-derived-read-keeps-maybe-dirty-direct/_config.js b/packages/svelte/tests/runtime-runes/samples/async-derived-read-keeps-maybe-dirty-direct/_config.js new file mode 100644 index 0000000000..dcec0fc11d --- /dev/null +++ b/packages/svelte/tests/runtime-runes/samples/async-derived-read-keeps-maybe-dirty-direct/_config.js @@ -0,0 +1,30 @@ +import { tick } from 'svelte'; +import { test } from '../../test'; + +// Like `async-derived-read-keeps-maybe-dirty`, but the `` also reads `s`, which makes it DIRTY +// rather than MAYBE_DIRTY, so A doesn't evaluate `d` even when checking the `` effect eagerly +// (`is_dirty` returns right away). C's read of `d` must still not mark it as clean +export default test({ + mode: ['client'], + async test({ assert, target, logs }) { + await tick(); + const [s, u, d, , resolve_b] = target.querySelectorAll('button'); + + // batch A: `d` becomes 10, but is not evaluated yet (the `` is deferred) + s.click(); + await tick(); + + // batch B + u.click(); + await tick(); + + // batch C writes `s` and reads `d` (20) while A and B are pending + d.click(); + await tick(); + + // B sees A's change (but not C's), so `d` is 10 in its world + resolve_b.click(); + await tick(); + assert.equal(logs[0], 'probe 10'); + } +}); diff --git a/packages/svelte/tests/runtime-runes/samples/async-derived-read-keeps-maybe-dirty-direct/main.svelte b/packages/svelte/tests/runtime-runes/samples/async-derived-read-keeps-maybe-dirty-direct/main.svelte new file mode 100644 index 0000000000..2f6bc41e2f --- /dev/null +++ b/packages/svelte/tests/runtime-runes/samples/async-derived-read-keeps-maybe-dirty-direct/main.svelte @@ -0,0 +1,44 @@ + + + + + + + + + +

{await delay('a', s)}

+

{await delay('c', w)}

+ +{#if show} + + {s}-{d} +{/if} + +{#if await delay('b', u)} + {await probe(d)} +{/if} diff --git a/packages/svelte/tests/runtime-runes/samples/async-derived-read-keeps-maybe-dirty/_config.js b/packages/svelte/tests/runtime-runes/samples/async-derived-read-keeps-maybe-dirty/_config.js new file mode 100644 index 0000000000..e6a608cb8c --- /dev/null +++ b/packages/svelte/tests/runtime-runes/samples/async-derived-read-keeps-maybe-dirty/_config.js @@ -0,0 +1,31 @@ +import { tick } from 'svelte'; +import { test } from '../../test'; + +// Like `async-derived-assign-keeps-maybe-dirty`, but C reads `d` in the event handler instead of +// assigning to it. That records `d`'s stale value (A never evaluated it) as C's previous value, and +// must not mark `d` as clean: B never stashed a reaction that reads `d`, so it wouldn't re-validate +// it and would use that stale value +export default test({ + mode: ['client'], + async test({ assert, target, logs }) { + await tick(); + const [s, u, d, , resolve_b] = target.querySelectorAll('button'); + + // batch A: `d` becomes 10, but is not evaluated yet (the `` is deferred) + s.click(); + await tick(); + + // batch B + u.click(); + await tick(); + + // batch C writes `s` and reads `d` (20) while A and B are pending + d.click(); + await tick(); + + // B sees A's change (but not C's), so `d` is 10 in its world + resolve_b.click(); + await tick(); + assert.equal(logs[0], 'probe 10'); + } +}); diff --git a/packages/svelte/tests/runtime-runes/samples/async-derived-read-keeps-maybe-dirty/main.svelte b/packages/svelte/tests/runtime-runes/samples/async-derived-read-keeps-maybe-dirty/main.svelte new file mode 100644 index 0000000000..67f0978c54 --- /dev/null +++ b/packages/svelte/tests/runtime-runes/samples/async-derived-read-keeps-maybe-dirty/main.svelte @@ -0,0 +1,43 @@ + + + + + + + + + +

{await delay('a', s)}

+

{await delay('c', w)}

+ +{#if show} + {d} +{/if} + +{#if await delay('b', u)} + {await probe(d)} +{/if}