feat: allow state created in deriveds/effects to be written/read locally without self-invalidation (#15553)

* move parent property onto Signal

* don't self-invalidate when updating a source create inside current reaction

* lazily create deep state with parent reaction

* no need to push_derived_source with mutable_state, as it never coexists with $.derived

* reduce indirection

* remove state_unsafe_local_read error

* changeset

* tests

* fix test

* inelegant fix

* remove arg

* tweak

* some progress

* more

* tidy up

* parent -> p

* tmp

* alternative approach

* tidy up

* reduce diff size

* more

* update comment

.changeset/dirty-pianos-sparkle.md

@@ -0,0 +1,5 @@
+---
+'svelte': minor
+---
+
+feat: allow state created in deriveds/effects to be written/read locally without self-invalidation

documentation/docs/98-reference/.generated/client-errors.md

@@ -122,12 +122,6 @@ Property descriptors defined on `$state` objects must contain `value` and always
 Cannot set prototype of `$state` object
 ```
 
-### state_unsafe_local_read
-
-```
-Reading state that was created inside the same derived is forbidden. Consider using `untrack` to read locally created state
-```
-
 ### state_unsafe_mutation
 
 ```

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

@@ -80,10 +80,6 @@ See the [migration guide](/docs/svelte/v5-migration-guide#Components-are-no-long
 
 > Cannot set prototype of `$state` object
 
-## state_unsafe_local_read
-
-> Reading state that was created inside the same derived is forbidden. Consider using `untrack` to read locally created state
-
 ## state_unsafe_mutation
 
 > Updating state inside a derived or a template expression is forbidden. If the value should not be reactive, declare it without `$state`

packages/svelte/src/compiler/phases/3-transform/client/transform-client.js

@@ -219,7 +219,10 @@ export function client_component(analysis, options) {
 	for (const [name, binding] of analysis.instance.scope.declarations) {
 		if (binding.kind === 'legacy_reactive') {
 			legacy_reactive_declarations.push(
-				b.const(name, b.call('$.mutable_state', undefined, analysis.immutable ? b.true : undefined))
+				b.const(
+					name,
+					b.call('$.mutable_source', undefined, analysis.immutable ? b.true : undefined)
+				)
 			);
 		}
 		if (binding.kind === 'store_sub') {

packages/svelte/src/compiler/phases/3-transform/client/visitors/VariableDeclaration.js

@@ -299,7 +299,7 @@ function create_state_declarators(declarator, { scope, analysis }, value) {
 		return [
 			b.declarator(
 				declarator.id,
-				b.call('$.mutable_state', value, analysis.immutable ? b.true : undefined)
+				b.call('$.mutable_source', value, analysis.immutable ? b.true : undefined)
 			)
 		];
 	}
@@ -314,7 +314,7 @@ function create_state_declarators(declarator, { scope, analysis }, value) {
 			return b.declarator(
 				path.node,
 				binding?.kind === 'state'
-					? b.call('$.mutable_state', value, analysis.immutable ? b.true : undefined)
+					? b.call('$.mutable_source', value, analysis.immutable ? b.true : undefined)
 					: value
 			);
 		})

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

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

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

@@ -307,21 +307,6 @@ export function state_prototype_fixed() {
 	}
 }
 
-/**
- * Reading state that was created inside the same derived is forbidden. Consider using `untrack` to read locally created state
- * @returns {never}
- */
-export function state_unsafe_local_read() {
-	if (DEV) {
-		const error = new Error(`state_unsafe_local_read\nReading state that was created inside the same derived is forbidden. Consider using \`untrack\` to read locally created state\nhttps://svelte.dev/e/state_unsafe_local_read`);
-
-		error.name = 'Svelte error';
-		throw error;
-	} else {
-		throw new Error(`https://svelte.dev/e/state_unsafe_local_read`);
-	}
-}
-
 /**
  * Updating state inside a derived or a template expression is forbidden. If the value should not be reactive, declare it without `$state`
  * @returns {never}

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

@@ -113,7 +113,14 @@ export {
 	user_effect,
 	user_pre_effect
 } from './reactivity/effects.js';
-export { mutable_state, mutate, set, state, update, update_pre } from './reactivity/sources.js';
+export {
+	mutable_source,
+	mutate,
+	set,
+	source as state,
+	update,
+	update_pre
+} from './reactivity/sources.js';
 export {
 	prop,
 	rest_props,

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

@@ -1,6 +1,6 @@
 /** @import { ProxyMetadata, Source } from '#client' */
 import { DEV } from 'esm-env';
-import { get, active_effect } from './runtime.js';
+import { get, active_effect, active_reaction, set_active_reaction } from './runtime.js';
 import { component_context } from './context.js';
 import {
 	array_prototype,
@@ -17,14 +17,16 @@ import * as e from './errors.js';
 import { get_stack } from './dev/tracing.js';
 import { tracing_mode_flag } from '../flags/index.js';
 
+/** @type {ProxyMetadata | null} */
+var parent_metadata = null;
+
 /**
  * @template T
  * @param {T} value
- * @param {ProxyMetadata | null} [parent]
  * @param {Source<T>} [prev] dev mode only
  * @returns {T}
  */
-export function proxy(value, parent = null, prev) {
+export function proxy(value, prev) {
 	// if non-proxyable, or is already a proxy, return `value`
 	if (typeof value !== 'object' || value === null || STATE_SYMBOL in value) {
 		return value;
@@ -42,6 +44,31 @@ export function proxy(value, parent = null, prev) {
 	var version = source(0);
 
 	var stack = DEV && tracing_mode_flag ? get_stack('CreatedAt') : null;
+	var reaction = active_reaction;
+
+	/**
+	 * @template T
+	 * @param {() => T} fn
+	 */
+	var with_parent = (fn) => {
+		var previous_reaction = active_reaction;
+		set_active_reaction(reaction);
+
+		/** @type {T} */
+		var result;
+
+		if (DEV) {
+			var previous_metadata = parent_metadata;
+			parent_metadata = metadata;
+			result = fn();
+			parent_metadata = previous_metadata;
+		} else {
+			result = fn();
+		}
+
+		set_active_reaction(previous_reaction);
+		return result;
+	};
 
 	if (is_proxied_array) {
 		// We need to create the length source eagerly to ensure that
@@ -54,7 +81,7 @@ export function proxy(value, parent = null, prev) {
 
 	if (DEV) {
 		metadata = {
-			parent,
+			parent: parent_metadata,
 			owners: null
 		};
 
@@ -66,7 +93,7 @@ export function proxy(value, parent = null, prev) {
 			metadata.owners = prev_owners ? new Set(prev_owners) : null;
 		} else {
 			metadata.owners =
-				parent === null
+				parent_metadata === null
 					? component_context !== null
 						? new Set([component_context.function])
 						: null
@@ -92,10 +119,13 @@ export function proxy(value, parent = null, prev) {
 			var s = sources.get(prop);
 
 			if (s === undefined) {
-				s = source(descriptor.value, stack);
+				s = with_parent(() => source(descriptor.value, stack));
 				sources.set(prop, s);
 			} else {
-				set(s, proxy(descriptor.value, metadata));
+				set(
+					s,
+					with_parent(() => proxy(descriptor.value))
+				);
 			}
 
 			return true;
@@ -106,7 +136,10 @@ export function proxy(value, parent = null, prev) {
 
 			if (s === undefined) {
 				if (prop in target) {
-					sources.set(prop, source(UNINITIALIZED, stack));
+					sources.set(
+						prop,
+						with_parent(() => source(UNINITIALIZED, stack))
+					);
 				}
 			} else {
 				// When working with arrays, we need to also ensure we update the length when removing
@@ -140,7 +173,7 @@ export function proxy(value, parent = null, prev) {
 
 			// create a source, but only if it's an own property and not a prototype property
 			if (s === undefined && (!exists || get_descriptor(target, prop)?.writable)) {
-				s = source(proxy(exists ? target[prop] : UNINITIALIZED, metadata), stack);
+				s = with_parent(() => source(proxy(exists ? target[prop] : UNINITIALIZED), stack));
 				sources.set(prop, s);
 			}
 
@@ -208,7 +241,7 @@ export function proxy(value, parent = null, prev) {
 				(active_effect !== null && (!has || get_descriptor(target, prop)?.writable))
 			) {
 				if (s === undefined) {
-					s = source(has ? proxy(target[prop], metadata) : UNINITIALIZED, stack);
+					s = with_parent(() => source(has ? proxy(target[prop]) : UNINITIALIZED, stack));
 					sources.set(prop, s);
 				}
 
@@ -235,7 +268,7 @@ export function proxy(value, parent = null, prev) {
 						// If the item exists in the original, we need to create a uninitialized source,
 						// else a later read of the property would result in a source being created with
 						// the value of the original item at that index.
-						other_s = source(UNINITIALIZED, stack);
+						other_s = with_parent(() => source(UNINITIALIZED, stack));
 						sources.set(i + '', other_s);
 					}
 				}
@@ -247,13 +280,19 @@ export function proxy(value, parent = null, prev) {
 			// object property before writing to that property.
 			if (s === undefined) {
 				if (!has || get_descriptor(target, prop)?.writable) {
-					s = source(undefined, stack);
-					set(s, proxy(value, metadata));
+					s = with_parent(() => source(undefined, stack));
+					set(
+						s,
+						with_parent(() => proxy(value))
+					);
 					sources.set(prop, s);
 				}
 			} else {
 				has = s.v !== UNINITIALIZED;
-				set(s, proxy(value, metadata));
+				set(
+					s,
+					with_parent(() => proxy(value))
+				);
 			}
 
 			if (DEV) {

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

@@ -11,8 +11,8 @@ import {
 	untrack,
 	increment_write_version,
 	update_effect,
-	derived_sources,
-	set_derived_sources,
+	reaction_sources,
+	set_reaction_sources,
 	check_dirtiness,
 	untracking,
 	is_destroying_effect
@@ -27,7 +27,8 @@ import {
 	UNOWNED,
 	MAYBE_DIRTY,
 	BLOCK_EFFECT,
-	ROOT_EFFECT
+	ROOT_EFFECT,
+	EFFECT_IS_UPDATING
 } from '../constants.js';
 import * as e from '../errors.js';
 import { legacy_mode_flag, tracing_mode_flag } from '../../flags/index.js';
@@ -51,6 +52,7 @@ export function set_inspect_effects(v) {
  * @param {Error | null} [stack]
  * @returns {Source<V>}
  */
+// TODO rename this to `state` throughout the codebase
 export function source(v, stack) {
 	/** @type {Value} */
 	var signal = {
@@ -62,6 +64,14 @@ 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;
@@ -70,14 +80,6 @@ export function source(v, stack) {
 	return signal;
 }
 
-/**
- * @template V
- * @param {V} v
- */
-export function state(v) {
-	return push_derived_source(source(v));
-}
-
 /**
  * @template V
  * @param {V} initial_value
@@ -100,33 +102,6 @@ export function mutable_source(initial_value, immutable = false) {
 	return s;
 }
 
-/**
- * @template V
- * @param {V} v
- * @param {boolean} [immutable]
- * @returns {Source<V>}
- */
-export function mutable_state(v, immutable = false) {
-	return push_derived_source(mutable_source(v, immutable));
-}
-
-/**
- * @template V
- * @param {Source<V>} source
- */
-/*#__NO_SIDE_EFFECTS__*/
-function push_derived_source(source) {
-	if (active_reaction !== null && !untracking && (active_reaction.f & DERIVED) !== 0) {
-		if (derived_sources === null) {
-			set_derived_sources([source]);
-		} else {
-			derived_sources.push(source);
-		}
-	}
-
-	return source;
-}
-
 /**
  * @template V
  * @param {Value<V>} source
@@ -153,14 +128,12 @@ export function set(source, value, should_proxy = false) {
 		!untracking &&
 		is_runes() &&
 		(active_reaction.f & (DERIVED | BLOCK_EFFECT)) !== 0 &&
-		// If the source was created locally within the current derived, then
-		// we allow the mutation.
-		(derived_sources === null || !derived_sources.includes(source))
+		!reaction_sources?.includes(source)
 	) {
 		e.state_unsafe_mutation();
 	}
 
-	let new_value = should_proxy ? proxy(value, null, source) : value;
+	let new_value = should_proxy ? proxy(value, source) : value;
 
 	return internal_set(source, new_value);
 }

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

@@ -22,7 +22,8 @@ import {
 	ROOT_EFFECT,
 	LEGACY_DERIVED_PROP,
 	DISCONNECTED,
-	BOUNDARY_EFFECT
+	BOUNDARY_EFFECT,
+	EFFECT_IS_UPDATING
 } from './constants.js';
 import { flush_tasks } from './dom/task.js';
 import { internal_set, old_values } from './reactivity/sources.js';
@@ -87,17 +88,17 @@ export function set_active_effect(effect) {
 }
 
 /**
- * When sources are created within a derived, we record them so that we can safely allow
- * local mutations to these sources without the side-effect error being invoked unnecessarily.
+ * When sources are created within a reaction, reading and writing
+ * them should not cause a re-run
  * @type {null | Source[]}
  */
-export let derived_sources = null;
+export let reaction_sources = null;
 
 /**
  * @param {Source[] | null} sources
  */
-export function set_derived_sources(sources) {
-	derived_sources = sources;
+export function set_reaction_sources(sources) {
+	reaction_sources = sources;
 }
 
 /**
@@ -367,6 +368,9 @@ function schedule_possible_effect_self_invalidation(signal, effect, root = true)
 
 	for (var i = 0; i < reactions.length; i++) {
 		var reaction = reactions[i];
+
+		if (reaction_sources?.includes(signal)) continue;
+
 		if ((reaction.f & DERIVED) !== 0) {
 			schedule_possible_effect_self_invalidation(/** @type {Derived} */ (reaction), effect, false);
 		} else if (effect === reaction) {
@@ -391,9 +395,10 @@ export function update_reaction(reaction) {
 	var previous_untracked_writes = untracked_writes;
 	var previous_reaction = active_reaction;
 	var previous_skip_reaction = skip_reaction;
-	var prev_derived_sources = derived_sources;
+	var previous_reaction_sources = reaction_sources;
 	var previous_component_context = component_context;
 	var previous_untracking = untracking;
+
 	var flags = reaction.f;
 
 	new_deps = /** @type {null | Value[]} */ (null);
@@ -403,11 +408,13 @@ export function update_reaction(reaction) {
 		(flags & UNOWNED) !== 0 && (untracking || !is_updating_effect || active_reaction === null);
 	active_reaction = (flags & (BRANCH_EFFECT | ROOT_EFFECT)) === 0 ? reaction : null;
 
-	derived_sources = null;
+	reaction_sources = null;
 	set_component_context(reaction.ctx);
 	untracking = false;
 	read_version++;
 
+	reaction.f |= EFFECT_IS_UPDATING;
+
 	try {
 		var result = /** @type {Function} */ (0, reaction.fn)();
 		var deps = reaction.deps;
@@ -477,9 +484,11 @@ export function update_reaction(reaction) {
 		untracked_writes = previous_untracked_writes;
 		active_reaction = previous_reaction;
 		skip_reaction = previous_skip_reaction;
-		derived_sources = prev_derived_sources;
+		reaction_sources = previous_reaction_sources;
 		set_component_context(previous_component_context);
 		untracking = previous_untracking;
+
+		reaction.f ^= EFFECT_IS_UPDATING;
 	}
 }
 
@@ -866,9 +875,6 @@ export function get(signal) {
 
 	// Register the dependency on the current reaction signal.
 	if (active_reaction !== null && !untracking) {
-		if (derived_sources !== null && derived_sources.includes(signal)) {
-			e.state_unsafe_local_read();
-		}
 		var deps = active_reaction.deps;
 		if (signal.rv < read_version) {
 			signal.rv = read_version;

packages/svelte/tests/signals/test.ts

@@ -8,7 +8,12 @@ import {
 	render_effect,
 	user_effect
 } from '../../src/internal/client/reactivity/effects';
-import { state, set, update, update_pre } from '../../src/internal/client/reactivity/sources';
+import {
+	source as 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';
@@ -487,6 +492,26 @@ describe('signals', () => {
 		};
 	});
 
+	test('schedules rerun when updating deeply nested value', (runes) => {
+		if (!runes) return () => {};
+
+		const value = proxy({ a: { b: { c: 0 } } });
+		user_effect(() => {
+			value.a.b.c += 1;
+		});
+
+		return () => {
+			let errored = false;
+			try {
+				flushSync();
+			} catch (e: any) {
+				assert.include(e.message, 'effect_update_depth_exceeded');
+				errored = true;
+			}
+			assert.equal(errored, true);
+		};
+	});
+
 	test('schedules rerun when writing to signal before reading it', (runes) => {
 		if (!runes) return () => {};
 
@@ -958,14 +983,30 @@ describe('signals', () => {
 		};
 	});
 
-	test('deriveds cannot depend on state they own', () => {
+	test('deriveds do not depend on state they own', () => {
 		return () => {
+			let s;
+
 			const d = derived(() => {
-				const s = state(0);
+				s = state(0);
 				return $.get(s);
 			});
 
-			assert.throws(() => $.get(d), 'state_unsafe_local_read');
+			assert.equal($.get(d), 0);
+
+			set(s!, 1);
+			assert.equal($.get(d), 0);
+		};
+	});
+
+	test('effects do not depend on state they own', () => {
+		user_effect(() => {
+			const value = state(0);
+			set(value, $.get(value) + 1);
+		});
+
+		return () => {
+			flushSync();
 		};
 	});