fix: derived reactivity and perf regressions

.changeset/beige-sloths-cry.md

@@ -0,0 +1,5 @@
+---
+'svelte': patch
+---
+
+fix: performance regression affecting deriveds with no dependencies (#17342)

.changeset/free-phones-exist.md

@@ -0,0 +1,5 @@
+---
+'svelte': patch
+---
+
+fix: reconnect deriveds inside branch effects

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

@@ -22,7 +22,8 @@ import {
 	increment_write_version,
 	set_active_effect,
 	push_reaction_value,
-	is_destroying_effect
+	is_destroying_effect,
+	update_derived_status
 } from '../runtime.js';
 import { equals, safe_equals } from './equality.js';
 import * as e from '../errors.js';
@@ -360,7 +361,10 @@ export function update_derived(derived) {
 		// the underlying value will be updated when the fork is committed.
 		// otherwise, the next time we get here after a 'real world' state
 		// change, `derived.equals` may incorrectly return `true`
-		if (!current_batch?.is_fork) {
+		//
+		// deriveds with no deps should always update `derived.v`
+		// since they will never change and need the value after fork commits
+		if (!current_batch?.is_fork || derived.deps === null) {
 			derived.v = value;
 		}
 
@@ -381,8 +385,9 @@ export function update_derived(derived) {
 		if (effect_tracking() || current_batch?.is_fork) {
 			batch_values.set(derived, value);
 		}
-	} else {
-		var status = (derived.f & CONNECTED) === 0 ? MAYBE_DIRTY : CLEAN;
-		set_signal_status(derived, status);
+	}
+
+	if (batch_values === null || derived.deps === null) {
+		update_derived_status(derived);
 	}
 }

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

@@ -16,7 +16,8 @@ import {
 	is_destroying_effect,
 	push_reaction_value,
 	set_is_updating_effect,
-	is_updating_effect
+	is_updating_effect,
+	update_derived_status
 } from '../runtime.js';
 import { equals, safe_equals } from './equality.js';
 import {
@@ -218,12 +219,14 @@ export function internal_set(source, value) {
 		}
 
 		if ((source.f & DERIVED) !== 0) {
+			const derived = /** @type {Derived} */ (source);
+
 			// if we are assigning to a dirty derived we set it to clean/maybe dirty but we also eagerly execute it to track the dependencies
 			if ((source.f & DIRTY) !== 0) {
-				execute_derived(/** @type {Derived} */ (source));
+				execute_derived(derived);
 			}
 
-			set_signal_status(source, (source.f & CONNECTED) !== 0 ? CLEAN : MAYBE_DIRTY);
+			update_derived_status(derived);
 		}
 
 		source.wv = increment_write_version();

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

@@ -28,7 +28,6 @@ import { old_values } from './reactivity/sources.js';
 import {
 	destroy_derived_effects,
 	execute_derived,
-	current_async_effect,
 	recent_async_deriveds,
 	update_derived
 } from './reactivity/deriveds.js';
@@ -186,12 +185,12 @@ export function is_dirty(reaction) {
 		}
 
 		if (
-			(flags & CONNECTED) !== 0 &&
+			(flags & DERIVED) !== 0 &&
 			// During time traveling we don't want to reset the status so that
 			// traversal of the graph in the other batches still happens
-			batch_values === null
+			(batch_values === null || reaction.deps === null)
 		) {
-			set_signal_status(reaction, CLEAN);
+			update_derived_status(/** @type {Derived} */ (reaction));
 		}
 	}
 
@@ -629,7 +628,14 @@ export function get(signal) {
 			update_derived(derived);
 		}
 
-		if (is_updating_effect && effect_tracking() && (derived.f & CONNECTED) === 0) {
+		if (
+			(derived.f & CONNECTED) === 0 &&
+			((is_updating_effect &&
+				(effect_tracking() ||
+					(active_effect !== null && (active_effect.f & BRANCH_EFFECT) !== 0))) ||
+				// evaluating connected parent derived, so reconnect child deriveds too
+				(active_reaction !== null && (active_reaction.f & CONNECTED) !== 0))
+		) {
 			reconnect(derived);
 		}
 	}
@@ -729,6 +735,19 @@ export function set_signal_status(signal, status) {
 	signal.f = (signal.f & STATUS_MASK) | status;
 }
 
+/**
+ * Set a derived's status to CLEAN or MAYBE_DIRTY based on its connection state.
+ * @param {Derived} derived
+ */
+export function update_derived_status(derived) {
+	// Only mark as MAYBE_DIRTY if disconnected and has dependencies.
+	if ((derived.f & CONNECTED) !== 0 || derived.deps === null) {
+		set_signal_status(derived, CLEAN);
+	} else {
+		set_signal_status(derived, MAYBE_DIRTY);
+	}
+}
+
 /**
  * @param {Record<string | symbol, unknown>} obj
  * @param {Array<string | symbol>} keys

packages/svelte/tests/runtime-runes/samples/fork-derived-class-instance/Child.svelte

@@ -0,0 +1,10 @@
+<script>
+	class Y {
+		foo = $state(0);
+	}
+
+	let y = $derived(new Y());
+</script>
+
+<button onclick={() => y.foo++}>click</button>
+<p>{y.foo}</p>

packages/svelte/tests/runtime-runes/samples/fork-derived-class-instance/_config.js

@@ -0,0 +1,24 @@
+import { flushSync } from 'svelte';
+import { test } from '../../test';
+
+export default test({
+	skip_no_async: true,
+	async test({ assert, target }) {
+		const forkButton = target.querySelector('button');
+
+		flushSync(() => {
+			forkButton?.click();
+		});
+
+		const [, clickButton] = target.querySelectorAll('button');
+		const p = target.querySelector('p');
+
+		assert.equal(p?.textContent, '0');
+
+		flushSync(() => {
+			clickButton?.click();
+		});
+
+		assert.equal(p?.textContent, '1');
+	}
+});

packages/svelte/tests/runtime-runes/samples/fork-derived-class-instance/main.svelte

@@ -0,0 +1,17 @@
+<script>
+	import { fork } from 'svelte';
+	import Child from './Child.svelte';
+
+	let x = $state(false);
+</script>
+
+<button onclick={() => {
+	const f = fork(() => {
+		x = true;
+	});
+	f.commit();
+}}>fork</button>
+
+{#if x}
+	<Child />
+{/if}

packages/svelte/tests/signals/test.ts

@@ -1,3 +1,4 @@
+/** @import { Reaction } from '#client' */
 import { describe, assert, it } from 'vitest';
 import { flushSync } from '../../src/index-client';
 import * as $ from '../../src/internal/client/runtime';
@@ -15,9 +16,10 @@ 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';
+import { CLEAN, CONNECTED, DESTROYED, MAYBE_DIRTY } from '../../src/internal/client/constants';
 import { noop } from 'svelte/internal/client';
 import { disable_async_mode_flag, enable_async_mode_flag } from '../../src/internal/flags';
+import { branch } from '../../src/internal/client/reactivity/effects';
 
 /**
  * @param runes runes mode
@@ -1493,4 +1495,159 @@ describe('signals', () => {
 			assert.deepEqual(log, ['inner destroyed', 'inner destroyed']);
 		};
 	});
+
+	// reproduces conditions leading to https://github.com/sveltejs/kit/issues/15059
+	test('derived read inside branch effect should be reconnected even when effect_tracking is false', () => {
+		let count = state(0);
+		let d: Derived<number> | null = null;
+
+		render_effect(() => {
+			branch(() => {
+				if (!d) {
+					d = derived(() => $.get(count) * 2);
+				}
+
+				$.get(d);
+			});
+		});
+
+		return () => {
+			flushSync();
+
+			const isConnected = (d!.f & CONNECTED) !== 0;
+			assert.ok(
+				isConnected,
+				'derived should be CONNECTED after being read inside branch during effect update'
+			);
+
+			const countReactions = count.reactions || [];
+			assert.ok(countReactions.includes(d!), 'derived should be in source reactions');
+		};
+	});
+
+	// Test that deriveds with no dependencies are always CLEAN
+	test('deriveds with no deps should be CLEAN and not re-evaluate', () => {
+		let evalCount = 0;
+		let d: Derived<number> | null = null;
+
+		render_effect(() => {
+			branch(() => {
+				if (!d) {
+					d = derived(() => {
+						evalCount++;
+						return 42;
+					});
+				}
+
+				$.get(d);
+			});
+		});
+
+		return () => {
+			flushSync();
+
+			const initialEvalCount = evalCount;
+			assert.equal(initialEvalCount, 1, 'derived should evaluate once initially');
+
+			for (let i = 0; i < 100; i++) {
+				$.get(d!);
+			}
+
+			assert.equal(
+				evalCount,
+				initialEvalCount,
+				'derived with no deps should not re-evaluate on subsequent reads'
+			);
+
+			const isClean = (d!.f & CLEAN) !== 0;
+			assert.ok(isClean, 'derived with no deps should be CLEAN');
+		};
+	});
+
+	// reproduction of https://github.com/sveltejs/svelte/issues/17352
+	test('nested deriveds with parent-child dependencies stay reactive', () => {
+		const log: any[] = [];
+
+		return () => {
+			const expanded_ids = new SvelteSet<string>();
+			const nodes = proxy([
+				{
+					id: 'folder',
+					children: [{ id: 'file' }]
+				},
+				{ id: 'other' }
+			]);
+
+			let items_derived: Derived<any[]>;
+			let visible_items_derived: Derived<any[]>;
+
+			const destroy = effect_root(() => {
+				items_derived = derived(function create_items(
+					list: any[] = nodes,
+					parent?: any,
+					result: any[] = []
+				) {
+					for (const node of list) {
+						const expanded_d = derived(() => expanded_ids.has(node.id));
+						const visible_d = derived(() =>
+							parent === undefined ? true : $.get(parent.expanded_d) && $.get(parent.visible_d)
+						);
+
+						const item = {
+							node,
+							expanded_d,
+							visible_d,
+							get expanded() {
+								return $.get(expanded_d);
+							},
+							get visible() {
+								return $.get(visible_d);
+							}
+						};
+						result.push(item);
+
+						if (node.children) {
+							create_items(node.children, item, result);
+						}
+					}
+					return result;
+				});
+
+				visible_items_derived = derived(() => $.get(items_derived).filter((item) => item.visible));
+
+				render_effect(() => {
+					log.push($.get(visible_items_derived).length);
+				});
+			});
+
+			flushSync();
+			assert.deepEqual(log, [2], 'initial: folder and other visible');
+
+			expanded_ids.add('folder');
+			flushSync();
+			assert.deepEqual(log, [2, 3], 'after expand: folder, file, other visible');
+
+			expanded_ids.delete('folder');
+			flushSync();
+			assert.deepEqual(log, [2, 3, 2], 'after collapse');
+
+			nodes.splice(1, 1); // Remove 'other'
+
+			const snapshot = $.get(visible_items_derived!);
+			assert.equal(snapshot.length, 1, 'after delete: only folder visible');
+
+			flushSync();
+			assert.deepEqual(log, [2, 3, 2, 1], 'effect ran after delete');
+
+			expanded_ids.add('folder');
+			flushSync();
+			assert.deepEqual(
+				log,
+				[2, 3, 2, 1, 2],
+				'after expand post-delete: folder and file should be visible'
+			);
+
+			destroy();
+		};
+	});
 });