Skip to content

Commit 3fe6813

Browse files
committed
fix: ensure effects destroy owned deriveds upon teardown
1 parent 531ff62 commit 3fe6813

File tree

8 files changed

+69
-24
lines changed

8 files changed

+69
-24
lines changed

.changeset/ninety-months-laugh.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 effects destroy owned deriveds upon teardown

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

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -58,6 +58,9 @@ export function derived(fn) {
5858
var derived = /** @type {Derived} */ (active_reaction);
5959
(derived.children ??= []).push(signal);
6060
}
61+
if (active_effect !== null) {
62+
(active_effect.deriveds ??= []).push(signal);
63+
}
6164

6265
return signal;
6366
}
@@ -153,7 +156,7 @@ export function update_derived(derived) {
153156
* @param {Derived} signal
154157
* @returns {void}
155158
*/
156-
function destroy_derived(signal) {
159+
export function destroy_derived(signal) {
157160
destroy_derived_children(signal);
158161
remove_reactions(signal, 0);
159162
set_signal_status(signal, DESTROYED);

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

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -97,6 +97,7 @@ function create_effect(type, fn, sync, push = true) {
9797
var effect = {
9898
ctx: component_context,
9999
deps: null,
100+
deriveds: null,
100101
nodes_start: null,
101102
nodes_end: null,
102103
f: type | DIRTY,

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

Lines changed: 45 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -10,10 +10,17 @@ import {
1010
import { get_descriptor, is_function } from '../../shared/utils.js';
1111
import { mutable_source, set, source } from './sources.js';
1212
import { derived, derived_safe_equal } from './deriveds.js';
13-
import { get, is_signals_recorded, untrack, update } from '../runtime.js';
13+
import {
14+
active_effect,
15+
get,
16+
is_signals_recorded,
17+
set_active_effect,
18+
untrack,
19+
update
20+
} from '../runtime.js';
1421
import { safe_equals } from './equality.js';
1522
import * as e from '../errors.js';
16-
import { LEGACY_DERIVED_PROP } from '../constants.js';
23+
import { BRANCH_EFFECT, LEGACY_DERIVED_PROP, ROOT_EFFECT } from '../constants.js';
1724
import { proxy } from '../proxy.js';
1825

1926
/**
@@ -217,6 +224,26 @@ export function spread_props(...props) {
217224
return new Proxy({ props }, spread_props_handler);
218225
}
219226

227+
/**
228+
* @template T
229+
* @param {() => T} fn
230+
* @returns {T}
231+
*/
232+
function with_parent_branch(fn) {
233+
var effect = active_effect;
234+
var previous_effect = active_effect;
235+
236+
while (effect !== null && (effect.f & (BRANCH_EFFECT | ROOT_EFFECT)) === 0) {
237+
effect = effect.parent;
238+
}
239+
try {
240+
set_active_effect(effect);
241+
return fn();
242+
} finally {
243+
set_active_effect(previous_effect);
244+
}
245+
}
246+
220247
/**
221248
* This function is responsible for synchronizing a possibly bound prop with the inner component state.
222249
* It is used whenever the compiler sees that the component writes to the prop, or when it has a default prop_value.
@@ -276,8 +303,8 @@ export function prop(props, key, flags, fallback) {
276303
} else {
277304
// Svelte 4 did not trigger updates when a primitive value was updated to the same value.
278305
// Replicate that behavior through using a derived
279-
var derived_getter = (immutable ? derived : derived_safe_equal)(
280-
() => /** @type {V} */ (props[key])
306+
var derived_getter = with_parent_branch(() =>
307+
(immutable ? derived : derived_safe_equal)(() => /** @type {V} */ (props[key]))
281308
);
282309
derived_getter.f |= LEGACY_DERIVED_PROP;
283310
getter = () => {
@@ -321,19 +348,21 @@ export function prop(props, key, flags, fallback) {
321348
// The derived returns the current value. The underlying mutable
322349
// source is written to from various places to persist this value.
323350
var inner_current_value = mutable_source(prop_value);
324-
var current_value = derived(() => {
325-
var parent_value = getter();
326-
var child_value = get(inner_current_value);
327-
328-
if (from_child) {
329-
from_child = false;
330-
was_from_child = true;
331-
return child_value;
332-
}
351+
var current_value = with_parent_branch(() =>
352+
derived(() => {
353+
var parent_value = getter();
354+
var child_value = get(inner_current_value);
355+
356+
if (from_child) {
357+
from_child = false;
358+
was_from_child = true;
359+
return child_value;
360+
}
333361

334-
was_from_child = false;
335-
return (inner_current_value.v = parent_value);
336-
});
362+
was_from_child = false;
363+
return (inner_current_value.v = parent_value);
364+
})
365+
);
337366

338367
if (!immutable) current_value.equals = safe_equals;
339368

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

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -42,6 +42,8 @@ export interface Effect extends Reaction {
4242
nodes_end: null | TemplateNode;
4343
/** The associated component context */
4444
ctx: null | ComponentContext;
45+
/** Reactions created inside this signal */
46+
deriveds: null | Derived[];
4547
/** The effect function */
4648
fn: null | (() => void | (() => void));
4749
/** The teardown function returned from the effect function */

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

Lines changed: 11 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -27,7 +27,7 @@ import {
2727
import { flush_tasks } from './dom/task.js';
2828
import { add_owner } from './dev/ownership.js';
2929
import { mutate, set, source } from './reactivity/sources.js';
30-
import { update_derived } from './reactivity/deriveds.js';
30+
import { destroy_derived, update_derived } from './reactivity/deriveds.js';
3131
import * as e from './errors.js';
3232
import { lifecycle_outside_component } from '../shared/errors.js';
3333
import { FILENAME } from '../../constants.js';
@@ -408,6 +408,16 @@ export function remove_reactions(signal, start_index) {
408408
* @returns {void}
409409
*/
410410
export function destroy_effect_children(signal, remove_dom = false) {
411+
var deriveds = signal.deriveds;
412+
413+
if (deriveds !== null) {
414+
signal.deriveds = null;
415+
416+
for (var i = 0; i < deriveds.length; i += 1) {
417+
destroy_derived(deriveds[i]);
418+
}
419+
}
420+
411421
var effect = signal.first;
412422
signal.first = signal.last = null;
413423

packages/svelte/tests/runtime-runes/samples/derived-unowned-3/main.svelte

Lines changed: 0 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,4 @@
11
<script>
2-
import { untrack } from 'svelte';
3-
42
class Model {
53
data = $state();
64
@@ -28,9 +26,6 @@
2826
$effect(() => {
2927
if(needsSet) {
3028
setModel('effect');
31-
untrack(() => {
32-
needsSet = false;
33-
});
3429
}
3530
});
3631

packages/svelte/tests/signals/test.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -488,7 +488,7 @@ describe('signals', () => {
488488
assert.equal(a?.deps?.length, 1);
489489
assert.equal(s?.reactions?.length, 1);
490490
destroy();
491-
assert.equal(a?.deps?.length, 1);
491+
assert.equal(a?.deps, null);
492492
assert.equal(s?.reactions, null);
493493
};
494494
});

0 commit comments

Comments
 (0)