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..b273e4c5183f 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; } @@ -103,10 +106,11 @@ function destroy_derived_children(derived) { let stack = []; /** + * @template T * @param {Derived} derived - * @returns {void} + * @returns {T} */ -export function update_derived(derived) { +export function execute_derived(derived) { var value; var prev_active_effect = active_effect; @@ -138,6 +142,15 @@ export function update_derived(derived) { } } + return value; +} + +/** + * @param {Derived} derived + * @returns {void} + */ +export function update_derived(derived) { + var value = execute_derived(derived); var status = (skip_reaction || (derived.f & UNOWNED) !== 0) && derived.deps !== null ? MAYBE_DIRTY : CLEAN; @@ -153,17 +166,11 @@ 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); // TODO we need to ensure we remove the derived from any parent derives - - signal.children = - signal.deps = - signal.reactions = - // @ts-expect-error `signal.fn` cannot be `null` while the signal is alive - signal.fn = - null; + signal.v = signal.children = signal.deps = signal.reactions = null; } 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..6f0131b8ea9d 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, execute_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'; @@ -300,12 +300,13 @@ export function update_reaction(reaction) { var previous_reaction = active_reaction; var previous_skip_reaction = skip_reaction; var prev_derived_sources = derived_sources; + var flags = reaction.f; new_deps = /** @type {null | Value[]} */ (null); skipped_deps = 0; untracked_writes = null; - active_reaction = (reaction.f & (BRANCH_EFFECT | ROOT_EFFECT)) === 0 ? reaction : null; - skip_reaction = !is_flushing_effect && (reaction.f & UNOWNED) !== 0; + active_reaction = (flags & (BRANCH_EFFECT | ROOT_EFFECT)) === 0 ? reaction : null; + skip_reaction = !is_flushing_effect && (flags & UNOWNED) !== 0; derived_sources = null; try { @@ -408,6 +409,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; @@ -729,9 +740,15 @@ export async function tick() { */ export function get(signal) { var flags = signal.f; - - if ((flags & DESTROYED) !== 0) { - return signal.v; + var is_derived = (flags & DERIVED) !== 0; + + // If the derived is destroyed, just execute it again without retaining + // its memoisation properties as the derived is stale + if (is_derived && (flags & DESTROYED) !== 0) { + var value = execute_derived(/** @type {Derived} */ (signal)); + // Ensure the derived remains destroyed + destroy_derived(/** @type {Derived} */ (signal)); + return value; } if (is_signals_recorded) { @@ -768,7 +785,7 @@ export function get(signal) { } } - if ((flags & DERIVED) !== 0) { + if (is_derived) { var derived = /** @type {Derived} */ (signal); if (check_dirtiness(derived)) { 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..f9dafd047097 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 @@