fix: don't set derived values during time traveling (#17163)

* chore: failing test for derived + fork + block

* fix: don't set derived values during time travel

* fix: skip no async

* fix: only set `derived.v` outside a fork

* chore: simplify

Co-authored-by: Simon H <[email protected]>

* tidy up test a bit (missing text makes things confusing in the sandbox)

* update comment

* tweak

---------

Co-authored-by: Simon H <[email protected]>
Co-authored-by: Rich Harris <[email protected]>

Author: 3 people

packages/svelte/src/internal/client/reactivity/deriveds.js

@@ -356,9 +356,14 @@ export function update_derived(derived) {
 	var value = execute_derived(derived);
 
 	if (!derived.equals(value)) {
-		// TODO can we avoid setting `derived.v` when `batch_values !== null`,
-		// without causing the value to be stale later?
-		derived.v = value;
+		// in a fork, we don't update the underlying value, just `batch_values`.
+		// the underlying value will be updated when the fork is committed.
+		// otherwise, the next time we get here after a 'real world' state
+		// change, `derived.equals` may incorrectly return `true`
+		if (!current_batch?.is_fork) {
+			derived.v = value;
+		}
+
 		derived.wv = increment_write_version();
 	}
 
@@ -374,7 +379,7 @@ export function update_derived(derived) {
 		// only cache the value if we're in a tracking context, otherwise we won't
 		// clear the cache in `mark_reactions` when dependencies are updated
 		if (effect_tracking()) {
-			batch_values.set(derived, derived.v);
+			batch_values.set(derived, value);
 		}
 	} else {
 		var status = (derived.f & CONNECTED) === 0 ? MAYBE_DIRTY : CLEAN;

packages/svelte/src/internal/client/runtime.js

@@ -611,21 +611,19 @@ export function get(signal) {
 
 			return value;
 		}
-	} else if (is_derived) {
+	} else if (is_derived && !batch_values?.has(signal)) {
 		derived = /** @type {Derived} */ (signal);
 
-		if (batch_values?.has(derived)) {
-			return batch_values.get(derived);
-		}
-
 		if (is_dirty(derived)) {
 			update_derived(derived);
 		}
 
 		if (is_updating_effect && effect_tracking() && (derived.f & CONNECTED) === 0) {
 			reconnect(derived);
 		}
-	} else if (batch_values?.has(signal)) {
+	}
+
+	if (batch_values?.has(signal)) {
 		return batch_values.get(signal);
 	}
 

packages/svelte/tests/runtime-runes/samples/fork-derived-value/_config.js

@@ -0,0 +1,20 @@
+import { flushSync } from 'svelte';
+import { test } from '../../test';
+
+export default test({
+	skip_no_async: true,
+	async test({ assert, target }) {
+		const [fork, update] = target.querySelectorAll('button');
+
+		flushSync(() => {
+			fork.click();
+		});
+		flushSync(() => {
+			update.click();
+		});
+
+		const p = target.querySelector('p');
+
+		assert.equal(p?.textContent, 'one');
+	}
+});

packages/svelte/tests/runtime-runes/samples/fork-derived-value/main.svelte

@@ -0,0 +1,20 @@
+<script>
+	import { fork } from "svelte";
+
+	let state = $state(0);
+	let count = $derived(state);
+</script>
+
+<button onclick={() => {
+	fork(() => {
+		state++;
+	});
+}}>fork</button>
+
+<button onclick={() => {
+	state++;
+}}>update</button>
+
+{#if count === 1}
+	<p>one</p>
+{/if}