From cb28da55967d6b6e8ec71684a0ff9509d6a16fe1 Mon Sep 17 00:00:00 2001 From: Simon Holthausen Date: Tue, 17 Jun 2025 18:07:09 +0200 Subject: [PATCH 1/2] fix: ensure sources within nested effects still register correctly When an effect creates another effect and a proxy property is read that wasn't before, the proxy will have logic where it creates the new signal in its original reaction context. But it did not have logic to correctly handle the `reaction_sources` array which prevents effects/deriveds rerunning when state created inside themselves is read and then changed inside them: Since `reaction_sources` wasn't take the reaction context into account, false positives could occur where nested effects would not register the sources as dependencies. Fixes #15870 --- .changeset/rich-emus-study.md | 5 +++ packages/svelte/src/internal/client/proxy.js | 1 + .../svelte/src/internal/client/runtime.js | 12 +++---- packages/svelte/tests/signals/test.ts | 35 +++++++++++++++++++ 4 files changed, 47 insertions(+), 6 deletions(-) create mode 100644 .changeset/rich-emus-study.md diff --git a/.changeset/rich-emus-study.md b/.changeset/rich-emus-study.md new file mode 100644 index 000000000000..dcadafacb19b --- /dev/null +++ b/.changeset/rich-emus-study.md @@ -0,0 +1,5 @@ +--- +'svelte': patch +--- + +fix: ensure sources within nested effects still register correctly diff --git a/packages/svelte/src/internal/client/proxy.js b/packages/svelte/src/internal/client/proxy.js index 60eba6aa8708..d9063aee3436 100644 --- a/packages/svelte/src/internal/client/proxy.js +++ b/packages/svelte/src/internal/client/proxy.js @@ -44,6 +44,7 @@ export function proxy(value) { var reaction = active_reaction; /** + * Executes the proxy in the context of the reaction it was originally created in, if any * @template T * @param {() => T} fn */ diff --git a/packages/svelte/src/internal/client/runtime.js b/packages/svelte/src/internal/client/runtime.js index 954406095904..56bc157f33cf 100644 --- a/packages/svelte/src/internal/client/runtime.js +++ b/packages/svelte/src/internal/client/runtime.js @@ -84,8 +84,8 @@ export function set_active_effect(effect) { /** * When sources are created within a reaction, reading and writing - * them should not cause a re-run - * @type {null | Source[]} + * them within that reaction should not cause a re-run + * @type {null | [active_reaction: Reaction, sources: Source[]]} */ export let reaction_sources = null; @@ -93,9 +93,9 @@ export let reaction_sources = null; export function push_reaction_value(value) { if (active_reaction !== null && active_reaction.f & EFFECT_IS_UPDATING) { if (reaction_sources === null) { - reaction_sources = [value]; + reaction_sources = [active_reaction, [value]]; } else { - reaction_sources.push(value); + reaction_sources[1].push(value); } } } @@ -234,7 +234,7 @@ 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_sources?.[1].includes(signal) && reaction_sources[0] === active_reaction) continue; if ((reaction.f & DERIVED) !== 0) { schedule_possible_effect_self_invalidation(/** @type {Derived} */ (reaction), effect, false); @@ -724,7 +724,7 @@ export function get(signal) { // Register the dependency on the current reaction signal. if (active_reaction !== null && !untracking) { - if (!reaction_sources?.includes(signal)) { + if (!reaction_sources?.[1].includes(signal) || reaction_sources[0] !== active_reaction) { 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 8421ae4a7cbf..78d7919e0fb1 100644 --- a/packages/svelte/tests/signals/test.ts +++ b/packages/svelte/tests/signals/test.ts @@ -1021,6 +1021,41 @@ describe('signals', () => { }; }); + test('nested effects depend on state of upper effects', () => { + const logs: number[] = []; + + user_effect(() => { + const raw = state(0); + const proxied = proxy({ current: 0 }); + + // We need those separate, else one working and rerunning the effect + // could mask the other one not rerunning + user_effect(() => { + logs.push($.get(raw)); + }); + + user_effect(() => { + logs.push(proxied.current); + }); + + // Important so that the updating effect is not running + // together with the reading effects + flushSync(); + + user_effect(() => { + $.untrack(() => { + set(raw, $.get(raw) + 1); + proxied.current += 1; + }); + }); + }); + + return () => { + flushSync(); + assert.deepEqual(logs, [0, 0, 1, 1]); + }; + }); + test('proxy version state does not trigger self-dependency guard', () => { return () => { const s = proxy({ a: { b: 1 } }); From efb9b63935d1fa9624b9e5b3d5074c5960ed8694 Mon Sep 17 00:00:00 2001 From: Simon Holthausen Date: Tue, 17 Jun 2025 18:24:54 +0200 Subject: [PATCH 2/2] oops forgot to push this --- 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 56f41382520a..4959bc1abc85 100644 --- a/packages/svelte/src/internal/client/reactivity/sources.js +++ b/packages/svelte/src/internal/client/reactivity/sources.js @@ -138,7 +138,7 @@ export function set(source, value, should_proxy = false) { !untracking && is_runes() && (active_reaction.f & (DERIVED | BLOCK_EFFECT)) !== 0 && - !reaction_sources?.includes(source) + !(reaction_sources?.[1].includes(source) && reaction_sources[0] === active_reaction) ) { e.state_unsafe_mutation(); }