From bce1c4898c1460926662f47aeff1588681720806 Mon Sep 17 00:00:00 2001 From: Dominic Gannaway Date: Wed, 30 Oct 2024 19:35:06 +0000 Subject: [PATCH] fix: ensure child effects are destroyed before their deriveds (#14043) * fix: ensure legacy props cache last value when destroyed * fix runes * better approach * better approach * kill code * lint --- .changeset/khaki-carrots-mix.md | 5 +++++ .../src/internal/client/reactivity/effects.js | 2 +- .../src/internal/client/reactivity/props.js | 12 +++--------- packages/svelte/src/internal/client/runtime.js | 2 +- .../samples/props-reactive-destroy/Child.svelte | 11 +++++++++++ .../samples/props-reactive-destroy/_config.js | 10 ++++++++++ .../samples/props-reactive-destroy/main.svelte | 17 +++++++++++++++++ .../samples/props-reactive-destroy/Child.svelte | 11 +++++++++++ .../samples/props-reactive-destroy/_config.js | 10 ++++++++++ .../samples/props-reactive-destroy/main.svelte | 17 +++++++++++++++++ 10 files changed, 86 insertions(+), 11 deletions(-) create mode 100644 .changeset/khaki-carrots-mix.md create mode 100644 packages/svelte/tests/runtime-legacy/samples/props-reactive-destroy/Child.svelte create mode 100644 packages/svelte/tests/runtime-legacy/samples/props-reactive-destroy/_config.js create mode 100644 packages/svelte/tests/runtime-legacy/samples/props-reactive-destroy/main.svelte create mode 100644 packages/svelte/tests/runtime-runes/samples/props-reactive-destroy/Child.svelte create mode 100644 packages/svelte/tests/runtime-runes/samples/props-reactive-destroy/_config.js create mode 100644 packages/svelte/tests/runtime-runes/samples/props-reactive-destroy/main.svelte diff --git a/.changeset/khaki-carrots-mix.md b/.changeset/khaki-carrots-mix.md new file mode 100644 index 0000000000..f5ef6b6418 --- /dev/null +++ b/.changeset/khaki-carrots-mix.md @@ -0,0 +1,5 @@ +--- +'svelte': patch +--- + +fix: ensure child effects are destroyed before their deriveds diff --git a/packages/svelte/src/internal/client/reactivity/effects.js b/packages/svelte/src/internal/client/reactivity/effects.js index 82baeae288..daf28eeec2 100644 --- a/packages/svelte/src/internal/client/reactivity/effects.js +++ b/packages/svelte/src/internal/client/reactivity/effects.js @@ -437,8 +437,8 @@ export function destroy_effect(effect, remove_dom = true) { removed = true; } - destroy_effect_deriveds(effect); destroy_effect_children(effect, remove_dom && !removed); + destroy_effect_deriveds(effect); remove_reactions(effect, 0); set_signal_status(effect, DESTROYED); diff --git a/packages/svelte/src/internal/client/reactivity/props.js b/packages/svelte/src/internal/client/reactivity/props.js index eec8e70dc6..e24deb5a20 100644 --- a/packages/svelte/src/internal/client/reactivity/props.js +++ b/packages/svelte/src/internal/client/reactivity/props.js @@ -1,4 +1,4 @@ -/** @import { Derived, Source } from './types.js' */ +/** @import { Source } from './types.js' */ import { DEV } from 'esm-env'; import { PROPS_IS_BINDABLE, @@ -12,7 +12,6 @@ import { mutable_source, set, source } from './sources.js'; import { derived, derived_safe_equal } from './deriveds.js'; import { active_effect, - active_reaction, get, is_signals_recorded, set_active_effect, @@ -21,7 +20,7 @@ import { } from '../runtime.js'; import { safe_equals } from './equality.js'; import * as e from '../errors.js'; -import { BRANCH_EFFECT, DESTROYED, LEGACY_DERIVED_PROP, ROOT_EFFECT } from '../constants.js'; +import { BRANCH_EFFECT, LEGACY_DERIVED_PROP, ROOT_EFFECT } from '../constants.js'; import { proxy } from '../proxy.js'; import { capture_store_binding } from './store.js'; @@ -369,17 +368,12 @@ 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 = with_parent_branch(() => derived(() => { var parent_value = getter(); var child_value = get(inner_current_value); - var current_derived = /** @type {Derived} */ (active_reaction); - // If the getter from the parent returns undefined, switch - // to using the local value from inner_current_value instead, - // as the parent value might have been torn down - if (from_child || (parent_value === undefined && (current_derived.f & DESTROYED) !== 0)) { + if (from_child) { from_child = false; was_from_child = true; return child_value; diff --git a/packages/svelte/src/internal/client/runtime.js b/packages/svelte/src/internal/client/runtime.js index 26c8d9adbb..6bed3825c7 100644 --- a/packages/svelte/src/internal/client/runtime.js +++ b/packages/svelte/src/internal/client/runtime.js @@ -432,12 +432,12 @@ export function update_effect(effect) { } try { - destroy_effect_deriveds(effect); if ((flags & BLOCK_EFFECT) !== 0) { destroy_block_effect_children(effect); } else { destroy_effect_children(effect); } + destroy_effect_deriveds(effect); execute_effect_teardown(effect); var teardown = update_reaction(effect); diff --git a/packages/svelte/tests/runtime-legacy/samples/props-reactive-destroy/Child.svelte b/packages/svelte/tests/runtime-legacy/samples/props-reactive-destroy/Child.svelte new file mode 100644 index 0000000000..dc6326f8c4 --- /dev/null +++ b/packages/svelte/tests/runtime-legacy/samples/props-reactive-destroy/Child.svelte @@ -0,0 +1,11 @@ + + +{data ? '' : null} \ No newline at end of file diff --git a/packages/svelte/tests/runtime-legacy/samples/props-reactive-destroy/_config.js b/packages/svelte/tests/runtime-legacy/samples/props-reactive-destroy/_config.js new file mode 100644 index 0000000000..363c850c8b --- /dev/null +++ b/packages/svelte/tests/runtime-legacy/samples/props-reactive-destroy/_config.js @@ -0,0 +1,10 @@ +import { flushSync } from 'svelte'; +import { test } from '../../test'; + +export default test({ + test({ assert, logs, target }) { + target.querySelector('button')?.click(); + flushSync(); + assert.deepEqual(logs, ['should fire once']); + } +}); diff --git a/packages/svelte/tests/runtime-legacy/samples/props-reactive-destroy/main.svelte b/packages/svelte/tests/runtime-legacy/samples/props-reactive-destroy/main.svelte new file mode 100644 index 0000000000..00c6a5f71c --- /dev/null +++ b/packages/svelte/tests/runtime-legacy/samples/props-reactive-destroy/main.svelte @@ -0,0 +1,17 @@ + + + + +{#if active} + +{/if} \ No newline at end of file diff --git a/packages/svelte/tests/runtime-runes/samples/props-reactive-destroy/Child.svelte b/packages/svelte/tests/runtime-runes/samples/props-reactive-destroy/Child.svelte new file mode 100644 index 0000000000..d16689b07d --- /dev/null +++ b/packages/svelte/tests/runtime-runes/samples/props-reactive-destroy/Child.svelte @@ -0,0 +1,11 @@ + + +{data ? '' : null} diff --git a/packages/svelte/tests/runtime-runes/samples/props-reactive-destroy/_config.js b/packages/svelte/tests/runtime-runes/samples/props-reactive-destroy/_config.js new file mode 100644 index 0000000000..363c850c8b --- /dev/null +++ b/packages/svelte/tests/runtime-runes/samples/props-reactive-destroy/_config.js @@ -0,0 +1,10 @@ +import { flushSync } from 'svelte'; +import { test } from '../../test'; + +export default test({ + test({ assert, logs, target }) { + target.querySelector('button')?.click(); + flushSync(); + assert.deepEqual(logs, ['should fire once']); + } +}); diff --git a/packages/svelte/tests/runtime-runes/samples/props-reactive-destroy/main.svelte b/packages/svelte/tests/runtime-runes/samples/props-reactive-destroy/main.svelte new file mode 100644 index 0000000000..604237072a --- /dev/null +++ b/packages/svelte/tests/runtime-runes/samples/props-reactive-destroy/main.svelte @@ -0,0 +1,17 @@ + + + + +{#if active} + +{/if}