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. #17024 (comment) 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 #17049 (comment) (and therefore everything that was still buggy in that issue I think)

Author: dummdidumm

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';

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) {

packages/svelte/tests/runtime-runes/samples/async-derived-unowned/Component.svelte

@@ -0,0 +1,6 @@
+<script>
+	let { double } = $props();
+	double; // forces derived into UNOWNED mode
+</script>
+
+<p>{double}</p>

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,
+			`
+				<button>1</button>
+				<p>2</p>
+			`
+		);
+
+		button?.click();
+		await tick();
+
+		assert.htmlEqual(
+			target.innerHTML,
+			`
+				<button>2</button>
+				<p>4</p>
+			`
+		);
+	}
+});

packages/svelte/tests/runtime-runes/samples/async-derived-unowned/main.svelte

@@ -0,0 +1,19 @@
+<script>
+	import Component from './Component.svelte';
+	let count = $state(0);
+	const double = $derived(count * 2);
+</script>
+
+<svelte:boundary>
+	{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}
+</svelte:boundary>
+
+<button onclick={() => count += 1}>{count}</button>
+
+{#if count > 0}
+	<Component {double} />
+{/if}