From 1d1e4aeeb11580d0d94d6ae480ed66ae85206acd Mon Sep 17 00:00:00 2001 From: Rich Harris Date: Wed, 18 Sep 2024 15:13:55 -0400 Subject: [PATCH] fix: attach effects-inside-deriveds to the parent of the derived (#13309) * WIP * fix types * ensure effects with deriveds are added to the tree * test * changeset * oops --- .changeset/plenty-rings-stare.md | 5 +++ .../svelte/src/internal/client/constants.js | 1 + .../internal/client/reactivity/deriveds.js | 37 ++++++++++++++++--- .../src/internal/client/reactivity/effects.js | 6 ++- .../src/internal/client/reactivity/types.d.ts | 2 +- .../samples/effect-inside-derived/_config.js | 16 ++++++++ .../samples/effect-inside-derived/main.svelte | 19 ++++++++++ 7 files changed, 77 insertions(+), 9 deletions(-) create mode 100644 .changeset/plenty-rings-stare.md create mode 100644 packages/svelte/tests/runtime-runes/samples/effect-inside-derived/_config.js create mode 100644 packages/svelte/tests/runtime-runes/samples/effect-inside-derived/main.svelte diff --git a/.changeset/plenty-rings-stare.md b/.changeset/plenty-rings-stare.md new file mode 100644 index 0000000000..2e9dfa5f48 --- /dev/null +++ b/.changeset/plenty-rings-stare.md @@ -0,0 +1,5 @@ +--- +'svelte': patch +--- + +fix: attach effects-inside-deriveds to the parent of the derived diff --git a/packages/svelte/src/internal/client/constants.js b/packages/svelte/src/internal/client/constants.js index 35ee8292ef..e5d3904689 100644 --- a/packages/svelte/src/internal/client/constants.js +++ b/packages/svelte/src/internal/client/constants.js @@ -19,6 +19,7 @@ export const LEGACY_DERIVED_PROP = 1 << 16; export const INSPECT_EFFECT = 1 << 17; export const HEAD_EFFECT = 1 << 18; export const EFFECT_QUEUED = 1 << 19; +export const EFFECT_HAS_DERIVED = 1 << 20; export const STATE_SYMBOL = Symbol('$state'); export const STATE_SYMBOL_METADATA = Symbol('$state metadata'); diff --git a/packages/svelte/src/internal/client/reactivity/deriveds.js b/packages/svelte/src/internal/client/reactivity/deriveds.js index 66f11cda7c..d632840bfa 100644 --- a/packages/svelte/src/internal/client/reactivity/deriveds.js +++ b/packages/svelte/src/internal/client/reactivity/deriveds.js @@ -1,6 +1,14 @@ /** @import { Derived, Effect } from '#client' */ import { DEV } from 'esm-env'; -import { CLEAN, DERIVED, DESTROYED, DIRTY, MAYBE_DIRTY, UNOWNED } from '../constants.js'; +import { + CLEAN, + DERIVED, + DESTROYED, + DIRTY, + EFFECT_HAS_DERIVED, + MAYBE_DIRTY, + UNOWNED +} from '../constants.js'; import { active_reaction, active_effect, @@ -8,7 +16,8 @@ import { set_signal_status, skip_reaction, update_reaction, - increment_version + increment_version, + set_active_effect } from '../runtime.js'; import { equals, safe_equals } from './equality.js'; import * as e from '../errors.js'; @@ -23,7 +32,14 @@ import { inspect_effects, set_inspect_effects } from './sources.js'; /*#__NO_SIDE_EFFECTS__*/ export function derived(fn) { let flags = DERIVED | DIRTY; - if (active_effect === null) flags |= UNOWNED; + + if (active_effect === null) { + flags |= UNOWNED; + } else { + // Since deriveds are evaluated lazily, any effects created inside them are + // created too late to ensure that the parent effect is added to the tree + active_effect.f |= EFFECT_HAS_DERIVED; + } /** @type {Derived} */ const signal = { @@ -34,7 +50,8 @@ export function derived(fn) { fn, reactions: null, v: /** @type {V} */ (null), - version: 0 + version: 0, + parent: active_effect }; if (active_reaction !== null && (active_reaction.f & DERIVED) !== 0) { @@ -91,6 +108,9 @@ let stack = []; */ export function update_derived(derived) { var value; + var prev_active_effect = active_effect; + + set_active_effect(derived.parent); if (DEV) { let prev_inspect_effects = inspect_effects; @@ -105,12 +125,17 @@ export function update_derived(derived) { destroy_derived_children(derived); value = update_reaction(derived); } finally { + set_active_effect(prev_active_effect); set_inspect_effects(prev_inspect_effects); stack.pop(); } } else { - destroy_derived_children(derived); - value = update_reaction(derived); + try { + destroy_derived_children(derived); + value = update_reaction(derived); + } finally { + set_active_effect(prev_active_effect); + } } var status = diff --git a/packages/svelte/src/internal/client/reactivity/effects.js b/packages/svelte/src/internal/client/reactivity/effects.js index 431ea1eca8..b933c3d033 100644 --- a/packages/svelte/src/internal/client/reactivity/effects.js +++ b/packages/svelte/src/internal/client/reactivity/effects.js @@ -34,7 +34,8 @@ import { CLEAN, INSPECT_EFFECT, HEAD_EFFECT, - MAYBE_DIRTY + MAYBE_DIRTY, + EFFECT_HAS_DERIVED } from '../constants.js'; import { set } from './sources.js'; import * as e from '../errors.js'; @@ -138,7 +139,8 @@ function create_effect(type, fn, sync, push = true) { effect.deps === null && effect.first === null && effect.nodes_start === null && - effect.teardown === null; + effect.teardown === null && + (effect.f & EFFECT_HAS_DERIVED) === 0; if (!inert && !is_root && push) { if (parent_effect !== null) { diff --git a/packages/svelte/src/internal/client/reactivity/types.d.ts b/packages/svelte/src/internal/client/reactivity/types.d.ts index ecc3029580..8742905842 100644 --- a/packages/svelte/src/internal/client/reactivity/types.d.ts +++ b/packages/svelte/src/internal/client/reactivity/types.d.ts @@ -21,6 +21,7 @@ export interface Reaction extends Signal { fn: null | Function; /** Signals that this signal reads from */ deps: null | Value[]; + parent: Effect | null; } export interface Derived extends Value, Reaction { @@ -31,7 +32,6 @@ export interface Derived extends Value, Reaction { } export interface Effect extends Reaction { - parent: Effect | null; /** * Branch effects store their start/end nodes so that they can be * removed when the effect is destroyed, or moved when an `each` diff --git a/packages/svelte/tests/runtime-runes/samples/effect-inside-derived/_config.js b/packages/svelte/tests/runtime-runes/samples/effect-inside-derived/_config.js new file mode 100644 index 0000000000..34597fb66c --- /dev/null +++ b/packages/svelte/tests/runtime-runes/samples/effect-inside-derived/_config.js @@ -0,0 +1,16 @@ +import { flushSync } from 'svelte'; +import { test } from '../../test'; + +export default test({ + html: '', + + test({ assert, target }) { + const button = target.querySelector('button'); + + flushSync(() => button?.click()); + assert.htmlEqual(target.innerHTML, ''); + + flushSync(() => button?.click()); + assert.htmlEqual(target.innerHTML, ''); + } +}); diff --git a/packages/svelte/tests/runtime-runes/samples/effect-inside-derived/main.svelte b/packages/svelte/tests/runtime-runes/samples/effect-inside-derived/main.svelte new file mode 100644 index 0000000000..bed341da91 --- /dev/null +++ b/packages/svelte/tests/runtime-runes/samples/effect-inside-derived/main.svelte @@ -0,0 +1,19 @@ + + +