From ec1b7d8fe5911136a65755cc74d1e669545c5577 Mon Sep 17 00:00:00 2001 From: Rich Harris Date: Wed, 19 Mar 2025 21:08:04 -0400 Subject: [PATCH 01/22] move parent property onto Signal --- packages/svelte/src/internal/client/reactivity/sources.js | 1 + packages/svelte/src/internal/client/reactivity/types.d.ts | 4 ++-- 2 files changed, 3 insertions(+), 2 deletions(-) diff --git a/packages/svelte/src/internal/client/reactivity/sources.js b/packages/svelte/src/internal/client/reactivity/sources.js index 92508945c96c..3ae3091613e7 100644 --- a/packages/svelte/src/internal/client/reactivity/sources.js +++ b/packages/svelte/src/internal/client/reactivity/sources.js @@ -56,6 +56,7 @@ export function source(v, stack) { var signal = { f: 0, // TODO ideally we could skip this altogether, but it causes type errors v, + parent: active_reaction, reactions: null, equals, rv: 0, diff --git a/packages/svelte/src/internal/client/reactivity/types.d.ts b/packages/svelte/src/internal/client/reactivity/types.d.ts index 5ef0097649a4..7a44017229d3 100644 --- a/packages/svelte/src/internal/client/reactivity/types.d.ts +++ b/packages/svelte/src/internal/client/reactivity/types.d.ts @@ -5,6 +5,8 @@ export interface Signal { f: number; /** Write version */ wv: number; + /** Parent effect or derived */ + parent: Reaction | null; } export interface Value extends Signal { @@ -38,8 +40,6 @@ export interface Derived extends Value, Reaction { fn: () => V; /** Effects created inside this signal */ effects: null | Effect[]; - /** Parent effect or derived */ - parent: Effect | Derived | null; } export interface Effect extends Reaction { From 14ca327e4b9ea90f3cdeaa38ca18dd965d786cc9 Mon Sep 17 00:00:00 2001 From: Rich Harris Date: Wed, 19 Mar 2025 22:31:43 -0400 Subject: [PATCH 02/22] don't self-invalidate when updating a source create inside current reaction --- packages/svelte/src/internal/client/reactivity/sources.js | 4 +--- packages/svelte/src/internal/client/runtime.js | 2 ++ 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/packages/svelte/src/internal/client/reactivity/sources.js b/packages/svelte/src/internal/client/reactivity/sources.js index 3ae3091613e7..93bb77126acd 100644 --- a/packages/svelte/src/internal/client/reactivity/sources.js +++ b/packages/svelte/src/internal/client/reactivity/sources.js @@ -154,9 +154,7 @@ 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)) + active_reaction !== source.parent ) { e.state_unsafe_mutation(); } diff --git a/packages/svelte/src/internal/client/runtime.js b/packages/svelte/src/internal/client/runtime.js index 0a65c6e45a13..c59e89fcedbc 100644 --- a/packages/svelte/src/internal/client/runtime.js +++ b/packages/svelte/src/internal/client/runtime.js @@ -367,6 +367,8 @@ function schedule_possible_effect_self_invalidation(signal, effect, root = true) for (var i = 0; i < reactions.length; i++) { var reaction = reactions[i]; + if (signal.parent === reaction) continue; + if ((reaction.f & DERIVED) !== 0) { schedule_possible_effect_self_invalidation(/** @type {Derived} */ (reaction), effect, false); } else if (effect === reaction) { From 6f4326552f6dfc55f8453aa3c08bbfbe28123c26 Mon Sep 17 00:00:00 2001 From: Rich Harris Date: Wed, 19 Mar 2025 22:44:27 -0400 Subject: [PATCH 03/22] lazily create deep state with parent reaction --- packages/svelte/src/internal/client/proxy.js | 24 ++++++++++++++------ 1 file changed, 17 insertions(+), 7 deletions(-) diff --git a/packages/svelte/src/internal/client/proxy.js b/packages/svelte/src/internal/client/proxy.js index 29828a7c995d..9f93f61778ab 100644 --- a/packages/svelte/src/internal/client/proxy.js +++ b/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, @@ -42,6 +42,16 @@ 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; + + /** @param {any} value */ + var child = (value) => { + var previous_reaction = active_reaction; + set_active_reaction(reaction); + var s = source(value, stack); + set_active_reaction(previous_reaction); + return s; + }; if (is_proxied_array) { // We need to create the length source eagerly to ensure that @@ -92,7 +102,7 @@ export function proxy(value, parent = null, prev) { var s = sources.get(prop); if (s === undefined) { - s = source(descriptor.value, stack); + s = child(descriptor.value); sources.set(prop, s); } else { set(s, proxy(descriptor.value, metadata)); @@ -106,7 +116,7 @@ export function proxy(value, parent = null, prev) { if (s === undefined) { if (prop in target) { - sources.set(prop, source(UNINITIALIZED, stack)); + sources.set(prop, child(UNINITIALIZED)); } } else { // When working with arrays, we need to also ensure we update the length when removing @@ -140,7 +150,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 = child(proxy(exists ? target[prop] : UNINITIALIZED, metadata)); sources.set(prop, s); } @@ -208,7 +218,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 = child(has ? proxy(target[prop], metadata) : UNINITIALIZED); sources.set(prop, s); } @@ -235,7 +245,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 = child(UNINITIALIZED); sources.set(i + '', other_s); } } @@ -247,7 +257,7 @@ 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); + s = child(undefined); set(s, proxy(value, metadata)); sources.set(prop, s); } From 8197537c9ac7b950b75d27b103cc4766ea31f61e Mon Sep 17 00:00:00 2001 From: Rich Harris Date: Wed, 19 Mar 2025 22:53:22 -0400 Subject: [PATCH 04/22] no need to push_derived_source with mutable_state, as it never coexists with $.derived --- .../phases/3-transform/client/transform-client.js | 5 ++++- .../3-transform/client/visitors/VariableDeclaration.js | 4 ++-- packages/svelte/src/internal/client/index.js | 2 +- .../svelte/src/internal/client/reactivity/sources.js | 10 ---------- 4 files changed, 7 insertions(+), 14 deletions(-) diff --git a/packages/svelte/src/compiler/phases/3-transform/client/transform-client.js b/packages/svelte/src/compiler/phases/3-transform/client/transform-client.js index ac8263b91669..0bdfbae746d0 100644 --- a/packages/svelte/src/compiler/phases/3-transform/client/transform-client.js +++ b/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') { diff --git a/packages/svelte/src/compiler/phases/3-transform/client/visitors/VariableDeclaration.js b/packages/svelte/src/compiler/phases/3-transform/client/visitors/VariableDeclaration.js index baffc5dec374..3a914fb56099 100644 --- a/packages/svelte/src/compiler/phases/3-transform/client/visitors/VariableDeclaration.js +++ b/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 ); }) diff --git a/packages/svelte/src/internal/client/index.js b/packages/svelte/src/internal/client/index.js index 31da00dbb448..6b74a16ec893 100644 --- a/packages/svelte/src/internal/client/index.js +++ b/packages/svelte/src/internal/client/index.js @@ -113,7 +113,7 @@ 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, state, update, update_pre } from './reactivity/sources.js'; export { prop, rest_props, diff --git a/packages/svelte/src/internal/client/reactivity/sources.js b/packages/svelte/src/internal/client/reactivity/sources.js index 93bb77126acd..e99e83b343d4 100644 --- a/packages/svelte/src/internal/client/reactivity/sources.js +++ b/packages/svelte/src/internal/client/reactivity/sources.js @@ -101,16 +101,6 @@ export function mutable_source(initial_value, immutable = false) { return s; } -/** - * @template V - * @param {V} v - * @param {boolean} [immutable] - * @returns {Source} - */ -export function mutable_state(v, immutable = false) { - return push_derived_source(mutable_source(v, immutable)); -} - /** * @template V * @param {Source} source From 550a06421230880a350f2c3808166e5a88818bde Mon Sep 17 00:00:00 2001 From: Rich Harris Date: Wed, 19 Mar 2025 22:55:08 -0400 Subject: [PATCH 05/22] reduce indirection --- .../src/internal/client/reactivity/sources.js | 30 ++++++++----------- 1 file changed, 12 insertions(+), 18 deletions(-) diff --git a/packages/svelte/src/internal/client/reactivity/sources.js b/packages/svelte/src/internal/client/reactivity/sources.js index e99e83b343d4..9a3eed616429 100644 --- a/packages/svelte/src/internal/client/reactivity/sources.js +++ b/packages/svelte/src/internal/client/reactivity/sources.js @@ -76,7 +76,18 @@ export function source(v, stack) { * @param {V} v */ export function state(v) { - return push_derived_source(source(v)); + var s = source(v); + + // TODO maybe we make this dev-only? + if (active_reaction !== null && !untracking && (active_reaction.f & DERIVED) !== 0) { + if (derived_sources === null) { + set_derived_sources([s]); + } else { + derived_sources.push(s); + } + } + + return s; } /** @@ -101,23 +112,6 @@ export function mutable_source(initial_value, immutable = false) { return s; } -/** - * @template V - * @param {Source} 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} source From e407e8b38e62a9254559b31c0fff3260eefe5acf Mon Sep 17 00:00:00 2001 From: Rich Harris Date: Wed, 19 Mar 2025 23:05:32 -0400 Subject: [PATCH 06/22] remove state_unsafe_local_read error --- .../98-reference/.generated/client-errors.md | 6 ----- .../svelte/messages/client-errors/errors.md | 4 ---- packages/svelte/src/internal/client/errors.js | 15 ------------- packages/svelte/src/internal/client/index.js | 9 +++++++- .../src/internal/client/reactivity/sources.js | 22 +------------------ .../svelte/src/internal/client/runtime.js | 20 ----------------- packages/svelte/tests/signals/test.ts | 18 +++++---------- 7 files changed, 15 insertions(+), 79 deletions(-) diff --git a/documentation/docs/98-reference/.generated/client-errors.md b/documentation/docs/98-reference/.generated/client-errors.md index 0beb3cb9a96c..62d9c3302a3c 100644 --- a/documentation/docs/98-reference/.generated/client-errors.md +++ b/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 ``` diff --git a/packages/svelte/messages/client-errors/errors.md b/packages/svelte/messages/client-errors/errors.md index ab4d1519c18c..bc8ec3625605 100644 --- a/packages/svelte/messages/client-errors/errors.md +++ b/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` diff --git a/packages/svelte/src/internal/client/errors.js b/packages/svelte/src/internal/client/errors.js index 682816e1d64b..8a5b5033a78c 100644 --- a/packages/svelte/src/internal/client/errors.js +++ b/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} diff --git a/packages/svelte/src/internal/client/index.js b/packages/svelte/src/internal/client/index.js index 6b74a16ec893..723ff57678b0 100644 --- a/packages/svelte/src/internal/client/index.js +++ b/packages/svelte/src/internal/client/index.js @@ -113,7 +113,14 @@ export { user_effect, user_pre_effect } from './reactivity/effects.js'; -export { mutable_source, 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, diff --git a/packages/svelte/src/internal/client/reactivity/sources.js b/packages/svelte/src/internal/client/reactivity/sources.js index 9a3eed616429..b6181a81dc7d 100644 --- a/packages/svelte/src/internal/client/reactivity/sources.js +++ b/packages/svelte/src/internal/client/reactivity/sources.js @@ -11,8 +11,6 @@ import { untrack, increment_write_version, update_effect, - derived_sources, - set_derived_sources, check_dirtiness, untracking, is_destroying_effect @@ -51,6 +49,7 @@ export function set_inspect_effects(v) { * @param {Error | null} [stack] * @returns {Source} */ +// TODO rename this to `state` throughout the codebase export function source(v, stack) { /** @type {Value} */ var signal = { @@ -71,25 +70,6 @@ export function source(v, stack) { return signal; } -/** - * @template V - * @param {V} v - */ -export function state(v) { - var s = source(v); - - // TODO maybe we make this dev-only? - if (active_reaction !== null && !untracking && (active_reaction.f & DERIVED) !== 0) { - if (derived_sources === null) { - set_derived_sources([s]); - } else { - derived_sources.push(s); - } - } - - return s; -} - /** * @template V * @param {V} initial_value diff --git a/packages/svelte/src/internal/client/runtime.js b/packages/svelte/src/internal/client/runtime.js index c59e89fcedbc..ccce3f6bd9ec 100644 --- a/packages/svelte/src/internal/client/runtime.js +++ b/packages/svelte/src/internal/client/runtime.js @@ -86,20 +86,6 @@ export function set_active_effect(effect) { 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. - * @type {null | Source[]} - */ -export let derived_sources = null; - -/** - * @param {Source[] | null} sources - */ -export function set_derived_sources(sources) { - derived_sources = sources; -} - /** * 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 @@ -393,7 +379,6 @@ 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_component_context = component_context; var previous_untracking = untracking; var flags = reaction.f; @@ -405,7 +390,6 @@ 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; set_component_context(reaction.ctx); untracking = false; read_version++; @@ -479,7 +463,6 @@ export function update_reaction(reaction) { untracked_writes = previous_untracked_writes; active_reaction = previous_reaction; skip_reaction = previous_skip_reaction; - derived_sources = prev_derived_sources; set_component_context(previous_component_context); untracking = previous_untracking; } @@ -868,9 +851,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; diff --git a/packages/svelte/tests/signals/test.ts b/packages/svelte/tests/signals/test.ts index ef4cf16d3b6c..4f6691915ee3 100644 --- a/packages/svelte/tests/signals/test.ts +++ b/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'; @@ -958,17 +963,6 @@ describe('signals', () => { }; }); - test('deriveds cannot depend on state they own', () => { - return () => { - const d = derived(() => { - const s = state(0); - return $.get(s); - }); - - assert.throws(() => $.get(d), 'state_unsafe_local_read'); - }; - }); - test('proxy version state does not trigger self-dependency guard', () => { return () => { const s = proxy({ a: { b: 1 } }); From 9c6e4e1d567c8c575dd95ac5b38f01567e266fa9 Mon Sep 17 00:00:00 2001 From: Rich Harris Date: Wed, 19 Mar 2025 23:06:15 -0400 Subject: [PATCH 07/22] changeset --- .changeset/dirty-pianos-sparkle.md | 5 +++++ 1 file changed, 5 insertions(+) create mode 100644 .changeset/dirty-pianos-sparkle.md diff --git a/.changeset/dirty-pianos-sparkle.md b/.changeset/dirty-pianos-sparkle.md new file mode 100644 index 000000000000..b3e4dd1d8c1b --- /dev/null +++ b/.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 From f4969af31a57281ebaee0c199b7cd90d0d0bd2bf Mon Sep 17 00:00:00 2001 From: Rich Harris Date: Thu, 20 Mar 2025 09:34:07 -0400 Subject: [PATCH 08/22] tests --- packages/svelte/tests/signals/test.ts | 30 +++++++++++++++++++++++++++ 1 file changed, 30 insertions(+) diff --git a/packages/svelte/tests/signals/test.ts b/packages/svelte/tests/signals/test.ts index 4f6691915ee3..f227d68f03af 100644 --- a/packages/svelte/tests/signals/test.ts +++ b/packages/svelte/tests/signals/test.ts @@ -963,6 +963,36 @@ describe('signals', () => { }; }); + test('deriveds do not depend on state they own', () => { + return () => { + let s; + + const d = derived(() => { + s = state(0); + return $.get(s); + }); + + assert.equal($.get(d), 0); + + set(s!, 1); + assert.equal($.get(d), 0); + }; + }); + + test('effects do not depend on state they own', () => { + return () => { + const destroy = effect_root(() => { + user_effect(() => { + const s = state(0); + set(s, $.get(s) + 1); + }); + }); + + assert.ok(true); + destroy(); + }; + }); + test('proxy version state does not trigger self-dependency guard', () => { return () => { const s = proxy({ a: { b: 1 } }); From c2bc9bbb90546f0dd110bf774d39fd76e599cf33 Mon Sep 17 00:00:00 2001 From: Rich Harris Date: Thu, 20 Mar 2025 09:42:48 -0400 Subject: [PATCH 09/22] fix test --- packages/svelte/tests/signals/test.ts | 15 ++++++--------- 1 file changed, 6 insertions(+), 9 deletions(-) diff --git a/packages/svelte/tests/signals/test.ts b/packages/svelte/tests/signals/test.ts index f227d68f03af..c931631d6ce9 100644 --- a/packages/svelte/tests/signals/test.ts +++ b/packages/svelte/tests/signals/test.ts @@ -980,16 +980,13 @@ describe('signals', () => { }); test('effects do not depend on state they own', () => { - return () => { - const destroy = effect_root(() => { - user_effect(() => { - const s = state(0); - set(s, $.get(s) + 1); - }); - }); + user_effect(() => { + const value = state(0); + set(value, $.get(value) + 1); + }); - assert.ok(true); - destroy(); + return () => { + flushSync(); }; }); From 31cfc1cf44be85d15fa255babc37ccb168264f61 Mon Sep 17 00:00:00 2001 From: Rich Harris Date: Thu, 20 Mar 2025 10:53:54 -0400 Subject: [PATCH 10/22] inelegant fix --- packages/svelte/src/internal/client/proxy.js | 29 +++++++++++++------- packages/svelte/tests/signals/test.ts | 20 ++++++++++++++ 2 files changed, 39 insertions(+), 10 deletions(-) diff --git a/packages/svelte/src/internal/client/proxy.js b/packages/svelte/src/internal/client/proxy.js index 9f93f61778ab..9778909960b5 100644 --- a/packages/svelte/src/internal/client/proxy.js +++ b/packages/svelte/src/internal/client/proxy.js @@ -45,7 +45,7 @@ export function proxy(value, parent = null, prev) { var reaction = active_reaction; /** @param {any} value */ - var child = (value) => { + var child_source = (value) => { var previous_reaction = active_reaction; set_active_reaction(reaction); var s = source(value, stack); @@ -53,6 +53,15 @@ export function proxy(value, parent = null, prev) { return s; }; + /** @param {any} value */ + var child_proxy = (value) => { + var previous_reaction = active_reaction; + set_active_reaction(reaction); + var p = proxy(value, metadata); + set_active_reaction(previous_reaction); + return p; + }; + if (is_proxied_array) { // We need to create the length source eagerly to ensure that // mutations to the array are properly synced with our proxy @@ -102,10 +111,10 @@ export function proxy(value, parent = null, prev) { var s = sources.get(prop); if (s === undefined) { - s = child(descriptor.value); + s = child_source(descriptor.value); sources.set(prop, s); } else { - set(s, proxy(descriptor.value, metadata)); + set(s, child_proxy(descriptor.value)); } return true; @@ -116,7 +125,7 @@ export function proxy(value, parent = null, prev) { if (s === undefined) { if (prop in target) { - sources.set(prop, child(UNINITIALIZED)); + sources.set(prop, child_source(UNINITIALIZED)); } } else { // When working with arrays, we need to also ensure we update the length when removing @@ -150,7 +159,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 = child(proxy(exists ? target[prop] : UNINITIALIZED, metadata)); + s = child_source(child_proxy(exists ? target[prop] : UNINITIALIZED)); sources.set(prop, s); } @@ -218,7 +227,7 @@ export function proxy(value, parent = null, prev) { (active_effect !== null && (!has || get_descriptor(target, prop)?.writable)) ) { if (s === undefined) { - s = child(has ? proxy(target[prop], metadata) : UNINITIALIZED); + s = child_source(has ? child_proxy(target[prop]) : UNINITIALIZED); sources.set(prop, s); } @@ -245,7 +254,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 = child(UNINITIALIZED); + other_s = child_source(UNINITIALIZED); sources.set(i + '', other_s); } } @@ -257,13 +266,13 @@ 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 = child(undefined); - set(s, proxy(value, metadata)); + s = child_source(undefined); + set(s, child_proxy(value)); sources.set(prop, s); } } else { has = s.v !== UNINITIALIZED; - set(s, proxy(value, metadata)); + set(s, child_proxy(value)); } if (DEV) { diff --git a/packages/svelte/tests/signals/test.ts b/packages/svelte/tests/signals/test.ts index c931631d6ce9..72f99c90e55b 100644 --- a/packages/svelte/tests/signals/test.ts +++ b/packages/svelte/tests/signals/test.ts @@ -492,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 () => {}; From d4bd04fef5655edd57bccfa16932f45b34c40122 Mon Sep 17 00:00:00 2001 From: Rich Harris Date: Thu, 20 Mar 2025 10:58:43 -0400 Subject: [PATCH 11/22] remove arg --- packages/svelte/src/internal/client/proxy.js | 15 ++++++++++----- .../src/internal/client/reactivity/sources.js | 2 +- 2 files changed, 11 insertions(+), 6 deletions(-) diff --git a/packages/svelte/src/internal/client/proxy.js b/packages/svelte/src/internal/client/proxy.js index 9778909960b5..3a33b6b82ad1 100644 --- a/packages/svelte/src/internal/client/proxy.js +++ b/packages/svelte/src/internal/client/proxy.js @@ -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} [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; @@ -57,8 +59,11 @@ export function proxy(value, parent = null, prev) { var child_proxy = (value) => { var previous_reaction = active_reaction; set_active_reaction(reaction); - var p = proxy(value, metadata); + var previous_metadata = parent_metadata; + parent_metadata = metadata; + var p = proxy(value); set_active_reaction(previous_reaction); + parent_metadata = previous_metadata; return p; }; @@ -73,7 +78,7 @@ export function proxy(value, parent = null, prev) { if (DEV) { metadata = { - parent, + parent: parent_metadata, owners: null }; @@ -85,7 +90,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 diff --git a/packages/svelte/src/internal/client/reactivity/sources.js b/packages/svelte/src/internal/client/reactivity/sources.js index b6181a81dc7d..552102aeb28d 100644 --- a/packages/svelte/src/internal/client/reactivity/sources.js +++ b/packages/svelte/src/internal/client/reactivity/sources.js @@ -123,7 +123,7 @@ export function set(source, value, should_proxy = false) { 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); } From 3a7ec659a6f11ead46854cd8c92da559ef3a6bcb Mon Sep 17 00:00:00 2001 From: Rich Harris Date: Thu, 20 Mar 2025 11:46:12 -0400 Subject: [PATCH 12/22] tweak --- packages/svelte/src/internal/client/proxy.js | 25 +++++++++++++------- 1 file changed, 17 insertions(+), 8 deletions(-) diff --git a/packages/svelte/src/internal/client/proxy.js b/packages/svelte/src/internal/client/proxy.js index 3a33b6b82ad1..dcaf9e25e134 100644 --- a/packages/svelte/src/internal/client/proxy.js +++ b/packages/svelte/src/internal/client/proxy.js @@ -55,13 +55,13 @@ export function proxy(value, prev) { return s; }; - /** @param {any} value */ - var child_proxy = (value) => { + /** @param {() => void} fn */ + var with_parent = (fn) => { var previous_reaction = active_reaction; set_active_reaction(reaction); var previous_metadata = parent_metadata; parent_metadata = metadata; - var p = proxy(value); + var p = fn(); set_active_reaction(previous_reaction); parent_metadata = previous_metadata; return p; @@ -119,7 +119,10 @@ export function proxy(value, prev) { s = child_source(descriptor.value); sources.set(prop, s); } else { - set(s, child_proxy(descriptor.value)); + set( + s, + with_parent(() => proxy(descriptor.value)) + ); } return true; @@ -164,7 +167,7 @@ export function proxy(value, 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 = child_source(child_proxy(exists ? target[prop] : UNINITIALIZED)); + s = child_source(with_parent(() => proxy(exists ? target[prop] : UNINITIALIZED))); sources.set(prop, s); } @@ -232,7 +235,7 @@ export function proxy(value, prev) { (active_effect !== null && (!has || get_descriptor(target, prop)?.writable)) ) { if (s === undefined) { - s = child_source(has ? child_proxy(target[prop]) : UNINITIALIZED); + s = child_source(has ? with_parent(() => proxy(target[prop])) : UNINITIALIZED); sources.set(prop, s); } @@ -272,12 +275,18 @@ export function proxy(value, prev) { if (s === undefined) { if (!has || get_descriptor(target, prop)?.writable) { s = child_source(undefined); - set(s, child_proxy(value)); + set( + s, + with_parent(() => proxy(value)) + ); sources.set(prop, s); } } else { has = s.v !== UNINITIALIZED; - set(s, child_proxy(value)); + set( + s, + with_parent(() => proxy(value)) + ); } if (DEV) { From 8c61af68fa3eb814e306ac724481976a53060165 Mon Sep 17 00:00:00 2001 From: Rich Harris Date: Thu, 20 Mar 2025 12:00:30 -0400 Subject: [PATCH 13/22] some progress --- packages/svelte/src/internal/client/proxy.js | 30 ++++++++++++++------ 1 file changed, 21 insertions(+), 9 deletions(-) diff --git a/packages/svelte/src/internal/client/proxy.js b/packages/svelte/src/internal/client/proxy.js index dcaf9e25e134..4c14d33bbb05 100644 --- a/packages/svelte/src/internal/client/proxy.js +++ b/packages/svelte/src/internal/client/proxy.js @@ -46,8 +46,11 @@ export function proxy(value, prev) { var stack = DEV && tracing_mode_flag ? get_stack('CreatedAt') : null; var reaction = active_reaction; - /** @param {any} value */ - var child_source = (value) => { + /** + * @param {any} value + * @param {Error | null | undefined} stack + */ + var child_source = (value, stack) => { var previous_reaction = active_reaction; set_active_reaction(reaction); var s = source(value, stack); @@ -55,7 +58,10 @@ export function proxy(value, prev) { return s; }; - /** @param {() => void} fn */ + /** + * @template T + * @param {() => T} fn + */ var with_parent = (fn) => { var previous_reaction = active_reaction; set_active_reaction(reaction); @@ -116,7 +122,7 @@ export function proxy(value, prev) { var s = sources.get(prop); if (s === undefined) { - s = child_source(descriptor.value); + s = with_parent(() => source(descriptor.value, stack)); sources.set(prop, s); } else { set( @@ -133,7 +139,10 @@ export function proxy(value, prev) { if (s === undefined) { if (prop in target) { - sources.set(prop, child_source(UNINITIALIZED)); + sources.set( + prop, + with_parent(() => source(UNINITIALIZED, stack)) + ); } } else { // When working with arrays, we need to also ensure we update the length when removing @@ -167,7 +176,10 @@ export function proxy(value, 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 = child_source(with_parent(() => proxy(exists ? target[prop] : UNINITIALIZED))); + s = child_source( + with_parent(() => proxy(exists ? target[prop] : UNINITIALIZED)), + stack + ); sources.set(prop, s); } @@ -235,7 +247,7 @@ export function proxy(value, prev) { (active_effect !== null && (!has || get_descriptor(target, prop)?.writable)) ) { if (s === undefined) { - s = child_source(has ? with_parent(() => proxy(target[prop])) : UNINITIALIZED); + s = child_source(has ? with_parent(() => proxy(target[prop])) : UNINITIALIZED, stack); sources.set(prop, s); } @@ -262,7 +274,7 @@ export function proxy(value, 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 = child_source(UNINITIALIZED); + other_s = with_parent(() => source(UNINITIALIZED, stack)); sources.set(i + '', other_s); } } @@ -274,7 +286,7 @@ export function proxy(value, prev) { // object property before writing to that property. if (s === undefined) { if (!has || get_descriptor(target, prop)?.writable) { - s = child_source(undefined); + s = child_source(undefined, stack); set( s, with_parent(() => proxy(value)) From deef8c5305dddfabc0197b7c9c10058377a8e8bf Mon Sep 17 00:00:00 2001 From: Rich Harris Date: Thu, 20 Mar 2025 12:04:31 -0400 Subject: [PATCH 14/22] more --- packages/svelte/src/internal/client/proxy.js | 21 +++----------------- 1 file changed, 3 insertions(+), 18 deletions(-) diff --git a/packages/svelte/src/internal/client/proxy.js b/packages/svelte/src/internal/client/proxy.js index 4c14d33bbb05..ac1beb22c3a0 100644 --- a/packages/svelte/src/internal/client/proxy.js +++ b/packages/svelte/src/internal/client/proxy.js @@ -46,18 +46,6 @@ export function proxy(value, prev) { var stack = DEV && tracing_mode_flag ? get_stack('CreatedAt') : null; var reaction = active_reaction; - /** - * @param {any} value - * @param {Error | null | undefined} stack - */ - var child_source = (value, stack) => { - var previous_reaction = active_reaction; - set_active_reaction(reaction); - var s = source(value, stack); - set_active_reaction(previous_reaction); - return s; - }; - /** * @template T * @param {() => T} fn @@ -176,10 +164,7 @@ export function proxy(value, 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 = child_source( - with_parent(() => proxy(exists ? target[prop] : UNINITIALIZED)), - stack - ); + s = with_parent(() => source(proxy(exists ? target[prop] : UNINITIALIZED), stack)); sources.set(prop, s); } @@ -247,7 +232,7 @@ export function proxy(value, prev) { (active_effect !== null && (!has || get_descriptor(target, prop)?.writable)) ) { if (s === undefined) { - s = child_source(has ? with_parent(() => proxy(target[prop])) : UNINITIALIZED, stack); + s = with_parent(() => source(has ? proxy(target[prop]) : UNINITIALIZED, stack)); sources.set(prop, s); } @@ -286,7 +271,7 @@ export function proxy(value, prev) { // object property before writing to that property. if (s === undefined) { if (!has || get_descriptor(target, prop)?.writable) { - s = child_source(undefined, stack); + s = with_parent(() => source(undefined, stack)); set( s, with_parent(() => proxy(value)) From f1095b70a2b9fd234d95c8f25756b93d5850bdce Mon Sep 17 00:00:00 2001 From: Rich Harris Date: Thu, 20 Mar 2025 12:14:41 -0400 Subject: [PATCH 15/22] tidy up --- packages/svelte/src/internal/client/proxy.js | 19 ++++++++++++++----- 1 file changed, 14 insertions(+), 5 deletions(-) diff --git a/packages/svelte/src/internal/client/proxy.js b/packages/svelte/src/internal/client/proxy.js index ac1beb22c3a0..9c3c0cf29f29 100644 --- a/packages/svelte/src/internal/client/proxy.js +++ b/packages/svelte/src/internal/client/proxy.js @@ -53,12 +53,21 @@ export function proxy(value, prev) { var with_parent = (fn) => { var previous_reaction = active_reaction; set_active_reaction(reaction); - var previous_metadata = parent_metadata; - parent_metadata = metadata; - var p = fn(); + + /** @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); - parent_metadata = previous_metadata; - return p; + return result; }; if (is_proxied_array) { From 46db072503c5b293c376fc45e453fcf0be676fd9 Mon Sep 17 00:00:00 2001 From: Rich Harris Date: Thu, 20 Mar 2025 12:58:17 -0400 Subject: [PATCH 16/22] parent -> p --- .../client/dom/elements/transitions.js | 4 ++-- .../internal/client/reactivity/deriveds.js | 6 +++--- .../src/internal/client/reactivity/effects.js | 10 +++++----- .../src/internal/client/reactivity/sources.js | 4 ++-- .../src/internal/client/reactivity/types.d.ts | 4 ++-- .../svelte/src/internal/client/runtime.js | 19 +++++++++---------- 6 files changed, 23 insertions(+), 24 deletions(-) diff --git a/packages/svelte/src/internal/client/dom/elements/transitions.js b/packages/svelte/src/internal/client/dom/elements/transitions.js index fbc1da95df95..61f331404ccc 100644 --- a/packages/svelte/src/internal/client/dom/elements/transitions.js +++ b/packages/svelte/src/internal/client/dom/elements/transitions.js @@ -289,11 +289,11 @@ export function transition(flags, element, get_fn, get_params) { var run = is_global; if (!run) { - var block = /** @type {Effect | null} */ (e.parent); + var block = /** @type {Effect | null} */ (e.p); // skip over transparent blocks (e.g. snippets, else-if blocks) while (block && (block.f & EFFECT_TRANSPARENT) !== 0) { - while ((block = block.parent)) { + while ((block = block.p)) { if ((block.f & BLOCK_EFFECT) !== 0) break; } } diff --git a/packages/svelte/src/internal/client/reactivity/deriveds.js b/packages/svelte/src/internal/client/reactivity/deriveds.js index 795417cc0fdb..a1e464a890ba 100644 --- a/packages/svelte/src/internal/client/reactivity/deriveds.js +++ b/packages/svelte/src/internal/client/reactivity/deriveds.js @@ -51,7 +51,7 @@ export function derived(fn) { rv: 0, v: /** @type {V} */ (null), wv: 0, - parent: parent_derived ?? active_effect + p: parent_derived ?? active_effect }; if (DEV && tracing_mode_flag) { @@ -101,12 +101,12 @@ let stack = []; * @returns {Effect | null} */ function get_derived_parent_effect(derived) { - var parent = derived.parent; + var parent = derived.p; while (parent !== null) { if ((parent.f & DERIVED) === 0) { return /** @type {Effect} */ (parent); } - parent = parent.parent; + parent = parent.p; } return null; } diff --git a/packages/svelte/src/internal/client/reactivity/effects.js b/packages/svelte/src/internal/client/reactivity/effects.js index 468bb94ab428..4317764fba9f 100644 --- a/packages/svelte/src/internal/client/reactivity/effects.js +++ b/packages/svelte/src/internal/client/reactivity/effects.js @@ -87,7 +87,7 @@ function create_effect(type, fn, sync, push = true) { if (DEV) { // Ensure the parent is never an inspect effect while (parent !== null && (parent.f & INSPECT_EFFECT) !== 0) { - parent = parent.parent; + parent = parent.p; } } @@ -102,7 +102,7 @@ function create_effect(type, fn, sync, push = true) { fn, last: null, next: null, - parent, + p: parent, prev: null, teardown: null, transitions: null, @@ -393,7 +393,7 @@ export function destroy_effect_children(signal, remove_dom = false) { if ((effect.f & ROOT_EFFECT) !== 0) { // this is now an independent root - effect.parent = null; + effect.p = null; } else { destroy_effect(effect, remove_dom); } @@ -456,7 +456,7 @@ export function destroy_effect(effect, remove_dom = true) { execute_effect_teardown(effect); - var parent = effect.parent; + var parent = effect.p; // If the parent doesn't have any children, then skip this work altogether if (parent !== null && parent.first !== null) { @@ -486,7 +486,7 @@ export function destroy_effect(effect, remove_dom = true) { * @param {Effect} effect */ export function unlink_effect(effect) { - var parent = effect.parent; + var parent = effect.p; var prev = effect.prev; var next = effect.next; diff --git a/packages/svelte/src/internal/client/reactivity/sources.js b/packages/svelte/src/internal/client/reactivity/sources.js index 552102aeb28d..4ead4d33bd32 100644 --- a/packages/svelte/src/internal/client/reactivity/sources.js +++ b/packages/svelte/src/internal/client/reactivity/sources.js @@ -55,7 +55,7 @@ export function source(v, stack) { var signal = { f: 0, // TODO ideally we could skip this altogether, but it causes type errors v, - parent: active_reaction, + p: active_reaction, reactions: null, equals, rv: 0, @@ -118,7 +118,7 @@ export function set(source, value, should_proxy = false) { !untracking && is_runes() && (active_reaction.f & (DERIVED | BLOCK_EFFECT)) !== 0 && - active_reaction !== source.parent + active_reaction !== source.p ) { e.state_unsafe_mutation(); } diff --git a/packages/svelte/src/internal/client/reactivity/types.d.ts b/packages/svelte/src/internal/client/reactivity/types.d.ts index 7a44017229d3..daad42c73771 100644 --- a/packages/svelte/src/internal/client/reactivity/types.d.ts +++ b/packages/svelte/src/internal/client/reactivity/types.d.ts @@ -6,7 +6,7 @@ export interface Signal { /** Write version */ wv: number; /** Parent effect or derived */ - parent: Reaction | null; + p: Reaction | null; } export interface Value extends Signal { @@ -66,7 +66,7 @@ export interface Effect extends Reaction { /** Last child effect created inside this signal */ last: null | Effect; /** Parent effect */ - parent: Effect | null; + p: Effect | null; /** Dev only */ component_function?: any; } diff --git a/packages/svelte/src/internal/client/runtime.js b/packages/svelte/src/internal/client/runtime.js index ccce3f6bd9ec..9a152497d0ae 100644 --- a/packages/svelte/src/internal/client/runtime.js +++ b/packages/svelte/src/internal/client/runtime.js @@ -161,7 +161,7 @@ export function check_dirtiness(reaction) { // then we need to re-connect the reaction to the dependency if (is_disconnected || is_unowned_connected) { var derived = /** @type {Derived} */ (reaction); - var parent = derived.parent; + var parent = derived.p; for (i = 0; i < length; i++) { dependency = dependencies[i]; @@ -228,7 +228,7 @@ function propagate_error(error, effect) { } } - current = current.parent; + current = current.p; } is_throwing_error = false; @@ -240,8 +240,7 @@ function propagate_error(error, effect) { */ function should_rethrow_error(effect) { return ( - (effect.f & DESTROYED) === 0 && - (effect.parent === null || (effect.parent.f & BOUNDARY_EFFECT) === 0) + (effect.f & DESTROYED) === 0 && (effect.p === null || (effect.p.f & BOUNDARY_EFFECT) === 0) ); } @@ -353,7 +352,7 @@ function schedule_possible_effect_self_invalidation(signal, effect, root = true) for (var i = 0; i < reactions.length; i++) { var reaction = reactions[i]; - if (signal.parent === reaction) continue; + if (signal.p === reaction) continue; if ((reaction.f & DERIVED) !== 0) { schedule_possible_effect_self_invalidation(/** @type {Derived} */ (reaction), effect, false); @@ -720,8 +719,8 @@ export function schedule_effect(signal) { var effect = (last_scheduled_effect = signal); - while (effect.parent !== null) { - effect = effect.parent; + while (effect.p !== null) { + effect = effect.p; var flags = effect.f; if ((flags & (ROOT_EFFECT | BRANCH_EFFECT)) !== 0) { @@ -786,12 +785,12 @@ function process_effects(root) { } } - var parent = effect.parent; + var parent = effect.p; effect = effect.next; while (effect === null && parent !== null) { effect = parent.next; - parent = parent.parent; + parent = parent.p; } } @@ -874,7 +873,7 @@ export function get(signal) { /** @type {Derived} */ (signal).effects === null ) { var derived = /** @type {Derived} */ (signal); - var parent = derived.parent; + var parent = derived.p; if (parent !== null && (parent.f & UNOWNED) === 0) { // If the derived is owned by another derived then mark it as unowned From fea1b79b8d12adad25ea39dcad2c4c08549e2c89 Mon Sep 17 00:00:00 2001 From: Rich Harris Date: Thu, 20 Mar 2025 14:28:19 -0400 Subject: [PATCH 17/22] tmp --- packages/svelte/src/internal/client/reactivity/sources.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/svelte/src/internal/client/reactivity/sources.js b/packages/svelte/src/internal/client/reactivity/sources.js index 4ead4d33bd32..037a18d7981c 100644 --- a/packages/svelte/src/internal/client/reactivity/sources.js +++ b/packages/svelte/src/internal/client/reactivity/sources.js @@ -55,7 +55,7 @@ export function source(v, stack) { var signal = { f: 0, // TODO ideally we could skip this altogether, but it causes type errors v, - p: active_reaction, + // p: active_reaction, reactions: null, equals, rv: 0, From da4f0c349440bf6c513a114a212120fcc0f2f08d Mon Sep 17 00:00:00 2001 From: Rich Harris Date: Thu, 20 Mar 2025 15:19:26 -0400 Subject: [PATCH 18/22] alternative approach --- .../svelte/src/internal/client/constants.js | 1 + .../client/dom/elements/transitions.js | 4 +- .../internal/client/reactivity/deriveds.js | 6 +-- .../src/internal/client/reactivity/effects.js | 10 ++--- .../src/internal/client/reactivity/sources.js | 22 ++++++++++- .../src/internal/client/reactivity/types.d.ts | 6 +-- .../svelte/src/internal/client/runtime.js | 38 +++++++++++++------ 7 files changed, 61 insertions(+), 26 deletions(-) diff --git a/packages/svelte/src/internal/client/constants.js b/packages/svelte/src/internal/client/constants.js index a4840ce4ebd0..21377c1cc85f 100644 --- a/packages/svelte/src/internal/client/constants.js +++ b/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'); diff --git a/packages/svelte/src/internal/client/dom/elements/transitions.js b/packages/svelte/src/internal/client/dom/elements/transitions.js index 61f331404ccc..fbc1da95df95 100644 --- a/packages/svelte/src/internal/client/dom/elements/transitions.js +++ b/packages/svelte/src/internal/client/dom/elements/transitions.js @@ -289,11 +289,11 @@ export function transition(flags, element, get_fn, get_params) { var run = is_global; if (!run) { - var block = /** @type {Effect | null} */ (e.p); + var block = /** @type {Effect | null} */ (e.parent); // skip over transparent blocks (e.g. snippets, else-if blocks) while (block && (block.f & EFFECT_TRANSPARENT) !== 0) { - while ((block = block.p)) { + while ((block = block.parent)) { if ((block.f & BLOCK_EFFECT) !== 0) break; } } diff --git a/packages/svelte/src/internal/client/reactivity/deriveds.js b/packages/svelte/src/internal/client/reactivity/deriveds.js index a1e464a890ba..795417cc0fdb 100644 --- a/packages/svelte/src/internal/client/reactivity/deriveds.js +++ b/packages/svelte/src/internal/client/reactivity/deriveds.js @@ -51,7 +51,7 @@ export function derived(fn) { rv: 0, v: /** @type {V} */ (null), wv: 0, - p: parent_derived ?? active_effect + parent: parent_derived ?? active_effect }; if (DEV && tracing_mode_flag) { @@ -101,12 +101,12 @@ let stack = []; * @returns {Effect | null} */ function get_derived_parent_effect(derived) { - var parent = derived.p; + var parent = derived.parent; while (parent !== null) { if ((parent.f & DERIVED) === 0) { return /** @type {Effect} */ (parent); } - parent = parent.p; + parent = parent.parent; } return null; } diff --git a/packages/svelte/src/internal/client/reactivity/effects.js b/packages/svelte/src/internal/client/reactivity/effects.js index 4317764fba9f..0e6bda97c2e2 100644 --- a/packages/svelte/src/internal/client/reactivity/effects.js +++ b/packages/svelte/src/internal/client/reactivity/effects.js @@ -87,7 +87,7 @@ function create_effect(type, fn, sync, push = true) { if (DEV) { // Ensure the parent is never an inspect effect while (parent !== null && (parent.f & INSPECT_EFFECT) !== 0) { - parent = parent.p; + parent = parent.parent; } } @@ -102,7 +102,7 @@ function create_effect(type, fn, sync, push = true) { fn, last: null, next: null, - p: parent, + parent: parent, prev: null, teardown: null, transitions: null, @@ -393,7 +393,7 @@ export function destroy_effect_children(signal, remove_dom = false) { if ((effect.f & ROOT_EFFECT) !== 0) { // this is now an independent root - effect.p = null; + effect.parent = null; } else { destroy_effect(effect, remove_dom); } @@ -456,7 +456,7 @@ export function destroy_effect(effect, remove_dom = true) { execute_effect_teardown(effect); - var parent = effect.p; + var parent = effect.parent; // If the parent doesn't have any children, then skip this work altogether if (parent !== null && parent.first !== null) { @@ -486,7 +486,7 @@ export function destroy_effect(effect, remove_dom = true) { * @param {Effect} effect */ export function unlink_effect(effect) { - var parent = effect.p; + var parent = effect.parent; var prev = effect.prev; var next = effect.next; diff --git a/packages/svelte/src/internal/client/reactivity/sources.js b/packages/svelte/src/internal/client/reactivity/sources.js index 037a18d7981c..79519e55a616 100644 --- a/packages/svelte/src/internal/client/reactivity/sources.js +++ b/packages/svelte/src/internal/client/reactivity/sources.js @@ -25,7 +25,9 @@ import { UNOWNED, MAYBE_DIRTY, BLOCK_EFFECT, - ROOT_EFFECT + ROOT_EFFECT, + EFFECT, + EFFECT_IS_UPDATING } from '../constants.js'; import * as e from '../errors.js'; import { legacy_mode_flag, tracing_mode_flag } from '../../flags/index.js'; @@ -36,6 +38,14 @@ import { proxy } from '../proxy.js'; export let inspect_effects = new Set(); export const old_values = new Map(); +/** @type {Source[] | null} */ +export let reaction_sources = null; + +/** @param {Source[] | null} v */ +export function set_reaction_sources(v) { + reaction_sources = v; +} + /** * @param {Set} v */ @@ -62,6 +72,14 @@ export function source(v, stack) { wv: 0 }; + if (active_reaction !== null && active_reaction.f & EFFECT_IS_UPDATING) { + if (reaction_sources === null) { + reaction_sources = [signal]; + } else { + reaction_sources.push(signal); + } + } + if (DEV && tracing_mode_flag) { signal.created = stack ?? get_stack('CreatedAt'); signal.debug = null; @@ -118,7 +136,7 @@ export function set(source, value, should_proxy = false) { !untracking && is_runes() && (active_reaction.f & (DERIVED | BLOCK_EFFECT)) !== 0 && - active_reaction !== source.p + !reaction_sources?.includes(source) ) { e.state_unsafe_mutation(); } diff --git a/packages/svelte/src/internal/client/reactivity/types.d.ts b/packages/svelte/src/internal/client/reactivity/types.d.ts index daad42c73771..5ef0097649a4 100644 --- a/packages/svelte/src/internal/client/reactivity/types.d.ts +++ b/packages/svelte/src/internal/client/reactivity/types.d.ts @@ -5,8 +5,6 @@ export interface Signal { f: number; /** Write version */ wv: number; - /** Parent effect or derived */ - p: Reaction | null; } export interface Value extends Signal { @@ -40,6 +38,8 @@ export interface Derived extends Value, Reaction { fn: () => V; /** Effects created inside this signal */ effects: null | Effect[]; + /** Parent effect or derived */ + parent: Effect | Derived | null; } export interface Effect extends Reaction { @@ -66,7 +66,7 @@ export interface Effect extends Reaction { /** Last child effect created inside this signal */ last: null | Effect; /** Parent effect */ - p: Effect | null; + parent: Effect | null; /** Dev only */ component_function?: any; } diff --git a/packages/svelte/src/internal/client/runtime.js b/packages/svelte/src/internal/client/runtime.js index 9a152497d0ae..bccd3aae590a 100644 --- a/packages/svelte/src/internal/client/runtime.js +++ b/packages/svelte/src/internal/client/runtime.js @@ -22,10 +22,16 @@ 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'; +import { + internal_set, + old_values, + reaction_sources, + set_reaction_sources +} from './reactivity/sources.js'; import { destroy_derived_effects, update_derived } from './reactivity/deriveds.js'; import * as e from './errors.js'; import { FILENAME } from '../../constants.js'; @@ -161,7 +167,7 @@ export function check_dirtiness(reaction) { // then we need to re-connect the reaction to the dependency if (is_disconnected || is_unowned_connected) { var derived = /** @type {Derived} */ (reaction); - var parent = derived.p; + var parent = derived.parent; for (i = 0; i < length; i++) { dependency = dependencies[i]; @@ -228,7 +234,7 @@ function propagate_error(error, effect) { } } - current = current.p; + current = current.parent; } is_throwing_error = false; @@ -240,7 +246,8 @@ function propagate_error(error, effect) { */ function should_rethrow_error(effect) { return ( - (effect.f & DESTROYED) === 0 && (effect.p === null || (effect.p.f & BOUNDARY_EFFECT) === 0) + (effect.f & DESTROYED) === 0 && + (effect.parent === null || (effect.parent.f & BOUNDARY_EFFECT) === 0) ); } @@ -352,7 +359,8 @@ function schedule_possible_effect_self_invalidation(signal, effect, root = true) for (var i = 0; i < reactions.length; i++) { var reaction = reactions[i]; - if (signal.p === reaction) continue; + + if (reaction_sources?.includes(signal)) continue; if ((reaction.f & DERIVED) !== 0) { schedule_possible_effect_self_invalidation(/** @type {Derived} */ (reaction), effect, false); @@ -380,8 +388,11 @@ export function update_reaction(reaction) { var previous_skip_reaction = skip_reaction; var previous_component_context = component_context; var previous_untracking = untracking; + var previous_reaction_sources = reaction_sources; + var flags = reaction.f; + set_reaction_sources(null); new_deps = /** @type {null | Value[]} */ (null); skipped_deps = 0; untracked_writes = null; @@ -393,6 +404,8 @@ export function update_reaction(reaction) { untracking = false; read_version++; + reaction.f |= EFFECT_IS_UPDATING; + try { var result = /** @type {Function} */ (0, reaction.fn)(); var deps = reaction.deps; @@ -464,6 +477,9 @@ export function update_reaction(reaction) { skip_reaction = previous_skip_reaction; set_component_context(previous_component_context); untracking = previous_untracking; + set_reaction_sources(previous_reaction_sources); + + reaction.f ^= EFFECT_IS_UPDATING; } } @@ -719,8 +735,8 @@ export function schedule_effect(signal) { var effect = (last_scheduled_effect = signal); - while (effect.p !== null) { - effect = effect.p; + while (effect.parent !== null) { + effect = effect.parent; var flags = effect.f; if ((flags & (ROOT_EFFECT | BRANCH_EFFECT)) !== 0) { @@ -785,12 +801,12 @@ function process_effects(root) { } } - var parent = effect.p; + var parent = effect.parent; effect = effect.next; while (effect === null && parent !== null) { effect = parent.next; - parent = parent.p; + parent = parent.parent; } } @@ -873,7 +889,7 @@ export function get(signal) { /** @type {Derived} */ (signal).effects === null ) { var derived = /** @type {Derived} */ (signal); - var parent = derived.p; + var parent = derived.parent; if (parent !== null && (parent.f & UNOWNED) === 0) { // If the derived is owned by another derived then mark it as unowned From 548a696e3d2456f02145a8ec05e5887de508571a Mon Sep 17 00:00:00 2001 From: Rich Harris Date: Thu, 20 Mar 2025 15:24:34 -0400 Subject: [PATCH 19/22] tidy up --- packages/svelte/src/internal/client/reactivity/effects.js | 2 +- packages/svelte/src/internal/client/reactivity/sources.js | 1 - 2 files changed, 1 insertion(+), 2 deletions(-) diff --git a/packages/svelte/src/internal/client/reactivity/effects.js b/packages/svelte/src/internal/client/reactivity/effects.js index 0e6bda97c2e2..468bb94ab428 100644 --- a/packages/svelte/src/internal/client/reactivity/effects.js +++ b/packages/svelte/src/internal/client/reactivity/effects.js @@ -102,7 +102,7 @@ function create_effect(type, fn, sync, push = true) { fn, last: null, next: null, - parent: parent, + parent, prev: null, teardown: null, transitions: null, diff --git a/packages/svelte/src/internal/client/reactivity/sources.js b/packages/svelte/src/internal/client/reactivity/sources.js index 79519e55a616..6349df24d0cd 100644 --- a/packages/svelte/src/internal/client/reactivity/sources.js +++ b/packages/svelte/src/internal/client/reactivity/sources.js @@ -65,7 +65,6 @@ export function source(v, stack) { var signal = { f: 0, // TODO ideally we could skip this altogether, but it causes type errors v, - // p: active_reaction, reactions: null, equals, rv: 0, From 8e7f6bfe86580a8661e595e25b92553f68e08736 Mon Sep 17 00:00:00 2001 From: Rich Harris Date: Thu, 20 Mar 2025 15:39:44 -0400 Subject: [PATCH 20/22] reduce diff size --- .../src/internal/client/reactivity/sources.js | 14 +++------- .../svelte/src/internal/client/runtime.js | 27 ++++++++++++------- 2 files changed, 22 insertions(+), 19 deletions(-) diff --git a/packages/svelte/src/internal/client/reactivity/sources.js b/packages/svelte/src/internal/client/reactivity/sources.js index 6349df24d0cd..ba80b1cc98c7 100644 --- a/packages/svelte/src/internal/client/reactivity/sources.js +++ b/packages/svelte/src/internal/client/reactivity/sources.js @@ -13,7 +13,9 @@ import { update_effect, check_dirtiness, untracking, - is_destroying_effect + is_destroying_effect, + reaction_sources, + set_reaction_sources } from '../runtime.js'; import { equals, safe_equals } from './equality.js'; import { @@ -38,14 +40,6 @@ import { proxy } from '../proxy.js'; export let inspect_effects = new Set(); export const old_values = new Map(); -/** @type {Source[] | null} */ -export let reaction_sources = null; - -/** @param {Source[] | null} v */ -export function set_reaction_sources(v) { - reaction_sources = v; -} - /** * @param {Set} v */ @@ -73,7 +67,7 @@ export function source(v, stack) { if (active_reaction !== null && active_reaction.f & EFFECT_IS_UPDATING) { if (reaction_sources === null) { - reaction_sources = [signal]; + set_reaction_sources([signal]); } else { reaction_sources.push(signal); } diff --git a/packages/svelte/src/internal/client/runtime.js b/packages/svelte/src/internal/client/runtime.js index bccd3aae590a..e8a1911d99aa 100644 --- a/packages/svelte/src/internal/client/runtime.js +++ b/packages/svelte/src/internal/client/runtime.js @@ -26,12 +26,7 @@ import { EFFECT_IS_UPDATING } from './constants.js'; import { flush_tasks } from './dom/task.js'; -import { - internal_set, - old_values, - reaction_sources, - set_reaction_sources -} from './reactivity/sources.js'; +import { internal_set, old_values } from './reactivity/sources.js'; import { destroy_derived_effects, update_derived } from './reactivity/deriveds.js'; import * as e from './errors.js'; import { FILENAME } from '../../constants.js'; @@ -87,6 +82,20 @@ export function set_active_reaction(reaction) { /** @type {null | Effect} */ export let active_effect = null; +/** + * 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. + * @type {null | Source[]} + */ +export let reaction_sources = null; + +/** + * @param {Source[] | null} sources + */ +export function set_reaction_sources(sources) { + reaction_sources = sources; +} + /** @param {null | Effect} effect */ export function set_active_effect(effect) { active_effect = effect; @@ -386,13 +395,12 @@ export function update_reaction(reaction) { var previous_untracked_writes = untracked_writes; var previous_reaction = active_reaction; var previous_skip_reaction = skip_reaction; + var previous_reaction_sources = reaction_sources; var previous_component_context = component_context; var previous_untracking = untracking; - var previous_reaction_sources = reaction_sources; var flags = reaction.f; - set_reaction_sources(null); new_deps = /** @type {null | Value[]} */ (null); skipped_deps = 0; untracked_writes = null; @@ -400,6 +408,7 @@ 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; + reaction_sources = null; set_component_context(reaction.ctx); untracking = false; read_version++; @@ -475,9 +484,9 @@ export function update_reaction(reaction) { untracked_writes = previous_untracked_writes; active_reaction = previous_reaction; skip_reaction = previous_skip_reaction; + reaction_sources = previous_reaction_sources; set_component_context(previous_component_context); untracking = previous_untracking; - set_reaction_sources(previous_reaction_sources); reaction.f ^= EFFECT_IS_UPDATING; } From 0b0b7b80f0371abe4478bc9ba570b3256ace994c Mon Sep 17 00:00:00 2001 From: Rich Harris Date: Thu, 20 Mar 2025 15:41:03 -0400 Subject: [PATCH 21/22] more --- .../svelte/src/internal/client/reactivity/sources.js | 7 +++---- packages/svelte/src/internal/client/runtime.js | 10 +++++----- 2 files changed, 8 insertions(+), 9 deletions(-) diff --git a/packages/svelte/src/internal/client/reactivity/sources.js b/packages/svelte/src/internal/client/reactivity/sources.js index ba80b1cc98c7..cac8431b4e60 100644 --- a/packages/svelte/src/internal/client/reactivity/sources.js +++ b/packages/svelte/src/internal/client/reactivity/sources.js @@ -11,11 +11,11 @@ import { untrack, increment_write_version, update_effect, + reaction_sources, + set_reaction_sources, check_dirtiness, untracking, - is_destroying_effect, - reaction_sources, - set_reaction_sources + is_destroying_effect } from '../runtime.js'; import { equals, safe_equals } from './equality.js'; import { @@ -28,7 +28,6 @@ import { MAYBE_DIRTY, BLOCK_EFFECT, ROOT_EFFECT, - EFFECT, EFFECT_IS_UPDATING } from '../constants.js'; import * as e from '../errors.js'; diff --git a/packages/svelte/src/internal/client/runtime.js b/packages/svelte/src/internal/client/runtime.js index e8a1911d99aa..9ce62a564fa0 100644 --- a/packages/svelte/src/internal/client/runtime.js +++ b/packages/svelte/src/internal/client/runtime.js @@ -82,6 +82,11 @@ export function set_active_reaction(reaction) { /** @type {null | Effect} */ export let active_effect = null; +/** @param {null | Effect} effect */ +export function set_active_effect(effect) { + 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. @@ -96,11 +101,6 @@ export function set_reaction_sources(sources) { reaction_sources = sources; } -/** @param {null | Effect} effect */ -export function set_active_effect(effect) { - active_effect = effect; -} - /** * 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 From 82c6f39003fcea91b0d377d517f465ad39cf008e Mon Sep 17 00:00:00 2001 From: Rich Harris Date: Thu, 20 Mar 2025 15:41:59 -0400 Subject: [PATCH 22/22] update comment --- packages/svelte/src/internal/client/runtime.js | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/packages/svelte/src/internal/client/runtime.js b/packages/svelte/src/internal/client/runtime.js index 9ce62a564fa0..74b58ee1a935 100644 --- a/packages/svelte/src/internal/client/runtime.js +++ b/packages/svelte/src/internal/client/runtime.js @@ -88,8 +88,8 @@ 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 reaction_sources = null;