fix: keep reactions up to date even when read outside of effect (#17295)

In #17105 one line in `update_reaction` was changed that can cause reactivity loss. It checks if the reaction is updated inside of an effect and only then will push to the reactions. The prior version had an additional check to still add to the reactions if there is already at least one reaction on the derived, indicating it is connected. Removing this check fixes #17263 while keeping correctness: a connected derived by definition at least has one reaction and therefore can properly cleanup.

Author: dummdidumm

.changeset/small-llamas-drum.md

@@ -0,0 +1,5 @@
+---
+'svelte': patch
+---
+
+fix: keep reactions up to date even when read outside of effect

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

@@ -278,7 +278,7 @@ export function update_reaction(reaction) {
 				reaction.deps = deps = new_deps;
 			}
 
-			if (is_updating_effect && effect_tracking() && (reaction.f & CONNECTED) !== 0) {
+			if (effect_tracking() && (reaction.f & CONNECTED) !== 0) {
 				for (i = skipped_deps; i < deps.length; i++) {
 					(deps[i].reactions ??= []).push(reaction);
 				}

packages/svelte/tests/signals/test.ts

@@ -1418,6 +1418,31 @@ describe('signals', () => {
 		};
 	});
 
+	test('derived when connected should add new dependency to its reaction even when read outside effect', () => {
+		let count_a = state(0);
+		let count_b = state(0);
+		let which = state(true);
+		let double = derived(() => ($.get(which) ? $.get(count_a) * 2 : $.get(count_b) * 2));
+
+		render_effect(() => {
+			$.get(double);
+		});
+
+		return () => {
+			flushSync();
+			assert.equal($.get(double!), 0);
+
+			set(which, false);
+			$.get(double); // read before render effect has a chance to rerun
+			flushSync();
+			assert.equal($.get(double!), 0);
+
+			set(count_b, 1);
+			flushSync();
+			assert.equal($.get(double!), 2);
+		};
+	});
+
 	test('$effect.root inside deriveds stay alive independently', () => {
 		const log: any[] = [];
 		const c = state(0);