From c141887c573985670ce015f6905f3819be430055 Mon Sep 17 00:00:00 2001 From: Simon Holthausen Date: Sat, 1 Nov 2025 23:20:36 +0100 Subject: [PATCH] fix: traverse graph when time traveling Often deriveds are currently unowned but through graph traversal during `is_dirty()` might reconnect to the graph, therefore having dependent effects register themselves as reactions. The problem prior to this fix was that the graph was not traversed to update the derived's dependencies and unowned status of deriveds in batch_values - the batch value was just returned -, which results in reactivity loss. https://github.com/sveltejs/svelte/issues/17024#issuecomment-3476761662 contains a detailed rundown of such a case. The fix is to still traverse the graph, though not executing any deriveds in the process. Fixes #17024 Fixes https://github.com/sveltejs/svelte/issues/17049#issuecomment-3459910165 (and therefore everything that was still buggy in that issue I think) --- .../client/dom/elements/transitions.js | 8 +---- .../svelte/src/internal/client/runtime.js | 36 ++++++++++++------- .../async-derived-unowned/Component.svelte | 6 ++++ .../samples/async-derived-unowned/_config.js | 30 ++++++++++++++++ .../samples/async-derived-unowned/main.svelte | 19 ++++++++++ 5 files changed, 79 insertions(+), 20 deletions(-) create mode 100644 packages/svelte/tests/runtime-runes/samples/async-derived-unowned/Component.svelte create mode 100644 packages/svelte/tests/runtime-runes/samples/async-derived-unowned/_config.js create mode 100644 packages/svelte/tests/runtime-runes/samples/async-derived-unowned/main.svelte diff --git a/packages/svelte/src/internal/client/dom/elements/transitions.js b/packages/svelte/src/internal/client/dom/elements/transitions.js index 00fad9ffdb58..d1d034d402d3 100644 --- a/packages/svelte/src/internal/client/dom/elements/transitions.js +++ b/packages/svelte/src/internal/client/dom/elements/transitions.js @@ -1,13 +1,7 @@ /** @import { AnimateFn, Animation, AnimationConfig, EachItem, Effect, TransitionFn, TransitionManager } from '#client' */ import { noop, is_function } from '../../../shared/utils.js'; import { effect } from '../../reactivity/effects.js'; -import { - active_effect, - active_reaction, - set_active_effect, - set_active_reaction, - untrack -} from '../../runtime.js'; +import { active_effect, untrack } from '../../runtime.js'; import { loop } from '../../loop.js'; import { should_intro } from '../../render.js'; import { current_each_item } from '../blocks/each.js'; diff --git a/packages/svelte/src/internal/client/runtime.js b/packages/svelte/src/internal/client/runtime.js index 2197f34d16ad..13faecabeca0 100644 --- a/packages/svelte/src/internal/client/runtime.js +++ b/packages/svelte/src/internal/client/runtime.js @@ -148,24 +148,33 @@ export function increment_write_version() { /** * Determines whether a derived or effect is dirty. * If it is MAYBE_DIRTY, will set the status to CLEAN + * + * By default is_dirty executes deriveds and marks them as clean if not unowned etc. + * But when multiple batches are active, batch_values may contain a value for a derived. + * In this case we don't want to execute the derived (or its dependencies), but still + * traverse the graph in order to reconnect unowned deriveds to their dependencies. * @param {Reaction} reaction + * @param {boolean} [run_deriveds] * @returns {boolean} */ -export function is_dirty(reaction) { +export function is_dirty(reaction, run_deriveds = true) { var flags = reaction.f; + var dirty = (flags & DIRTY) !== 0; - if ((flags & DIRTY) !== 0) { + if (dirty && run_deriveds) { return true; } - if ((flags & MAYBE_DIRTY) !== 0) { + // We don't need this above the DIRTY check because if it's dirty + // the related derived update logic which is then called will also reset the flag + if ((flags & DERIVED) !== 0) { + reaction.f &= ~WAS_MARKED; + } + + if ((flags & MAYBE_DIRTY) !== 0 || dirty) { var dependencies = reaction.deps; var is_unowned = (flags & UNOWNED) !== 0; - if (flags & DERIVED) { - reaction.f &= ~WAS_MARKED; - } - if (dependencies !== null) { var i; var dependency; @@ -208,7 +217,7 @@ export function is_dirty(reaction) { for (i = 0; i < length; i++) { dependency = dependencies[i]; - if (is_dirty(/** @type {Derived} */ (dependency))) { + if (is_dirty(/** @type {Derived} */ (dependency), run_deriveds) && run_deriveds) { update_derived(/** @type {Derived} */ (dependency)); } @@ -220,7 +229,7 @@ export function is_dirty(reaction) { // Unowned signals should never be marked as clean unless they // are used within an active_effect without skip_reaction - if (!is_unowned || (active_effect !== null && !skip_reaction)) { + if ((!is_unowned || (active_effect !== null && !skip_reaction)) && run_deriveds) { set_signal_status(reaction, CLEAN); } } @@ -678,16 +687,17 @@ export function get(signal) { derived = /** @type {Derived} */ (signal); if (batch_values?.has(derived)) { + is_dirty(derived, false); return batch_values.get(derived); } if (is_dirty(derived)) { update_derived(derived); } - } - - if (batch_values?.has(signal)) { - return batch_values.get(signal); + } else { + if (batch_values?.has(signal)) { + return batch_values.get(signal); + } } if ((signal.f & ERROR_VALUE) !== 0) { diff --git a/packages/svelte/tests/runtime-runes/samples/async-derived-unowned/Component.svelte b/packages/svelte/tests/runtime-runes/samples/async-derived-unowned/Component.svelte new file mode 100644 index 000000000000..36ad0dfaea28 --- /dev/null +++ b/packages/svelte/tests/runtime-runes/samples/async-derived-unowned/Component.svelte @@ -0,0 +1,6 @@ + + +

{double}

diff --git a/packages/svelte/tests/runtime-runes/samples/async-derived-unowned/_config.js b/packages/svelte/tests/runtime-runes/samples/async-derived-unowned/_config.js new file mode 100644 index 000000000000..fc0135623d7a --- /dev/null +++ b/packages/svelte/tests/runtime-runes/samples/async-derived-unowned/_config.js @@ -0,0 +1,30 @@ +import { tick } from 'svelte'; +import { test } from '../../test'; + +export default test({ + async test({ assert, target }) { + const button = target.querySelector('button'); + + button?.click(); + await tick(); + + assert.htmlEqual( + target.innerHTML, + ` + +

2

+ ` + ); + + button?.click(); + await tick(); + + assert.htmlEqual( + target.innerHTML, + ` + +

4

+ ` + ); + } +}); diff --git a/packages/svelte/tests/runtime-runes/samples/async-derived-unowned/main.svelte b/packages/svelte/tests/runtime-runes/samples/async-derived-unowned/main.svelte new file mode 100644 index 000000000000..bd82e35a3bc3 --- /dev/null +++ b/packages/svelte/tests/runtime-runes/samples/async-derived-unowned/main.svelte @@ -0,0 +1,19 @@ + + + + {await new Promise((r) => { + // long enough for the test to do all its other stuff while this is pending + setTimeout(r, 10); + })} + {#snippet pending()}{/snippet} + + + + +{#if count > 0} + +{/if}