Skip to content

Commit 061ab31

Browse files
authored
fix: ensure sources within nested effects still register correctly (#16193)
* 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 * oops forgot to push this
1 parent 0524d16 commit 061ab31

File tree

5 files changed

+48
-7
lines changed

5 files changed

+48
-7
lines changed

.changeset/rich-emus-study.md

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
---
2+
'svelte': patch
3+
---
4+
5+
fix: ensure sources within nested effects still register correctly

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

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -44,6 +44,7 @@ export function proxy(value) {
4444
var reaction = active_reaction;
4545

4646
/**
47+
* Executes the proxy in the context of the reaction it was originally created in, if any
4748
* @template T
4849
* @param {() => T} fn
4950
*/

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -138,7 +138,7 @@ export function set(source, value, should_proxy = false) {
138138
!untracking &&
139139
is_runes() &&
140140
(active_reaction.f & (DERIVED | BLOCK_EFFECT)) !== 0 &&
141-
!reaction_sources?.includes(source)
141+
!(reaction_sources?.[1].includes(source) && reaction_sources[0] === active_reaction)
142142
) {
143143
e.state_unsafe_mutation();
144144
}

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

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -84,18 +84,18 @@ export function set_active_effect(effect) {
8484

8585
/**
8686
* When sources are created within a reaction, reading and writing
87-
* them should not cause a re-run
88-
* @type {null | Source[]}
87+
* them within that reaction should not cause a re-run
88+
* @type {null | [active_reaction: Reaction, sources: Source[]]}
8989
*/
9090
export let reaction_sources = null;
9191

9292
/** @param {Value} value */
9393
export function push_reaction_value(value) {
9494
if (active_reaction !== null && active_reaction.f & EFFECT_IS_UPDATING) {
9595
if (reaction_sources === null) {
96-
reaction_sources = [value];
96+
reaction_sources = [active_reaction, [value]];
9797
} else {
98-
reaction_sources.push(value);
98+
reaction_sources[1].push(value);
9999
}
100100
}
101101
}
@@ -234,7 +234,7 @@ function schedule_possible_effect_self_invalidation(signal, effect, root = true)
234234
for (var i = 0; i < reactions.length; i++) {
235235
var reaction = reactions[i];
236236

237-
if (reaction_sources?.includes(signal)) continue;
237+
if (reaction_sources?.[1].includes(signal) && reaction_sources[0] === active_reaction) continue;
238238

239239
if ((reaction.f & DERIVED) !== 0) {
240240
schedule_possible_effect_self_invalidation(/** @type {Derived} */ (reaction), effect, false);
@@ -724,7 +724,7 @@ export function get(signal) {
724724

725725
// Register the dependency on the current reaction signal.
726726
if (active_reaction !== null && !untracking) {
727-
if (!reaction_sources?.includes(signal)) {
727+
if (!reaction_sources?.[1].includes(signal) || reaction_sources[0] !== active_reaction) {
728728
var deps = active_reaction.deps;
729729
if (signal.rv < read_version) {
730730
signal.rv = read_version;

packages/svelte/tests/signals/test.ts

Lines changed: 35 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1021,6 +1021,41 @@ describe('signals', () => {
10211021
};
10221022
});
10231023

1024+
test('nested effects depend on state of upper effects', () => {
1025+
const logs: number[] = [];
1026+
1027+
user_effect(() => {
1028+
const raw = state(0);
1029+
const proxied = proxy({ current: 0 });
1030+
1031+
// We need those separate, else one working and rerunning the effect
1032+
// could mask the other one not rerunning
1033+
user_effect(() => {
1034+
logs.push($.get(raw));
1035+
});
1036+
1037+
user_effect(() => {
1038+
logs.push(proxied.current);
1039+
});
1040+
1041+
// Important so that the updating effect is not running
1042+
// together with the reading effects
1043+
flushSync();
1044+
1045+
user_effect(() => {
1046+
$.untrack(() => {
1047+
set(raw, $.get(raw) + 1);
1048+
proxied.current += 1;
1049+
});
1050+
});
1051+
});
1052+
1053+
return () => {
1054+
flushSync();
1055+
assert.deepEqual(logs, [0, 0, 1, 1]);
1056+
};
1057+
});
1058+
10241059
test('proxy version state does not trigger self-dependency guard', () => {
10251060
return () => {
10261061
const s = proxy({ a: { b: 1 } });

0 commit comments

Comments
 (0)