From 3fe681380d0b163b3a1c077860865b78e528f42e Mon Sep 17 00:00:00 2001 From: Dominic Gannaway Date: Thu, 10 Oct 2024 21:43:57 +0100 Subject: [PATCH 1/6] fix: ensure effects destroy owned deriveds upon teardown --- .changeset/ninety-months-laugh.md | 5 ++ .../internal/client/reactivity/deriveds.js | 5 +- .../src/internal/client/reactivity/effects.js | 1 + .../src/internal/client/reactivity/props.js | 61 ++++++++++++++----- .../src/internal/client/reactivity/types.d.ts | 2 + .../svelte/src/internal/client/runtime.js | 12 +++- .../samples/derived-unowned-3/main.svelte | 5 -- packages/svelte/tests/signals/test.ts | 2 +- 8 files changed, 69 insertions(+), 24 deletions(-) create mode 100644 .changeset/ninety-months-laugh.md diff --git a/.changeset/ninety-months-laugh.md b/.changeset/ninety-months-laugh.md new file mode 100644 index 000000000000..eff716c7602c --- /dev/null +++ b/.changeset/ninety-months-laugh.md @@ -0,0 +1,5 @@ +--- +'svelte': patch +--- + +fix: ensure effects destroy owned deriveds upon teardown diff --git a/packages/svelte/src/internal/client/reactivity/deriveds.js b/packages/svelte/src/internal/client/reactivity/deriveds.js index d632840bfa34..46f16d7d196e 100644 --- a/packages/svelte/src/internal/client/reactivity/deriveds.js +++ b/packages/svelte/src/internal/client/reactivity/deriveds.js @@ -58,6 +58,9 @@ export function derived(fn) { var derived = /** @type {Derived} */ (active_reaction); (derived.children ??= []).push(signal); } + if (active_effect !== null) { + (active_effect.deriveds ??= []).push(signal); + } return signal; } @@ -153,7 +156,7 @@ export function update_derived(derived) { * @param {Derived} signal * @returns {void} */ -function destroy_derived(signal) { +export function destroy_derived(signal) { destroy_derived_children(signal); remove_reactions(signal, 0); set_signal_status(signal, DESTROYED); diff --git a/packages/svelte/src/internal/client/reactivity/effects.js b/packages/svelte/src/internal/client/reactivity/effects.js index b5955e9b32c0..73286861d2b5 100644 --- a/packages/svelte/src/internal/client/reactivity/effects.js +++ b/packages/svelte/src/internal/client/reactivity/effects.js @@ -97,6 +97,7 @@ function create_effect(type, fn, sync, push = true) { var effect = { ctx: component_context, deps: null, + deriveds: null, nodes_start: null, nodes_end: null, f: type | DIRTY, diff --git a/packages/svelte/src/internal/client/reactivity/props.js b/packages/svelte/src/internal/client/reactivity/props.js index 7cebd85c45ec..b817ace3f0aa 100644 --- a/packages/svelte/src/internal/client/reactivity/props.js +++ b/packages/svelte/src/internal/client/reactivity/props.js @@ -10,10 +10,17 @@ import { import { get_descriptor, is_function } from '../../shared/utils.js'; import { mutable_source, set, source } from './sources.js'; import { derived, derived_safe_equal } from './deriveds.js'; -import { get, is_signals_recorded, untrack, update } from '../runtime.js'; +import { + active_effect, + get, + is_signals_recorded, + set_active_effect, + untrack, + update +} from '../runtime.js'; import { safe_equals } from './equality.js'; import * as e from '../errors.js'; -import { LEGACY_DERIVED_PROP } from '../constants.js'; +import { BRANCH_EFFECT, LEGACY_DERIVED_PROP, ROOT_EFFECT } from '../constants.js'; import { proxy } from '../proxy.js'; /** @@ -217,6 +224,26 @@ export function spread_props(...props) { return new Proxy({ props }, spread_props_handler); } +/** + * @template T + * @param {() => T} fn + * @returns {T} + */ +function with_parent_branch(fn) { + var effect = active_effect; + var previous_effect = active_effect; + + while (effect !== null && (effect.f & (BRANCH_EFFECT | ROOT_EFFECT)) === 0) { + effect = effect.parent; + } + try { + set_active_effect(effect); + return fn(); + } finally { + set_active_effect(previous_effect); + } +} + /** * This function is responsible for synchronizing a possibly bound prop with the inner component state. * 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) { } else { // Svelte 4 did not trigger updates when a primitive value was updated to the same value. // Replicate that behavior through using a derived - var derived_getter = (immutable ? derived : derived_safe_equal)( - () => /** @type {V} */ (props[key]) + var derived_getter = with_parent_branch(() => + (immutable ? derived : derived_safe_equal)(() => /** @type {V} */ (props[key])) ); derived_getter.f |= LEGACY_DERIVED_PROP; getter = () => { @@ -321,19 +348,21 @@ export function prop(props, key, flags, fallback) { // The derived returns the current value. The underlying mutable // source is written to from various places to persist this value. var inner_current_value = mutable_source(prop_value); - var current_value = derived(() => { - var parent_value = getter(); - var child_value = get(inner_current_value); - - if (from_child) { - from_child = false; - was_from_child = true; - return child_value; - } + var current_value = with_parent_branch(() => + derived(() => { + var parent_value = getter(); + var child_value = get(inner_current_value); + + if (from_child) { + from_child = false; + was_from_child = true; + return child_value; + } - was_from_child = false; - return (inner_current_value.v = parent_value); - }); + was_from_child = false; + return (inner_current_value.v = parent_value); + }) + ); if (!immutable) current_value.equals = safe_equals; diff --git a/packages/svelte/src/internal/client/reactivity/types.d.ts b/packages/svelte/src/internal/client/reactivity/types.d.ts index 874290584272..c9626c394185 100644 --- a/packages/svelte/src/internal/client/reactivity/types.d.ts +++ b/packages/svelte/src/internal/client/reactivity/types.d.ts @@ -42,6 +42,8 @@ export interface Effect extends Reaction { nodes_end: null | TemplateNode; /** The associated component context */ ctx: null | ComponentContext; + /** Reactions created inside this signal */ + deriveds: null | Derived[]; /** The effect function */ fn: null | (() => void | (() => void)); /** The teardown function returned from the effect function */ diff --git a/packages/svelte/src/internal/client/runtime.js b/packages/svelte/src/internal/client/runtime.js index 2a159724a687..6779727d25af 100644 --- a/packages/svelte/src/internal/client/runtime.js +++ b/packages/svelte/src/internal/client/runtime.js @@ -27,7 +27,7 @@ import { import { flush_tasks } from './dom/task.js'; import { add_owner } from './dev/ownership.js'; import { mutate, set, source } from './reactivity/sources.js'; -import { update_derived } from './reactivity/deriveds.js'; +import { destroy_derived, update_derived } from './reactivity/deriveds.js'; import * as e from './errors.js'; import { lifecycle_outside_component } from '../shared/errors.js'; import { FILENAME } from '../../constants.js'; @@ -408,6 +408,16 @@ export function remove_reactions(signal, start_index) { * @returns {void} */ export function destroy_effect_children(signal, remove_dom = false) { + var deriveds = signal.deriveds; + + if (deriveds !== null) { + signal.deriveds = null; + + for (var i = 0; i < deriveds.length; i += 1) { + destroy_derived(deriveds[i]); + } + } + var effect = signal.first; signal.first = signal.last = null; diff --git a/packages/svelte/tests/runtime-runes/samples/derived-unowned-3/main.svelte b/packages/svelte/tests/runtime-runes/samples/derived-unowned-3/main.svelte index 7c32ed0513a2..eb6d908cd4eb 100644 --- a/packages/svelte/tests/runtime-runes/samples/derived-unowned-3/main.svelte +++ b/packages/svelte/tests/runtime-runes/samples/derived-unowned-3/main.svelte @@ -1,6 +1,4 @@