fix: ensure signal write invalidation within effects is consistent (#14989)

* fix: ensure signal write invalidation within effects is persistent

* fix: ensure signal write invalidation within effects is persistent

* fix: ensure signal write invalidation within effects is persistent

* address feedback

Author: trueadm

.changeset/tricky-spiders-collect.md

@@ -0,0 +1,5 @@
+---
+'svelte': patch
+---
+
+fix: ensure signal write invalidation within effects is consistent

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

@@ -3,7 +3,6 @@ import { DEV } from 'esm-env';
 import {
 	component_context,
 	active_reaction,
-	new_deps,
 	active_effect,
 	untracked_writes,
 	get,
@@ -29,7 +28,8 @@ import {
 	INSPECT_EFFECT,
 	UNOWNED,
 	MAYBE_DIRTY,
-	BLOCK_EFFECT
+	BLOCK_EFFECT,
+	ROOT_EFFECT
 } from '../constants.js';
 import * as e from '../errors.js';
 import { legacy_mode_flag, tracing_mode_flag } from '../../flags/index.js';
@@ -182,26 +182,20 @@ export function internal_set(source, value) {
 
 		mark_reactions(source, DIRTY);
 
-		// If the current signal is running for the first time, it won't have any
-		// reactions as we only allocate and assign the reactions after the signal
-		// has fully executed. So in the case of ensuring it registers the reaction
+		// It's possible that the current reaction might not have up-to-date dependencies
+		// whilst it's actively running. So in the case of ensuring it registers the reaction
 		// properly for itself, we need to ensure the current effect actually gets
 		// scheduled. i.e: `$effect(() => x++)`
 		if (
 			is_runes() &&
 			active_effect !== null &&
 			(active_effect.f & CLEAN) !== 0 &&
-			(active_effect.f & BRANCH_EFFECT) === 0
+			(active_effect.f & (BRANCH_EFFECT | ROOT_EFFECT)) === 0
 		) {
-			if (new_deps !== null && new_deps.includes(source)) {
-				set_signal_status(active_effect, DIRTY);
-				schedule_effect(active_effect);
+			if (untracked_writes === null) {
+				set_untracked_writes([source]);
 			} else {
-				if (untracked_writes === null) {
-					set_untracked_writes([source]);
-				} else {
-					untracked_writes.push(source);
-				}
+				untracked_writes.push(source);
 			}
 		}
 

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

@@ -382,6 +382,34 @@ export function handle_error(error, effect, previous_effect, component_context)
 	}
 }
 
+/**
+ * @param {Value} signal
+ * @param {Effect} effect
+ * @param {number} [depth]
+ */
+function schedule_possible_effect_self_invalidation(signal, effect, depth = 0) {
+	var reactions = signal.reactions;
+	if (reactions === null) return;
+
+	for (var i = 0; i < reactions.length; i++) {
+		var reaction = reactions[i];
+		if ((reaction.f & DERIVED) !== 0) {
+			schedule_possible_effect_self_invalidation(
+				/** @type {Derived} */ (reaction),
+				effect,
+				depth + 1
+			);
+		} else if (effect === reaction) {
+			if (depth === 0) {
+				set_signal_status(reaction, DIRTY);
+			} else if ((reaction.f & CLEAN) !== 0) {
+				set_signal_status(reaction, MAYBE_DIRTY);
+			}
+			schedule_effect(/** @type {Effect} */ (reaction));
+		}
+	}
+}
+
 /**
  * @template V
  * @param {Reaction} reaction
@@ -434,6 +462,22 @@ export function update_reaction(reaction) {
 			deps.length = skipped_deps;
 		}
 
+		// If we're inside an effect and we have untracked writes, then we need to
+		// ensure that if any of those untracked writes result in re-invalidation
+		// of the current effect, then that happens accordingly
+		if (
+			is_runes() &&
+			untracked_writes !== null &&
+			(reaction.f & (DERIVED | MAYBE_DIRTY | DIRTY)) === 0
+		) {
+			for (i = 0; i < /** @type {Source[]} */ (untracked_writes).length; i++) {
+				schedule_possible_effect_self_invalidation(
+					untracked_writes[i],
+					/** @type {Effect} */ (reaction)
+				);
+			}
+		}
+
 		// If we are returning to an previous reaction then
 		// we need to increment the read version to ensure that
 		// any dependencies in this reaction aren't marked with
@@ -907,17 +951,6 @@ export function get(signal) {
 			} else {
 				new_deps.push(signal);
 			}
-
-			if (
-				untracked_writes !== null &&
-				active_effect !== null &&
-				(active_effect.f & CLEAN) !== 0 &&
-				(active_effect.f & BRANCH_EFFECT) === 0 &&
-				untracked_writes.includes(signal)
-			) {
-				set_signal_status(active_effect, DIRTY);
-				schedule_effect(active_effect);
-			}
 		}
 	} else if (is_derived && /** @type {Derived} */ (signal).deps === null) {
 		var derived = /** @type {Derived} */ (signal);

packages/svelte/tests/signals/test.ts

@@ -402,6 +402,42 @@ describe('signals', () => {
 		};
 	});
 
+	test('schedules rerun when writing to signal before reading it from derived', (runes) => {
+		if (!runes) return () => {};
+		let log: any[] = [];
+
+		const value = state(1);
+		const double = derived(() => $.get(value) * 2);
+
+		user_effect(() => {
+			set(value, 10);
+			log.push($.get(double));
+		});
+
+		return () => {
+			flushSync();
+			assert.deepEqual(log, [20]);
+		};
+	});
+
+	test('schedules rerun when writing to signal after reading it from derived', (runes) => {
+		if (!runes) return () => {};
+		let log: any[] = [];
+
+		const value = state(1);
+		const double = derived(() => $.get(value) * 2);
+
+		user_effect(() => {
+			log.push($.get(double));
+			set(value, 10);
+		});
+
+		return () => {
+			flushSync();
+			assert.deepEqual(log, [2, 20]);
+		};
+	});
+
 	test('effect teardown is removed on re-run', () => {
 		const count = state(0);
 		let first = true;