merge main

.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

benchmarking/compare/index.js

@@ -2,7 +2,6 @@ import fs from 'node:fs';
 import path from 'node:path';
 import { execSync, fork } from 'node:child_process';
 import { fileURLToPath } from 'node:url';
-import { benchmarks } from '../benchmarks.js';
 
 // if (execSync('git status --porcelain').toString().trim()) {
 // 	console.error('Working directory is not clean');

benchmarking/compare/runner.js

@@ -1,7 +1,7 @@
-import { benchmarks } from '../benchmarks.js';
+import { reactivity_benchmarks } from '../benchmarks/reactivity/index.js';
 
 const results = [];
-for (const benchmark of benchmarks) {
+for (const benchmark of reactivity_benchmarks) {
 	const result = await benchmark();
 	console.error(result.benchmark);
 	results.push(result);

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/AssignmentExpression.js

@@ -69,14 +69,7 @@ function build_assignment(operator, left, right, context) {
 				is_non_coercive_operator(operator) &&
 				should_proxy(value, context.state.scope);
 
-			return b.call(
+			return b.call('$.set', left, value, needs_proxy && b.true);
-				// inside the constructor, we use `$.simple_set` rather than using `$.set`,
-				// that only assign the value and eventually call onchange since nothing is tracking the signal at this point
-				context.state.in_constructor ? '$.simple_set' : '$.set',
-				left,
-				value,
-				needs_proxy && b.true
-			);
 		}
 	}
 

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

@@ -305,7 +305,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)
 			)
 		];
 	}
@@ -320,7 +320,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 PROXY_ONCHANGE_SYMBOL = Symbol('proxy onchange');

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

@@ -114,11 +114,10 @@ export {
 	user_pre_effect
 } from './reactivity/effects.js';
 export {
-	mutable_state,
+	mutable_source,
 	mutate,
 	set,
-	simple_set,
+	source as state,
-	state,
 	update,
 	update_pre,
 	get_options

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

@@ -2,6 +2,7 @@
 import { DEV } from 'esm-env';
 import { UNINITIALIZED } from '../../constants.js';
 import { tracing_mode_flag } from '../flags/index.js';
+import { get, active_effect, active_reaction, set_active_reaction } from './runtime.js';
 import { component_context } from './context.js';
 import {
 	array_prototype,
@@ -15,7 +16,6 @@ import { check_ownership, widen_ownership } from './dev/ownership.js';
 import { get_stack } from './dev/tracing.js';
 import * as e from './errors.js';
 import { batch_onchange, set, source, state } from './reactivity/sources.js';
-import { active_effect, get } from './runtime.js';
 
 const array_methods = ['push', 'pop', 'shift', 'unshift', 'splice', 'reverse', 'sort'];
 
@@ -40,15 +40,17 @@ function clone_options(options) {
 		: undefined;
 }
 
+/** @type {ProxyMetadata | null} */
+var parent_metadata = null;
+
 /**
  * @template T
  * @param {T} value
  * @param {ValueOptions} [_options]
- * @param {ProxyMetadata | null} [parent]
  * @param {Source<T>} [prev] dev mode only
  * @returns {T}
  */
-export function proxy(value, _options, parent = null, prev) {
+export function proxy(value, _options, prev) {
 	// if non-proxyable, or is already a proxy, return `value`
 	if (typeof value !== 'object' || value === null) {
 		return value;
@@ -85,6 +87,31 @@ export function proxy(value, _options, 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
@@ -100,7 +127,7 @@ export function proxy(value, _options, parent = null, prev) {
 
 	if (DEV) {
 		metadata = {
-			parent,
+			parent: parent_metadata,
 			owners: null
 		};
 
@@ -112,7 +139,7 @@ export function proxy(value, _options, 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
@@ -138,10 +165,13 @@ export function proxy(value, _options, parent = null, prev) {
 			var s = sources.get(prop);
 
 			if (s === undefined) {
-				s = source(descriptor.value, clone_options(options), stack);
+				s = with_parent(() => source(descriptor.value, clone_options(options), stack));
 				sources.set(prop, s);
 			} else {
-				set(s, proxy(descriptor.value, options, metadata));
+				set(
+					s,
+					with_parent(() => proxy(descriptor.value, options))
+				);
 			}
 
 			return true;
@@ -152,7 +182,10 @@ export function proxy(value, _options, parent = null, prev) {
 
 			if (s === undefined) {
 				if (prop in target) {
-					sources.set(prop, source(UNINITIALIZED, clone_options(options), stack));
+					sources.set(
+						prop,
+						with_parent(() => source(UNINITIALIZED, clone_options(options), stack))
+					);
 				}
 			} else {
 				// When working with arrays, we need to also ensure we update the length when removing
@@ -217,7 +250,9 @@ export function proxy(value, _options, 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)) {
 				let opt = clone_options(options);
-				s = source(proxy(exists ? target[prop] : UNINITIALIZED, opt, metadata), opt, stack);
+				s = with_parent(() =>
+					source(proxy(exists ? target[prop] : UNINITIALIZED, opt), opt, stack)
+				);
 				sources.set(prop, s);
 			}
 
@@ -296,7 +331,7 @@ export function proxy(value, _options, parent = null, prev) {
 			) {
 				if (s === undefined) {
 					let opt = clone_options(options);
-					s = source(has ? proxy(target[prop], opt, metadata) : UNINITIALIZED, opt, stack);
+					s = with_parent(() => source(has ? proxy(target[prop], opt) : UNINITIALIZED, opt, stack));
 					sources.set(prop, s);
 				}
 
@@ -334,7 +369,7 @@ export function proxy(value, _options, 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, clone_options(options), stack);
+							other_s = with_parent(() => source(UNINITIALIZED, clone_options(options), stack));
 							sources.set(i + '', other_s);
 						}
 					}
@@ -347,8 +382,11 @@ export function proxy(value, _options, parent = null, prev) {
 				if (s === undefined) {
 					if (!has || get_descriptor(target, prop)?.writable) {
 						const opt = clone_options(options);
-						s = source(undefined, opt, stack);
+						s = with_parent(() => source(undefined, opt, stack));
-						set(s, proxy(value, opt, metadata));
+						set(
+							s,
+							with_parent(() => proxy(value, opt))
+						);
 						sources.set(prop, s);
 					}
 				} else {
@@ -358,7 +396,10 @@ export function proxy(value, _options, parent = null, prev) {
 					if (typeof s.v === 'object' && s.v !== null && STATE_SYMBOL in s.v) {
 						s.v[PROXY_ONCHANGE_SYMBOL](options?.onchange, true);
 					}
-					set(s, proxy(value, clone_options(options), metadata));
+					set(
+						s,
+						with_parent(() => proxy(value, clone_options(options)))
+					);
 				}
 			})();
 			if (DEV) {

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

@@ -11,8 +11,8 @@ import {
 	untrack,
 	increment_write_version,
 	update_effect,
-	derived_sources,
+	reaction_sources,
-	set_derived_sources,
+	set_reaction_sources,
 	check_dirtiness,
 	untracking,
 	is_destroying_effect
@@ -28,7 +28,8 @@ import {
 	MAYBE_DIRTY,
 	BLOCK_EFFECT,
 	ROOT_EFFECT,
-	PROXY_ONCHANGE_SYMBOL
+	PROXY_ONCHANGE_SYMBOL,
+	EFFECT_IS_UPDATING
 } from '../constants.js';
 import * as e from '../errors.js';
 import { legacy_mode_flag, tracing_mode_flag } from '../../flags/index.js';
@@ -79,6 +80,7 @@ export function batch_onchange(fn) {
  * @param {Error | null} [stack]
  * @returns {Source<V>}
  */
+// TODO rename this to `state` throughout the codebase
 export function source(v, o, stack) {
 	/** @type {Value} */
 	var signal = {
@@ -91,6 +93,14 @@ export function source(v, o, stack) {
 		o
 	};
 
+	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;
@@ -99,14 +109,7 @@ export function source(v, o, stack) {
 	return signal;
 }
 
-/**
+export { source as state };
- * @template V
- * @param {V} v
- * @param {ValueOptions} [o]
- */
-export function state(v, o) {
-	return push_derived_source(source(v, o));
-}
 
 /**
  * @param {Source} source
@@ -138,33 +141,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
@@ -191,43 +167,20 @@ 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
+		!reaction_sources?.includes(source)
-		// we allow the mutation.
-		(derived_sources === null || !derived_sources.includes(source))
 	) {
 		e.state_unsafe_mutation();
 	}
 
 	let new_value = should_proxy
 		? DEV
-			? proxy(value, source.o, null, source)
+			? proxy(value, source.o, source)
 			: proxy(value, source.o)
 		: value;
 
 	return internal_set(source, new_value);
 }
 
-/**
- * @template V
- * @param {Source<V>} source
- * @param {V} value
- * @param {boolean} [should_proxy]
- * @returns {V}
- */
-export function simple_set(source, value, should_proxy = false) {
-	let new_value = should_proxy
-		? DEV
-			? proxy(value, source.o, null, source)
-			: proxy(value, source.o)
-		: value;
-
-	source.v = new_value;
-
-	source.o?.onchange?.();
-
-	return new_value;
-}
-
 /**
  * @template V
  * @param {Source<V>} source

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
+ * When sources are created within a reaction, reading and writing
- * local mutations to these sources without the side-effect error being invoked unnecessarily.
+ * 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) {
+export function set_reaction_sources(sources) {
-	derived_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();
 		};
 	});
 

packages/svelte/tests/snapshot/samples/class-state-field-constructor-assignment/_expected/client/index.svelte.js

@@ -19,9 +19,9 @@ export default function Class_state_field_constructor_assignment($$anchor, $$pro
 
 		constructor() {
 			this.a = 1;
-			$.simple_set(this.#b, 2);
+			$.set(this.#b, 2);
 		}
 	}
 
 	$.pop();
-}
+}