fix: improve derived connection to ownership graph

.changeset/blue-rocks-play.md

@@ -0,0 +1,5 @@
+---
+'svelte': patch
+---
+
+fix: improve derived connection to ownership graph

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

@@ -87,7 +87,7 @@ export function derived_safe_equal(fn) {
  * @param {Derived} derived
  * @returns {void}
  */
-function destroy_derived_children(derived) {
+export function destroy_derived_children(derived) {
 	var children = derived.children;
 
 	if (children !== null) {

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

@@ -28,7 +28,12 @@ import {
 } from './constants.js';
 import { flush_tasks } from './dom/task.js';
 import { internal_set, set } from './reactivity/sources.js';
-import { destroy_derived, execute_derived, update_derived } from './reactivity/deriveds.js';
+import {
+	destroy_derived,
+	destroy_derived_children,
+	execute_derived,
+	update_derived
+} from './reactivity/deriveds.js';
 import * as e from './errors.js';
 import { FILENAME } from '../../constants.js';
 import { legacy_mode_flag, tracing_mode_flag } from '../flags/index.js';
@@ -517,6 +522,8 @@ function remove_reaction(signal, dependency) {
 		if ((dependency.f & (UNOWNED | DISCONNECTED)) === 0) {
 			dependency.f ^= DISCONNECTED;
 		}
+		// Disconnect any reactions owned by this reaction
+		destroy_derived_children(/** @type {Derived} **/ (dependency));
 		remove_reactions(/** @type {Derived} **/ (dependency), 0);
 	}
 }
@@ -934,13 +941,7 @@ export function get(signal) {
 				new_deps.push(signal);
 			}
 		}
-	}
-
-	if (
-		is_derived &&
-		/** @type {Derived} */ (signal).deps === null &&
-		(active_reaction === null || untracking || (active_reaction.f & DERIVED) !== 0)
-	) {
+	} else if (is_derived && /** @type {Derived} */ (signal).deps === null) {
 		var derived = /** @type {Derived} */ (signal);
 		var parent = derived.parent;
 

packages/svelte/tests/signals/test.ts

@@ -9,11 +9,12 @@ import {
 	user_effect
 } from '../../src/internal/client/reactivity/effects';
 import { state, set } from '../../src/internal/client/reactivity/sources';
-import type { Derived, Value } from '../../src/internal/client/types';
+import type { Derived, Effect, Value } from '../../src/internal/client/types';
 import { proxy } from '../../src/internal/client/proxy';
 import { derived } from '../../src/internal/client/reactivity/deriveds';
 import { snapshot } from '../../src/internal/shared/clone.js';
 import { SvelteSet } from '../../src/reactivity/set';
+import { DESTROYED } from '../../src/internal/client/constants';
 
 /**
  * @param runes runes mode
@@ -256,12 +257,16 @@ describe('signals', () => {
 
 			const a = state(0);
 			const b = state(0);
-			const c = derived(() => {
-				const a_2 = derived(() => $.get(a) + '!');
-				const b_2 = derived(() => $.get(b) + '?');
-				nested.push(a_2, b_2);
+			let c: any;
+
+			const destroy = effect_root(() => {
+				c = derived(() => {
+					const a_2 = derived(() => $.get(a) + '!');
+					const b_2 = derived(() => $.get(b) + '?');
+					nested.push(a_2, b_2);
 
-				return { a: $.get(a_2), b: $.get(b_2) };
+					return { a: $.get(a_2), b: $.get(b_2) };
+				});
 			});
 
 			$.get(c);
@@ -274,11 +279,14 @@ describe('signals', () => {
 
 			$.get(c);
 
-			// Ensure we're not leaking dependencies
-			assert.deepEqual(
-				nested.slice(0, -2).map((s) => s.deps),
-				[null, null, null, null]
-			);
+			// Ensure we're not leaking
+			assert.equal(a.reactions?.[0], nested.at(-2));
+			assert.equal(b.reactions?.[0], nested.at(-1));
+
+			destroy();
+
+			assert.equal(a.reactions, null);
+			assert.equal(b.reactions, null);
 		};
 	});
 
@@ -831,12 +839,54 @@ describe('signals', () => {
 		};
 	});
 
+	test('deriveds containing effects work correctly', () => {
+		return () => {
+			let a = render_effect(() => {});
+			let b = state(0);
+			let c;
+			let effects: Effect[] = [];
+
+			const destroy = effect_root(() => {
+				a = render_effect(() => {
+					c = derived(() => {
+						effects.push(
+							effect(() => {
+								$.get(b);
+							})
+						);
+						$.get(b);
+					});
+					$.get(c);
+				});
+			});
+
+			assert.equal(c!.children?.length, 1);
+			assert.equal(a.first, a.last);
+
+			set(b, 1);
+
+			flushSync();
+
+			assert.equal(c!.children?.length, 1);
+			assert.equal(a.first, a.last);
+
+			destroy();
+
+			assert.equal(a.deriveds, null);
+			assert.equal(a.first, null);
+
+			assert.equal(effects.length, 2);
+			assert.equal((effects[0].f & DESTROYED) !== 0, true);
+			assert.equal((effects[1].f & DESTROYED) !== 0, true);
+		};
+	});
+
 	test('deriveds containing effects work correctly when used with untrack', () => {
 		return () => {
 			let a = render_effect(() => {});
 			let b = state(0);
 			let c;
-			let effects = [];
+			let effects: Effect[] = [];
 
 			const destroy = effect_root(() => {
 				a = render_effect(() => {
@@ -854,20 +904,24 @@ describe('signals', () => {
 				});
 			});
 
-			assert.deepEqual(c!.children?.length, 1);
-			assert.deepEqual(a.first, a.last);
+			assert.equal(c!.children?.length, 1);
+			assert.equal(a.first, a.last);
 
 			set(b, 1);
 
 			flushSync();
 
-			assert.deepEqual(c!.children?.length, 1);
-			assert.deepEqual(a.first, a.last);
+			assert.equal(c!.children?.length, 1);
+			assert.equal(a.first, a.last);
 
 			destroy();
 
-			assert.deepEqual(a.deriveds, null);
-			assert.deepEqual(a.first, null);
+			assert.equal(a.deriveds, null);
+			assert.equal(a.first, null);
+
+			assert.equal(effects.length, 2);
+			assert.equal((effects[0].f & DESTROYED) !== 0, true);
+			assert.equal((effects[1].f & DESTROYED) !== 0, true);
 		};
 	});