fix: attach effects-inside-deriveds to the parent of the derived (#13309)

* WIP

* fix types

* ensure effects with deriveds are added to the tree

* test

* changeset

* oops

.changeset/plenty-rings-stare.md

@@ -0,0 +1,5 @@
+---
+'svelte': patch
+---
+
+fix: attach effects-inside-deriveds to the parent of the derived

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

@@ -19,6 +19,7 @@ export const LEGACY_DERIVED_PROP = 1 << 16;
 export const INSPECT_EFFECT = 1 << 17;
 export const HEAD_EFFECT = 1 << 18;
 export const EFFECT_QUEUED = 1 << 19;
+export const EFFECT_HAS_DERIVED = 1 << 20;
 
 export const STATE_SYMBOL = Symbol('$state');
 export const STATE_SYMBOL_METADATA = Symbol('$state metadata');

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

@@ -1,6 +1,14 @@
 /** @import { Derived, Effect } from '#client' */
 import { DEV } from 'esm-env';
-import { CLEAN, DERIVED, DESTROYED, DIRTY, MAYBE_DIRTY, UNOWNED } from '../constants.js';
+import {
+	CLEAN,
+	DERIVED,
+	DESTROYED,
+	DIRTY,
+	EFFECT_HAS_DERIVED,
+	MAYBE_DIRTY,
+	UNOWNED
+} from '../constants.js';
 import {
 	active_reaction,
 	active_effect,
@@ -8,7 +16,8 @@ import {
 	set_signal_status,
 	skip_reaction,
 	update_reaction,
-	increment_version
+	increment_version,
+	set_active_effect
 } from '../runtime.js';
 import { equals, safe_equals } from './equality.js';
 import * as e from '../errors.js';
@@ -23,7 +32,14 @@ import { inspect_effects, set_inspect_effects } from './sources.js';
 /*#__NO_SIDE_EFFECTS__*/
 export function derived(fn) {
 	let flags = DERIVED | DIRTY;
-	if (active_effect === null) flags |= UNOWNED;
+
+	if (active_effect === null) {
+		flags |= UNOWNED;
+	} else {
+		// Since deriveds are evaluated lazily, any effects created inside them are
+		// created too late to ensure that the parent effect is added to the tree
+		active_effect.f |= EFFECT_HAS_DERIVED;
+	}
 
 	/** @type {Derived<V>} */
 	const signal = {
@@ -34,7 +50,8 @@ export function derived(fn) {
 		fn,
 		reactions: null,
 		v: /** @type {V} */ (null),
-		version: 0
+		version: 0,
+		parent: active_effect
 	};
 
 	if (active_reaction !== null && (active_reaction.f & DERIVED) !== 0) {
@@ -91,6 +108,9 @@ let stack = [];
  */
 export function update_derived(derived) {
 	var value;
+	var prev_active_effect = active_effect;
+
+	set_active_effect(derived.parent);
 
 	if (DEV) {
 		let prev_inspect_effects = inspect_effects;
@@ -105,12 +125,17 @@ export function update_derived(derived) {
 			destroy_derived_children(derived);
 			value = update_reaction(derived);
 		} finally {
+			set_active_effect(prev_active_effect);
 			set_inspect_effects(prev_inspect_effects);
 			stack.pop();
 		}
 	} else {
-		destroy_derived_children(derived);
-		value = update_reaction(derived);
+		try {
+			destroy_derived_children(derived);
+			value = update_reaction(derived);
+		} finally {
+			set_active_effect(prev_active_effect);
+		}
 	}
 
 	var status =

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

@@ -34,7 +34,8 @@ import {
 	CLEAN,
 	INSPECT_EFFECT,
 	HEAD_EFFECT,
-	MAYBE_DIRTY
+	MAYBE_DIRTY,
+	EFFECT_HAS_DERIVED
 } from '../constants.js';
 import { set } from './sources.js';
 import * as e from '../errors.js';
@@ -138,7 +139,8 @@ function create_effect(type, fn, sync, push = true) {
 		effect.deps === null &&
 		effect.first === null &&
 		effect.nodes_start === null &&
-		effect.teardown === null;
+		effect.teardown === null &&
+		(effect.f & EFFECT_HAS_DERIVED) === 0;
 
 	if (!inert && !is_root && push) {
 		if (parent_effect !== null) {

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

@@ -21,6 +21,7 @@ export interface Reaction extends Signal {
 	fn: null | Function;
 	/** Signals that this signal reads from */
 	deps: null | Value[];
+	parent: Effect | null;
 }
 
 export interface Derived<V = unknown> extends Value<V>, Reaction {
@@ -31,7 +32,6 @@ export interface Derived<V = unknown> extends Value<V>, Reaction {
 }
 
 export interface Effect extends Reaction {
-	parent: Effect | null;
 	/**
 	 * Branch effects store their start/end nodes so that they can be
 	 * removed when the effect is destroyed, or moved when an `each`

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

@@ -0,0 +1,16 @@
+import { flushSync } from 'svelte';
+import { test } from '../../test';
+
+export default test({
+	html: '<button>clicks: 0</button>',
+
+	test({ assert, target }) {
+		const button = target.querySelector('button');
+
+		flushSync(() => button?.click());
+		assert.htmlEqual(target.innerHTML, '<button>clicks: 1</button>');
+
+		flushSync(() => button?.click());
+		assert.htmlEqual(target.innerHTML, '<button>clicks: 2</button>');
+	}
+});

packages/svelte/tests/runtime-runes/samples/effect-inside-derived/main.svelte

@@ -0,0 +1,19 @@
+<script>
+	let count = $state(0);
+
+	const { value } = $derived.by(() => {
+		let value = $state(0);
+
+		$effect(() => {
+			value = count;
+		});
+
+		return {
+			get value() {
+				return value;
+			}
+		};
+	});
+</script>
+
+<button onclick={() => (count += 1)}>clicks: {value}</button>