fix: ensure unowned derived signals correctly re-connect to graph (#13184)

Fixes #13174. Turns out that we we skipping this important work because we return true early – thus not connecting the unowned derived back to the reaction.

Author: trueadm

.changeset/rotten-actors-rush.md

@@ -0,0 +1,5 @@
+---
+'svelte': patch
+---
+
+fix: ensure unowned derived signals correctly re-connect to graph

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

@@ -196,18 +196,20 @@ export function check_dirtiness(reaction) {
 					update_derived(/** @type {Derived} */ (dependency));
 				}
 
-				if (dependency.version > reaction.version) {
-					return true;
+				// If we are working with an unowned signal as part of an effect (due to !current_skip_reaction)
+				// and the version hasn't changed, we still need to check that this reaction
+				// is linked to the dependency source – otherwise future updates will not be caught.
+				if (
+					is_unowned &&
+					current_effect !== null &&
+					!current_skip_reaction &&
+					!dependency?.reactions?.includes(reaction)
+				) {
+					(dependency.reactions ??= []).push(reaction);
 				}
 
-				if (is_unowned) {
-					// TODO is there a more logical place to do this work?
-					if (!current_skip_reaction && !dependency?.reactions?.includes(reaction)) {
-						// If we are working with an unowned signal as part of an effect (due to !current_skip_reaction)
-						// and the version hasn't changed, we still need to check that this reaction
-						// if linked to the dependency source – otherwise future updates will not be caught.
-						(dependency.reactions ??= []).push(reaction);
-					}
+				if (dependency.version > reaction.version) {
+					return true;
 				}
 			}
 		}

packages/svelte/tests/runtime-runes/samples/derived-unowned-9/_config.js

@@ -0,0 +1,71 @@
+import { flushSync } from 'svelte';
+import { test } from '../../test';
+
+export default test({
+	mode: ['client'],
+	async test({ assert, target, logs }) {
+		let [btn1, btn2] = target.querySelectorAll('button');
+
+		btn1.click();
+		btn2.click();
+
+		flushSync();
+
+		assert.htmlEqual(
+			target.innerHTML,
+			`<button>a</button><button>b</button><div>1</div>\ndouble:\n2`
+		);
+
+		btn1.click();
+		btn2.click();
+
+		flushSync();
+
+		assert.htmlEqual(
+			target.innerHTML,
+			`<button>a</button><button>b</button><div>2</div>\ndouble:\n4`
+		);
+
+		btn1.click();
+
+		flushSync();
+
+		assert.htmlEqual(
+			target.innerHTML,
+			`<button>a</button><button>b</button><div>3</div>\ndouble:\n6`
+		);
+
+		btn1.click();
+
+		flushSync();
+
+		assert.htmlEqual(
+			target.innerHTML,
+			`<button>a</button><button>b</button><div>4</div>\ndouble:\n8`
+		);
+
+		btn1.click();
+
+		flushSync();
+
+		assert.htmlEqual(target.innerHTML, `<button>a</button><button>b</button><div>5</div>`);
+
+		btn1.click();
+
+		flushSync();
+
+		assert.htmlEqual(
+			target.innerHTML,
+			`<button>a</button><button>b</button><div>6</div>\ndouble:\n12`
+		);
+
+		btn1.click();
+
+		flushSync();
+
+		assert.htmlEqual(
+			target.innerHTML,
+			`<button>a</button><button>b</button><div>7</div>\ndouble:\n14`
+		);
+	}
+});

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

@@ -0,0 +1,24 @@
+<script>
+	let count = $state(0);
+
+	function increment() {
+		count += 1;
+	}
+
+	let double = $state();
+	function setDerived() {
+		const d = $derived(count * 2);
+		double = {
+			get v() { return d }
+		};
+	}
+</script>
+
+<button onclick={increment}>a</button>
+<button onclick={setDerived}>b</button>
+
+<div>{count}</div>
+
+{#if double && count % 5}
+	double: {double.v}
+{/if}