fix: don't depend on deriveds created inside the current reaction (#15564)

* WIP

* WIP

* add test

* update test

* changeset

* oops

* lint

.changeset/young-poets-wait.md

@@ -0,0 +1,5 @@
+---
+'svelte': patch
+---
+
+fix: don't depend on deriveds created inside the current reaction

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

@@ -101,7 +101,7 @@ export {
 	text,
 	props_id
 } from './dom/template.js';
-export { derived, derived_safe_equal } from './reactivity/deriveds.js';
+export { user_derived as derived, derived_safe_equal } from './reactivity/deriveds.js';
 export {
 	effect_tracking,
 	effect_root,
@@ -113,14 +113,7 @@ export {
 	user_effect,
 	user_pre_effect
 } from './reactivity/effects.js';
-export {
-	mutable_source,
-	mutate,
-	set,
-	source as state,
-	update,
-	update_pre
-} from './reactivity/sources.js';
+export { mutable_source, mutate, set, state, update, update_pre } from './reactivity/sources.js';
 export {
 	prop,
 	rest_props,

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

@@ -10,7 +10,7 @@ import {
 	object_prototype
 } from '../shared/utils.js';
 import { check_ownership, widen_ownership } from './dev/ownership.js';
-import { source, set } from './reactivity/sources.js';
+import { state as source, set } from './reactivity/sources.js';
 import { STATE_SYMBOL, STATE_SYMBOL_METADATA } from './constants.js';
 import { UNINITIALIZED } from '../../constants.js';
 import * as e from './errors.js';

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

@@ -8,7 +8,8 @@ import {
 	skip_reaction,
 	update_reaction,
 	increment_write_version,
-	set_active_effect
+	set_active_effect,
+	push_reaction_value
 } from '../runtime.js';
 import { equals, safe_equals } from './equality.js';
 import * as e from '../errors.js';
@@ -61,6 +62,19 @@ export function derived(fn) {
 	return signal;
 }
 
+/**
+ * @template V
+ * @param {() => V} fn
+ * @returns {Derived<V>}
+ */
+export function user_derived(fn) {
+	const d = derived(fn);
+
+	push_reaction_value(d);
+
+	return d;
+}
+
 /**
  * @template V
  * @param {() => V} fn

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

@@ -15,7 +15,8 @@ import {
 	set_reaction_sources,
 	check_dirtiness,
 	untracking,
-	is_destroying_effect
+	is_destroying_effect,
+	push_reaction_value
 } from '../runtime.js';
 import { equals, safe_equals } from './equality.js';
 import {
@@ -64,14 +65,6 @@ export function source(v, stack) {
 		wv: 0
 	};
 
-	if (active_reaction !== null && active_reaction.f & EFFECT_IS_UPDATING) {
-		if (reaction_sources === null) {
-			set_reaction_sources([signal]);
-		} else {
-			reaction_sources.push(signal);
-		}
-	}
-
 	if (DEV && tracing_mode_flag) {
 		signal.created = stack ?? get_stack('CreatedAt');
 		signal.debug = null;
@@ -80,6 +73,19 @@ export function source(v, stack) {
 	return signal;
 }
 
+/**
+ * @template V
+ * @param {V} v
+ * @param {Error | null} [stack]
+ */
+export function state(v, stack) {
+	const s = source(v, stack);
+
+	push_reaction_value(s);
+
+	return s;
+}
+
 /**
  * @template V
  * @param {V} initial_value

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

@@ -101,6 +101,17 @@ export function set_reaction_sources(sources) {
 	reaction_sources = sources;
 }
 
+/** @param {Value} value */
+export function push_reaction_value(value) {
+	if (active_reaction !== null && active_reaction.f & EFFECT_IS_UPDATING) {
+		if (reaction_sources === null) {
+			set_reaction_sources([value]);
+		} else {
+			reaction_sources.push(value);
+		}
+	}
+}
+
 /**
  * The dependencies of the reaction that is currently being executed. In many cases,
  * the dependencies are unchanged between runs, and so this will be `null` unless
@@ -875,21 +886,23 @@ export function get(signal) {
 
 	// Register the dependency on the current reaction signal.
 	if (active_reaction !== null && !untracking) {
-		var deps = active_reaction.deps;
-		if (signal.rv < read_version) {
-			signal.rv = read_version;
-			// If the signal is accessing the same dependencies in the same
-			// order as it did last time, increment `skipped_deps`
-			// rather than updating `new_deps`, which creates GC cost
-			if (new_deps === null && deps !== null && deps[skipped_deps] === signal) {
-				skipped_deps++;
-			} else if (new_deps === null) {
-				new_deps = [signal];
-			} else if (!skip_reaction || !new_deps.includes(signal)) {
-				// Normally we can push duplicated dependencies to `new_deps`, but if we're inside
-				// an unowned derived because skip_reaction is true, then we need to ensure that
-				// we don't have duplicates
-				new_deps.push(signal);
+		if (!reaction_sources?.includes(signal)) {
+			var deps = active_reaction.deps;
+			if (signal.rv < read_version) {
+				signal.rv = read_version;
+				// If the signal is accessing the same dependencies in the same
+				// order as it did last time, increment `skipped_deps`
+				// rather than updating `new_deps`, which creates GC cost
+				if (new_deps === null && deps !== null && deps[skipped_deps] === signal) {
+					skipped_deps++;
+				} else if (new_deps === null) {
+					new_deps = [signal];
+				} else if (!skip_reaction || !new_deps.includes(signal)) {
+					// Normally we can push duplicated dependencies to `new_deps`, but if we're inside
+					// an unowned derived because skip_reaction is true, then we need to ensure that
+					// we don't have duplicates
+					new_deps.push(signal);
+				}
 			}
 		}
 	} else if (

packages/svelte/tests/runtime-runes/samples/effect-cleanup/_config.js

@@ -10,6 +10,6 @@ export default test({
 		flushSync(() => {
 			b1.click();
 		});
-		assert.deepEqual(logs, ['init 0', 'cleanup 2', null, 'init 2', 'cleanup 4', null, 'init 4']);
+		assert.deepEqual(logs, ['init 0']);
 	}
 });

packages/svelte/tests/runtime-runes/samples/untrack-own-deriveds/_config.js

@@ -0,0 +1,20 @@
+import { flushSync } from 'svelte';
+import { test } from '../../test';
+
+export default test({
+	test({ assert, target, logs }) {
+		const button = target.querySelector('button');
+
+		flushSync(() => button?.click());
+
+		assert.htmlEqual(
+			target.innerHTML,
+			`
+				<button>increment</button>
+				<p>1/2</p
+			`
+		);
+
+		assert.deepEqual(logs, [0, 0]);
+	}
+});

packages/svelte/tests/runtime-runes/samples/untrack-own-deriveds/main.svelte

@@ -0,0 +1,26 @@
+<script>
+	class Foo {
+		value = $state(0);
+		double = $derived(this.value * 2);
+
+		constructor() {
+			console.log(this.value, this.double);
+		}
+
+		increment() {
+			this.value++;
+		}
+	}
+
+	let foo = $state();
+
+	$effect(() => {
+		foo = new Foo();
+	});
+</script>
+
+<button onclick={() => foo.increment()}>increment</button>
+
+{#if foo}
+	<p>{foo.value}/{foo.double}</p>
+{/if}

packages/svelte/tests/signals/test.ts

@@ -8,12 +8,7 @@ import {
 	render_effect,
 	user_effect
 } from '../../src/internal/client/reactivity/effects';
-import {
-	source as state,
-	set,
-	update,
-	update_pre
-} from '../../src/internal/client/reactivity/sources';
+import { state, set, update, update_pre } from '../../src/internal/client/reactivity/sources';
 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';