Skip to content

Commit 7eb11e0

Browse files
authored
fix: don't destroy effect roots created inside of deriveds (#16492)
We were wrongfully adding effect roots to `derived.effects`, too, which meant those were destroyed when the derived reran.
1 parent 4e74cd3 commit 7eb11e0

File tree

4 files changed

+62
-2
lines changed

4 files changed

+62
-2
lines changed

.changeset/lemon-weeks-call.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: don't destroy effect roots created inside of deriveds

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

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -148,7 +148,11 @@ function create_effect(type, fn, sync, push = true) {
148148
}
149149

150150
// if we're in a derived, add the effect there too
151-
if (active_reaction !== null && (active_reaction.f & DERIVED) !== 0) {
151+
if (
152+
active_reaction !== null &&
153+
(active_reaction.f & DERIVED) !== 0 &&
154+
(type & ROOT_EFFECT) === 0
155+
) {
152156
var derived = /** @type {Derived} */ (active_reaction);
153157
(derived.effects ??= []).push(effect);
154158
}

packages/svelte/src/internal/client/reactivity/types.d.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -54,7 +54,7 @@ export interface Reaction extends Signal {
5454
export interface Derived<V = unknown> extends Value<V>, Reaction {
5555
/** The derived function */
5656
fn: () => V;
57-
/** Effects created inside this signal */
57+
/** Effects created inside this signal. Used to destroy those effects when the derived reruns or is cleaned up */
5858
effects: null | Effect[];
5959
/** Parent effect or derived */
6060
parent: Effect | Derived | null;

packages/svelte/tests/signals/test.ts

Lines changed: 51 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1390,4 +1390,55 @@ describe('signals', () => {
13901390
destroy();
13911391
};
13921392
});
1393+
1394+
test('$effect.root inside deriveds stay alive independently', () => {
1395+
const log: any[] = [];
1396+
const c = state(0);
1397+
const cleanup: any[] = [];
1398+
const inner_states: any[] = [];
1399+
1400+
const d = derived(() => {
1401+
const destroy = effect_root(() => {
1402+
const x = state(0);
1403+
inner_states.push(x);
1404+
1405+
effect(() => {
1406+
log.push('inner ' + $.get(x));
1407+
return () => {
1408+
log.push('inner destroyed');
1409+
};
1410+
});
1411+
});
1412+
1413+
cleanup.push(destroy);
1414+
1415+
return $.get(c);
1416+
});
1417+
1418+
return () => {
1419+
log.push($.get(d));
1420+
flushSync();
1421+
1422+
assert.deepEqual(log, [0, 'inner 0']);
1423+
log.length = 0;
1424+
1425+
set(inner_states[0], 1);
1426+
flushSync();
1427+
1428+
assert.deepEqual(log, ['inner destroyed', 'inner 1']);
1429+
log.length = 0;
1430+
1431+
set(c, 1);
1432+
log.push($.get(d));
1433+
flushSync();
1434+
1435+
assert.deepEqual(log, [1, 'inner 0']);
1436+
log.length = 0;
1437+
1438+
cleanup.forEach((fn) => fn());
1439+
flushSync();
1440+
1441+
assert.deepEqual(log, ['inner destroyed', 'inner destroyed']);
1442+
};
1443+
});
13931444
});

0 commit comments

Comments
 (0)