From fff34c352e9c87678f6f5f622037e7d90ad7c6ce Mon Sep 17 00:00:00 2001 From: Rich Harris Date: Tue, 26 Mar 2024 14:19:39 -0400 Subject: [PATCH] WIP --- .../svelte/src/internal/client/constants.js | 1 + .../src/internal/client/dom/blocks/await.js | 14 ++++++------- .../src/internal/client/dom/blocks/each.js | 3 ++- .../src/internal/client/dom/blocks/if.js | 7 ++++--- .../src/internal/client/dom/blocks/key.js | 4 ++-- .../client/dom/blocks/svelte-component.js | 4 ++-- .../client/dom/blocks/svelte-element.js | 3 ++- .../internal/client/dom/blocks/svelte-head.js | 4 ++-- .../src/internal/client/reactivity/effects.js | 8 +++++++- .../svelte/src/internal/client/runtime.js | 20 +++++++++++++++---- 10 files changed, 45 insertions(+), 23 deletions(-) diff --git a/packages/svelte/src/internal/client/constants.js b/packages/svelte/src/internal/client/constants.js index 17917d848a..cf02a67555 100644 --- a/packages/svelte/src/internal/client/constants.js +++ b/packages/svelte/src/internal/client/constants.js @@ -2,6 +2,7 @@ export const DERIVED = 1 << 1; export const EFFECT = 1 << 2; export const PRE_EFFECT = 1 << 3; export const RENDER_EFFECT = 1 << 4; +export const BLOCK_EFFECT = 1 << 4; export const MANAGED = 1 << 6; export const UNOWNED = 1 << 7; export const CLEAN = 1 << 8; diff --git a/packages/svelte/src/internal/client/dom/blocks/await.js b/packages/svelte/src/internal/client/dom/blocks/await.js index 6afb629e2f..0993933aa1 100644 --- a/packages/svelte/src/internal/client/dom/blocks/await.js +++ b/packages/svelte/src/internal/client/dom/blocks/await.js @@ -7,7 +7,7 @@ import { set_current_effect, set_current_reaction } from '../../runtime.js'; -import { destroy_effect, pause_effect, render_effect } from '../../reactivity/effects.js'; +import { block, destroy_effect, pause_effect, render_effect } from '../../reactivity/effects.js'; import { INERT } from '../../constants.js'; /** @@ -39,10 +39,10 @@ export function await_block(anchor, get_input, pending_fn, then_fn, catch_fn) { * @param {any} value */ function create_effect(fn, value) { - set_current_effect(branch); - set_current_reaction(branch); // TODO do we need both? + set_current_effect(effect); + set_current_reaction(effect); // TODO do we need both? set_current_component_context(component_context); - var effect = render_effect(() => fn(anchor, value), true); + var e = render_effect(() => fn(anchor, value), true); set_current_component_context(null); set_current_reaction(null); set_current_effect(null); @@ -51,10 +51,10 @@ export function await_block(anchor, get_input, pending_fn, then_fn, catch_fn) { // resolves which is unexpected behaviour (and somewhat irksome to test) flushSync(); - return effect; + return e; } - const branch = render_effect(() => { + const effect = block(() => { if (input === (input = get_input())) return; if (is_promise(input)) { @@ -105,7 +105,7 @@ export function await_block(anchor, get_input, pending_fn, then_fn, catch_fn) { } }); - branch.ondestroy = () => { + effect.ondestroy = () => { // TODO this sucks, tidy it up if (pending_effect?.dom) remove(pending_effect.dom); if (then_effect?.dom) remove(then_effect.dom); diff --git a/packages/svelte/src/internal/client/dom/blocks/each.js b/packages/svelte/src/internal/client/dom/blocks/each.js index c23dabffae..b5a15d7ff9 100644 --- a/packages/svelte/src/internal/client/dom/blocks/each.js +++ b/packages/svelte/src/internal/client/dom/blocks/each.js @@ -11,6 +11,7 @@ import { empty } from '../operations.js'; import { insert, remove } from '../reconciler.js'; import { untrack } from '../../runtime.js'; import { + block, destroy_effect, effect, pause_effect, @@ -67,7 +68,7 @@ function each(anchor, flags, get_collection, get_key, render_fn, fallback_fn, re /** @type {import('#client').Effect | null} */ var fallback = null; - var effect = render_effect(() => { + var effect = block(() => { var collection = get_collection(); var array = is_array(collection) diff --git a/packages/svelte/src/internal/client/dom/blocks/if.js b/packages/svelte/src/internal/client/dom/blocks/if.js index d8e252c3f3..8abc244a39 100644 --- a/packages/svelte/src/internal/client/dom/blocks/if.js +++ b/packages/svelte/src/internal/client/dom/blocks/if.js @@ -2,6 +2,7 @@ import { IS_ELSEIF } from '../../constants.js'; import { hydrate_nodes, hydrating, set_hydrating } from '../hydration.js'; import { remove } from '../reconciler.js'; import { + block, destroy_effect, pause_effect, render_effect, @@ -32,7 +33,7 @@ export function if_block( /** @type {boolean | null} */ let condition = null; - const if_effect = render_effect(() => { + const effect = block(() => { if (condition === (condition = !!get_condition())) return; /** Whether or not there was a hydration mismatch. Needs to be a `let` or else it isn't treeshaken out */ @@ -90,10 +91,10 @@ export function if_block( }); if (elseif) { - if_effect.f |= IS_ELSEIF; + effect.f |= IS_ELSEIF; } - if_effect.ondestroy = () => { + effect.ondestroy = () => { // TODO why is this not automatic? this should be children of `if_effect` if (consequent_effect) { destroy_effect(consequent_effect); diff --git a/packages/svelte/src/internal/client/dom/blocks/key.js b/packages/svelte/src/internal/client/dom/blocks/key.js index 66cdb3ed68..f1b4ee3334 100644 --- a/packages/svelte/src/internal/client/dom/blocks/key.js +++ b/packages/svelte/src/internal/client/dom/blocks/key.js @@ -1,6 +1,6 @@ import { UNINITIALIZED } from '../../constants.js'; import { remove } from '../reconciler.js'; -import { pause_effect, render_effect } from '../../reactivity/effects.js'; +import { block, pause_effect, render_effect } from '../../reactivity/effects.js'; import { safe_not_equal } from '../../reactivity/equality.js'; /** @@ -24,7 +24,7 @@ export function key_block(anchor, get_key, render_fn) { */ let effects = new Set(); - const key_effect = render_effect(() => { + const key_effect = block(() => { if (safe_not_equal(key, (key = get_key()))) { if (effect) { var e = effect; diff --git a/packages/svelte/src/internal/client/dom/blocks/svelte-component.js b/packages/svelte/src/internal/client/dom/blocks/svelte-component.js index ff00b90127..76ef1168a4 100644 --- a/packages/svelte/src/internal/client/dom/blocks/svelte-component.js +++ b/packages/svelte/src/internal/client/dom/blocks/svelte-component.js @@ -1,4 +1,4 @@ -import { pause_effect, render_effect } from '../../reactivity/effects.js'; +import { block, pause_effect, render_effect } from '../../reactivity/effects.js'; import { remove } from '../reconciler.js'; import { current_effect } from '../../runtime.js'; @@ -26,7 +26,7 @@ export function component(anchor, get_component, render_fn) { */ let effects = new Set(); - const component_effect = render_effect(() => { + const component_effect = block(() => { if (component === (component = get_component())) return; if (effect) { diff --git a/packages/svelte/src/internal/client/dom/blocks/svelte-element.js b/packages/svelte/src/internal/client/dom/blocks/svelte-element.js index 58645c4eac..3e56e65105 100644 --- a/packages/svelte/src/internal/client/dom/blocks/svelte-element.js +++ b/packages/svelte/src/internal/client/dom/blocks/svelte-element.js @@ -2,6 +2,7 @@ import { namespace_svg } from '../../../../constants.js'; import { hydrate_anchor, hydrate_nodes, hydrating } from '../hydration.js'; import { empty } from '../operations.js'; import { + block, destroy_effect, pause_effect, render_effect, @@ -63,7 +64,7 @@ export function element(anchor, get_tag, is_svg, render_fn) { */ let each_item_block = current_each_item; - const wrapper = render_effect(() => { + const wrapper = block(() => { const next_tag = get_tag() || null; if (next_tag === tag) return; diff --git a/packages/svelte/src/internal/client/dom/blocks/svelte-head.js b/packages/svelte/src/internal/client/dom/blocks/svelte-head.js index dd092b092a..03ee2ec5f0 100644 --- a/packages/svelte/src/internal/client/dom/blocks/svelte-head.js +++ b/packages/svelte/src/internal/client/dom/blocks/svelte-head.js @@ -1,6 +1,6 @@ import { hydrate_anchor, hydrate_nodes, hydrating, set_hydrate_nodes } from '../hydration.js'; import { empty } from '../operations.js'; -import { render_effect } from '../../reactivity/effects.js'; +import { block, render_effect } from '../../reactivity/effects.js'; import { remove } from '../reconciler.js'; /** @@ -30,7 +30,7 @@ export function head(render_fn) { /** @type {import('#client').Dom | null} */ var dom = null; - const head_effect = render_effect(() => { + const head_effect = block(() => { if (dom !== null) { remove(dom); head_effect.dom = dom = null; diff --git a/packages/svelte/src/internal/client/reactivity/effects.js b/packages/svelte/src/internal/client/reactivity/effects.js index b3b76d8c6a..99cc57033a 100644 --- a/packages/svelte/src/internal/client/reactivity/effects.js +++ b/packages/svelte/src/internal/client/reactivity/effects.js @@ -23,7 +23,8 @@ import { DESTROYED, INERT, IS_ELSEIF, - EFFECT_RAN + EFFECT_RAN, + BLOCK_EFFECT } from '../constants.js'; import { set } from './sources.js'; import { noop } from '../../common.js'; @@ -216,6 +217,11 @@ export function render_effect(fn, managed = false) { return create_effect(flags, fn, true); } +/** @param {(() => void)} fn */ +export function block(fn) { + return create_effect(BLOCK_EFFECT, fn, true); +} + /** * @param {import('#client').Effect} effect * @returns {void} diff --git a/packages/svelte/src/internal/client/runtime.js b/packages/svelte/src/internal/client/runtime.js index 707f4d9619..22f7293ab9 100644 --- a/packages/svelte/src/internal/client/runtime.js +++ b/packages/svelte/src/internal/client/runtime.js @@ -21,7 +21,8 @@ import { DESTROYED, INERT, MANAGED, - STATE_SYMBOL + STATE_SYMBOL, + BLOCK_EFFECT } from './constants.js'; import { flush_tasks } from './dom/task.js'; import { add_owner } from './dev/ownership.js'; @@ -359,6 +360,12 @@ export function destroy_children(signal) { if (signal.effects) { for (var i = 0; i < signal.effects.length; i += 1) { var effect = signal.effects[i]; + + // TODO figure out why we need this `if` condition. if we remove it, + // the only test that fails relates to root effects (is there a reasaon + // we don't want to destroy root effects when their parent effects are + // updated? seems leaky), but it looks like there are other managed + // effects aside from the immediate children of blocks if ((effect.f & MANAGED) === 0) { destroy_effect(effect); } @@ -379,7 +386,9 @@ export function destroy_children(signal) { * @returns {void} */ export function execute_effect(effect) { - if ((effect.f & DESTROYED) !== 0) { + var flags = effect.f; + + if ((flags & DESTROYED) !== 0) { return; } @@ -394,7 +403,10 @@ export function execute_effect(effect) { current_component_context = component_context; try { - destroy_children(effect); + if ((flags & BLOCK_EFFECT) === 0) { + destroy_children(effect); + } + effect.teardown?.(); var teardown = execute_reaction_fn(effect); effect.teardown = typeof teardown === 'function' ? teardown : null; @@ -404,7 +416,7 @@ export function execute_effect(effect) { } const parent = effect.parent; - if ((effect.f & PRE_EFFECT) !== 0 && parent !== null) { + if ((flags & PRE_EFFECT) !== 0 && parent !== null) { flush_local_pre_effects(parent); } }