fix: ensure reactivity system remains consistent with removals (#13087)

* fix: ensure reactivity system remains consistent with removals

* more fixes

* add test

Author: trueadm

.changeset/chilly-waves-count.md

@@ -0,0 +1,5 @@
+---
+'svelte': patch
+---
+
+fix: ensure reactivity system remains consistent with removals

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

@@ -1,4 +1,4 @@
-/** @import { Derived } from '#client' */
+/** @import { Derived, Effect } from '#client' */
 import { DEV } from 'esm-env';
 import { CLEAN, DERIVED, DESTROYED, DIRTY, MAYBE_DIRTY, UNOWNED } from '../constants.js';
 import {
@@ -8,11 +8,11 @@ import {
 	set_signal_status,
 	current_skip_reaction,
 	update_reaction,
-	destroy_effect_children,
 	increment_version
 } from '../runtime.js';
 import { equals, safe_equals } from './equality.js';
 import * as e from '../errors.js';
+import { destroy_effect } from './effects.js';
 
 /**
  * @template V
@@ -26,25 +26,19 @@ export function derived(fn) {
 
 	/** @type {Derived<V>} */
 	const signal = {
+		children: null,
 		deps: null,
-		deriveds: null,
 		equals,
 		f: flags,
-		first: null,
 		fn,
-		last: null,
 		reactions: null,
 		v: /** @type {V} */ (null),
 		version: 0
 	};
 
 	if (current_reaction !== null && (current_reaction.f & DERIVED) !== 0) {
-		var current_derived = /** @type {Derived} */ (current_reaction);
-		if (current_derived.deriveds === null) {
-			current_derived.deriveds = [signal];
-		} else {
-			current_derived.deriveds.push(signal);
-		}
+		var derived = /** @type {Derived} */ (current_reaction);
+		(derived.children ??= []).push(signal);
 	}
 
 	return signal;
@@ -67,14 +61,18 @@ export function derived_safe_equal(fn) {
  * @returns {void}
  */
 function destroy_derived_children(derived) {
-	destroy_effect_children(derived);
-	var deriveds = derived.deriveds;
+	var children = derived.children;
 
-	if (deriveds !== null) {
-		derived.deriveds = null;
+	if (children !== null) {
+		derived.children = null;
 
-		for (var i = 0; i < deriveds.length; i += 1) {
-			destroy_derived(deriveds[i]);
+		for (var i = 0; i < children.length; i += 1) {
+			var child = children[i];
+			if ((child.f & DERIVED) !== 0) {
+				destroy_derived(/** @type {Derived} */ (child));
+			} else {
+				destroy_effect(/** @type {Effect} */ (child));
+			}
 		}
 	}
 }
@@ -135,8 +133,7 @@ function destroy_derived(signal) {
 
 	// TODO we need to ensure we remove the derived from any parent derives
 
-	signal.first =
-		signal.last =
+	signal.children =
 		signal.deps =
 		signal.reactions =
 		// @ts-expect-error `signal.fn` cannot be `null` while the signal is alive

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

@@ -1,4 +1,4 @@
-/** @import { ComponentContext, ComponentContextLegacy, Effect, Reaction, TemplateNode, TransitionManager } from '#client' */
+/** @import { ComponentContext, ComponentContextLegacy, Derived, Effect, Reaction, TemplateNode, TransitionManager } from '#client' */
 import {
 	check_dirtiness,
 	current_component_context,
@@ -61,7 +61,7 @@ export function validate_effect(rune) {
 
 /**
  * @param {Effect} effect
- * @param {Reaction} parent_effect
+ * @param {Effect} parent_effect
  */
 function push_effect(effect, parent_effect) {
 	var parent_last = parent_effect.last;
@@ -147,7 +147,8 @@ function create_effect(type, fn, sync, push = true) {
 
 		// if we're in a derived, add the effect there too
 		if (current_reaction !== null && (current_reaction.f & DERIVED) !== 0) {
-			push_effect(effect, current_reaction);
+			var derived = /** @type {Derived} */ (current_reaction);
+			(derived.children ??= []).push(effect);
 		}
 	}
 
@@ -396,7 +397,7 @@ export function destroy_effect(effect, remove_dom = true) {
 	var parent = effect.parent;
 
 	// If the parent doesn't have any children, then skip this work altogether
-	if (parent !== null && (effect.f & BRANCH_EFFECT) !== 0 && parent.first !== null) {
+	if (parent !== null && parent.first !== null) {
 		unlink_effect(effect);
 	}
 

packages/svelte/src/internal/client/reactivity/types.d.ts

@@ -21,17 +21,13 @@ export interface Reaction extends Signal {
 	fn: null | Function;
 	/** Signals that this signal reads from */
 	deps: null | Value[];
-	/** First child effect created inside this signal */
-	first: null | Effect;
-	/** Last child effect created inside this signal */
-	last: null | Effect;
 }
 
 export interface Derived<V = unknown> extends Value<V>, Reaction {
 	/** The derived function */
 	fn: () => V;
-	/** Deriveds created inside this signal */
-	deriveds: null | Derived[];
+	/** Reactions created inside this signal */
+	children: null | Reaction[];
 }
 
 export interface Effect extends Reaction {
@@ -56,6 +52,10 @@ export interface Effect extends Reaction {
 	prev: null | Effect;
 	/** Next sibling child effect created inside the parent signal */
 	next: null | Effect;
+	/** First child effect created inside this signal */
+	first: null | Effect;
+	/** Last child effect created inside this signal */
+	last: null | Effect;
 	/** Dev only */
 	component_function?: any;
 }

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

@@ -392,7 +392,7 @@ export function remove_reactions(signal, start_index) {
 }
 
 /**
- * @param {Reaction} signal
+ * @param {Effect} signal
  * @param {boolean} remove_dom
  * @returns {void}
  */
@@ -601,9 +601,9 @@ function process_effects(effect, collected_effects) {
 			if ((flags & RENDER_EFFECT) !== 0) {
 				if (!is_branch && check_dirtiness(current_effect)) {
 					update_effect(current_effect);
-					// Child might have been mutated since running the effect
-					child = current_effect.first;
 				}
+				// Child might have been mutated since running the effect or checking dirtiness
+				child = current_effect.first;
 
 				if (child !== null) {
 					current_effect = child;

packages/svelte/tests/runtime-runes/samples/store-from-state-2/_config.js

@@ -0,0 +1,15 @@
+import { flushSync } from 'svelte';
+import { test } from '../../test';
+
+export default test({
+	test({ assert, target }) {
+		const button = target.querySelector('button');
+
+		flushSync(() => {
+			button?.click();
+			button?.click();
+			button?.click();
+		});
+		assert.htmlEqual(target.innerHTML, `<div>3</div><div>3,  6</div><button>increment</button>`);
+	}
+});

packages/svelte/tests/runtime-runes/samples/store-from-state-2/main.svelte

@@ -0,0 +1,17 @@
+<script>
+	import { fromStore } from 'svelte/store';
+	import { writable } from 'svelte/store';
+
+	const store = writable(0);
+
+	const value = fromStore(store);
+</script>
+
+<div>{$store}</div>
+
+{#if true}
+	{@const doubled = value.current * 2}
+	<div>{value.current}, {doubled}</div>
+{/if}
+
+<button onclick={() => $store++}>increment</button>

packages/svelte/tests/runtime-runes/samples/store-from-state/_config.js

@@ -0,0 +1,15 @@
+import { flushSync } from 'svelte';
+import { test } from '../../test';
+
+export default test({
+	test({ assert, target }) {
+		const button = target.querySelector('button');
+
+		flushSync(() => {
+			button?.click();
+			button?.click();
+			button?.click();
+		});
+		assert.htmlEqual(target.innerHTML, `\n3\n<button>Increment</button><br>`);
+	}
+});

packages/svelte/tests/runtime-runes/samples/store-from-state/main.svelte

@@ -0,0 +1,23 @@
+<script>
+  import { writable, fromStore } from 'svelte/store';
+	const store = writable(0)
+	const state_from_store= fromStore(store)
+
+	const derived_value= $derived.by(() => {
+		if (state_from_store.current > 10) {
+			return state_from_store.current
+		}
+		else{
+			return 10
+		}
+	})
+
+	function increment() {
+		$store += 1;
+	}
+</script>
+
+{state_from_store.current}
+<button onclick={increment}>Increment</button><br>
+
+{#if derived_value > 10 }Exceeded 10!{/if}