fix: make $effect.active() true when updating deriveds (#11500)

* fix: make `$effect.active()` true when updating deriveds

* WIP

* this seems to work?

* prevent effects being created in unowned deriveds

* update test

* fix issue

---------

Co-authored-by: Dominic Gannaway <[email protected]>

Author: Rich-Harris

.changeset/serious-goats-tap.md

@@ -0,0 +1,5 @@
+---
+'svelte': patch
+---
+
+fix: make `$effect.active()` true when updating deriveds

packages/svelte/messages/client-errors/errors.md

@@ -24,6 +24,10 @@
 
 > `%rune%` cannot be used inside an effect cleanup function
 
+## effect_in_unowned_derived
+
+> Effect cannot be created inside a `$derived` value that was not itself created inside an effect
+
 ## effect_orphan
 
 > `%rune%` can only be used inside an effect (e.g. during component initialisation)

packages/svelte/src/internal/client/dev/inspect.js

@@ -20,7 +20,7 @@ export let inspect_captured_signals = [];
  */
 // eslint-disable-next-line no-console
 export function inspect(get_value, inspector = console.log) {
-	validate_effect(current_effect, '$inspect');
+	validate_effect('$inspect');
 
 	let initial = true;
 

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

@@ -111,6 +111,22 @@ export function effect_in_teardown(rune) {
 	}
 }
 
+/**
+ * Effect cannot be created inside a `$derived` value that was not itself created inside an effect
+ * @returns {never}
+ */
+export function effect_in_unowned_derived() {
+	if (DEV) {
+		const error = new Error(`${"effect_in_unowned_derived"}\n${"Effect cannot be created inside a `$derived` value that was not itself created inside an effect"}`);
+
+		error.name = 'Svelte error';
+		throw error;
+	} else {
+		// TODO print a link to the documentation
+		throw new Error("effect_in_unowned_derived");
+	}
+}
+
 /**
  * `%rune%` can only be used inside an effect (e.g. during component initialisation)
  * @param {string} rune

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

@@ -121,6 +121,7 @@ export function update_derived(derived, force_schedule) {
  */
 export function destroy_derived(signal) {
 	destroy_derived_children(signal);
+	destroy_effect_children(signal);
 	remove_reactions(signal, 0);
 	set_signal_status(signal, DESTROYED);
 

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

@@ -26,20 +26,20 @@ import {
 	EFFECT_RAN,
 	BLOCK_EFFECT,
 	ROOT_EFFECT,
-	EFFECT_TRANSPARENT
+	EFFECT_TRANSPARENT,
+	DERIVED,
+	UNOWNED
 } from '../constants.js';
 import { set } from './sources.js';
 import { remove } from '../dom/reconciler.js';
 import * as e from '../errors.js';
 import { DEV } from 'esm-env';
 
 /**
- * @param {import('#client').Effect | null} effect
  * @param {'$effect' | '$effect.pre' | '$inspect'} rune
- * @returns {asserts effect}
  */
-export function validate_effect(effect, rune) {
-	if (effect === null) {
+export function validate_effect(rune) {
+	if (current_effect === null && current_reaction === null) {
 		e.effect_orphan(rune);
 	}
 
@@ -93,6 +93,18 @@ function create_effect(type, fn, sync) {
 	}
 
 	if (current_reaction !== null && !is_root) {
+		var flags = current_reaction.f;
+		if ((flags & DERIVED) !== 0) {
+			if ((flags & UNOWNED) !== 0) {
+				e.effect_in_unowned_derived();
+			}
+			// If we are inside a derived, then we also need to attach the
+			// effect to the parent effect too.
+			if (current_effect !== null) {
+				push_effect(effect, current_effect);
+			}
+		}
+
 		push_effect(effect, current_reaction);
 	}
 
@@ -118,20 +130,29 @@ function create_effect(type, fn, sync) {
  * @returns {boolean}
  */
 export function effect_active() {
-	return current_effect ? (current_effect.f & (BRANCH_EFFECT | ROOT_EFFECT)) === 0 : false;
+	if (current_reaction && (current_reaction.f & DERIVED) !== 0) {
+		return (current_reaction.f & UNOWNED) === 0;
+	}
+
+	if (current_effect) {
+		return (current_effect.f & (BRANCH_EFFECT | ROOT_EFFECT)) === 0;
+	}
+
+	return false;
 }
 
 /**
  * Internal representation of `$effect(...)`
  * @param {() => void | (() => void)} fn
  */
 export function user_effect(fn) {
-	validate_effect(current_effect, '$effect');
+	validate_effect('$effect');
 
 	// Non-nested `$effect(...)` in a component should be deferred
 	// until the component is mounted
 	const defer =
-		current_effect.f & RENDER_EFFECT &&
+		current_effect !== null &&
+		(current_effect.f & RENDER_EFFECT) !== 0 &&
 		// TODO do we actually need this? removing them changes nothing
 		current_component_context !== null &&
 		!current_component_context.m;
@@ -150,7 +171,7 @@ export function user_effect(fn) {
  * @returns {import('#client').Effect}
  */
 export function user_pre_effect(fn) {
-	validate_effect(current_effect, '$effect.pre');
+	validate_effect('$effect.pre');
 	return render_effect(fn);
 }
 

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

@@ -0,0 +1,41 @@
+import { flushSync } from 'svelte';
+import { test } from '../../test';
+
+export default test({
+	html: `
+		<button>toggle outer</button>
+		<button>toggle inner</button>
+		<button>reset</button>
+	`,
+
+	test({ assert, target }) {
+		const [outer, inner, reset] = target.querySelectorAll('button');
+
+		flushSync(() => outer?.click());
+		flushSync(() => inner?.click());
+
+		assert.htmlEqual(
+			target.innerHTML,
+			`
+				<button>toggle outer</button>
+				<button>toggle inner</button>
+				<button>reset</button>
+				<p>v is true</p>
+			`
+		);
+
+		flushSync(() => reset?.click());
+		flushSync(() => inner?.click());
+		flushSync(() => outer?.click());
+
+		assert.htmlEqual(
+			target.innerHTML,
+			`
+				<button>toggle outer</button>
+				<button>toggle inner</button>
+				<button>reset</button>
+				<p>v is true</p>
+			`
+		);
+	}
+});

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

@@ -0,0 +1,31 @@
+<script>
+	let value = $state(false);
+	const fn = () => {
+		if ($effect.active()) {
+			$effect(() => {
+				value = true;
+			});
+		}
+		return value;
+	};
+
+	let outer = $state(false);
+	let inner = $state(false);
+	let v = $derived(inner ? fn() : false);
+</script>
+
+<button onclick={() => outer = !outer}>
+	toggle outer
+</button>
+
+<button onclick={() => inner = !inner}>
+	toggle inner
+</button>
+
+<button onclick={() => outer = inner = value = false}>
+	reset
+</button>
+
+{#if outer && v}
+	<p>v is true</p>
+{/if}