From ffc4f1b03737f4d5ddf2acd0c731ff272cdea044 Mon Sep 17 00:00:00 2001 From: Rich Harris Date: Thu, 30 Jan 2025 16:32:20 -0500 Subject: [PATCH] mostly working --- .../internal/client/dom/blocks/boundary.js | 88 +++++++++++++++++-- .../src/internal/client/dom/blocks/if.js | 6 +- .../src/internal/client/reactivity/sources.js | 16 ---- .../svelte/src/internal/client/runtime.js | 46 +++++----- 4 files changed, 106 insertions(+), 50 deletions(-) diff --git a/packages/svelte/src/internal/client/dom/blocks/boundary.js b/packages/svelte/src/internal/client/dom/blocks/boundary.js index fc47309534..c285f0fb77 100644 --- a/packages/svelte/src/internal/client/dom/blocks/boundary.js +++ b/packages/svelte/src/internal/client/dom/blocks/boundary.js @@ -1,6 +1,11 @@ /** @import { Effect, TemplateNode, } from '#client' */ -import { BOUNDARY_EFFECT, BOUNDARY_SUSPENDED, EFFECT_TRANSPARENT } from '../../constants.js'; +import { + BOUNDARY_EFFECT, + BOUNDARY_SUSPENDED, + EFFECT_TRANSPARENT, + RENDER_EFFECT +} from '../../constants.js'; import { component_context, set_component_context } from '../../context.js'; import { block, branch, destroy_effect, pause_effect } from '../../reactivity/effects.js'; import { @@ -10,7 +15,9 @@ import { set_active_effect, set_active_reaction, reset_is_throwing_error, - schedule_effect + schedule_effect, + check_dirtiness, + update_effect } from '../../runtime.js'; import { hydrate_next, @@ -28,6 +35,9 @@ import { run_all } from '../../../shared/utils.js'; const ASYNC_INCREMENT = Symbol(); const ASYNC_DECREMENT = Symbol(); const ADD_CALLBACK = Symbol(); +const ADD_RENDER_EFFECT = Symbol(); +const ADD_EFFECT = Symbol(); +const RELEASE = Symbol(); /** * @param {Effect} boundary @@ -86,6 +96,12 @@ export function boundary(node, props, children) { /** @type {Array<() => void>} */ var callbacks = []; + /** @type {Effect[]} */ + var render_effects = []; + + /** @type {Effect[]} */ + var effects = []; + /** * @param {() => void} snippet_fn * @returns {Effect | null} @@ -125,7 +141,19 @@ export function boundary(node, props, children) { } function unsuspend() { - boundary.f ^= BOUNDARY_SUSPENDED; + if ((boundary.f & BOUNDARY_SUSPENDED) !== 0) { + boundary.f ^= BOUNDARY_SUSPENDED; + } + + for (const e of render_effects) { + try { + if (check_dirtiness(e)) { + update_effect(e); + } + } catch (error) { + handle_error(error, e, null, e.ctx); + } + } run_all(callbacks); callbacks.length = 0; @@ -141,14 +169,21 @@ export function boundary(node, props, children) { offscreen_fragment = null; } - if (main_effect !== null) { - // TODO do we also need to `resume_effect` here? - schedule_effect(main_effect); + // TODO this timing is wrong, effects need to ~somehow~ end up + // in the right place + for (const e of effects) { + try { + if (check_dirtiness(e)) { + update_effect(e); + } + } catch (error) { + handle_error(error, e, null, e.ctx); + } } } // @ts-ignore We re-use the effect's fn property to avoid allocation of an additional field - boundary.fn = (/** @type {unknown} */ input, /** @type {() => void} */ payload) => { + boundary.fn = (/** @type {unknown} */ input, /** @type {any} */ payload) => { if (input === ASYNC_INCREMENT) { boundary.f |= BOUNDARY_SUSPENDED; async_count++; @@ -160,7 +195,12 @@ export function boundary(node, props, children) { if (input === ASYNC_DECREMENT) { if (--async_count === 0) { - queue_boundary_micro_task(unsuspend); + unsuspend(); + + if (main_effect !== null) { + // TODO do we also need to `resume_effect` here? + schedule_effect(main_effect); + } } return; @@ -171,6 +211,21 @@ export function boundary(node, props, children) { return; } + if (input === ADD_RENDER_EFFECT) { + render_effects.push(payload); + return; + } + + if (input === ADD_EFFECT) { + render_effects.push(payload); + return; + } + + if (input === RELEASE) { + unsuspend(); + return; + } + var error = input; var onerror = props.onerror; let failed = props.failed; @@ -372,3 +427,20 @@ export function add_boundary_callback(boundary, fn) { // @ts-ignore boundary.fn(ADD_CALLBACK, fn); } + +/** + * @param {Effect} boundary + * @param {Effect} effect + */ +export function add_boundary_effect(boundary, effect) { + // @ts-ignore + boundary.fn((effect.f & RENDER_EFFECT) !== 0 ? ADD_RENDER_EFFECT : ADD_EFFECT, effect); +} + +/** + * @param {Effect} boundary + */ +export function release_boundary(boundary) { + // @ts-ignore + boundary.fn?.(RELEASE); +} diff --git a/packages/svelte/src/internal/client/dom/blocks/if.js b/packages/svelte/src/internal/client/dom/blocks/if.js index cec06ddf74..589a187aba 100644 --- a/packages/svelte/src/internal/client/dom/blocks/if.js +++ b/packages/svelte/src/internal/client/dom/blocks/if.js @@ -12,6 +12,7 @@ import { block, branch, pause_effect, resume_effect } from '../../reactivity/eff import { HYDRATION_START_ELSE, UNINITIALIZED } from '../../../../constants.js'; import { active_effect, suspended } from '../../runtime.js'; import { add_boundary_callback, find_boundary } from './boundary.js'; +import { should_defer_append } from '../operations.js'; /** * @param {TemplateNode} node @@ -109,9 +110,10 @@ export function if_block(node, fn, elseif = false) { } } + var defer = boundary !== null && should_defer_append(); var target = anchor; - if (suspended) { + if (defer) { offscreen_fragment = document.createDocumentFragment(); offscreen_fragment.append((target = document.createComment(''))); } @@ -120,7 +122,7 @@ export function if_block(node, fn, elseif = false) { pending_effect = fn && branch(() => fn(target)); } - if (suspended) { + if (defer) { add_boundary_callback(boundary, commit); } else { commit(); diff --git a/packages/svelte/src/internal/client/reactivity/sources.js b/packages/svelte/src/internal/client/reactivity/sources.js index d1be99f69b..2bc3a1618c 100644 --- a/packages/svelte/src/internal/client/reactivity/sources.js +++ b/packages/svelte/src/internal/client/reactivity/sources.js @@ -285,22 +285,6 @@ function mark_reactions(signal, status) { continue; } - // if we're about to trip an async derived, mark the boundary as - // suspended _before_ we actually process effects - if ((flags & IS_ASYNC) !== 0) { - let boundary = /** @type {Derived} */ (reaction).parent; - - while (boundary !== null && (boundary.f & BOUNDARY_EFFECT) === 0) { - boundary = boundary.parent; - } - - if (boundary === null) { - // TODO this is presumably an error — throw here? - } else { - boundary.f |= BOUNDARY_SUSPENDED; - } - } - set_signal_status(reaction, status); // If the signal a) was previously clean or b) is an unowned derived, then mark it diff --git a/packages/svelte/src/internal/client/runtime.js b/packages/svelte/src/internal/client/runtime.js index 2fdcc4f048..fd7e5d1b15 100644 --- a/packages/svelte/src/internal/client/runtime.js +++ b/packages/svelte/src/internal/client/runtime.js @@ -31,7 +31,8 @@ import { import { flush_idle_tasks, flush_boundary_micro_tasks, - flush_post_micro_tasks + flush_post_micro_tasks, + queue_micro_task } from './dom/task.js'; import { internal_set } from './reactivity/sources.js'; import { @@ -51,6 +52,7 @@ import { set_component_context, set_dev_current_component_function } from './context.js'; +import { add_boundary_effect, release_boundary } from './dom/blocks/boundary.js'; const FLUSH_MICROTASK = 0; const FLUSH_SYNC = 1; @@ -808,12 +810,12 @@ export function schedule_effect(signal) { * * @param {Effect} effect * @param {Effect[]} collected_effects + * @param {Effect} [boundary] * @returns {void} */ -function process_effects(effect, collected_effects) { +function process_effects(effect, collected_effects, boundary) { var current_effect = effect.first; var effects = []; - suspended = false; main_loop: while (current_effect !== null) { var flags = current_effect.f; @@ -822,22 +824,27 @@ function process_effects(effect, collected_effects) { var sibling = current_effect.next; if (!is_skippable_branch && (flags & INERT) === 0) { - // We only want to skip suspended effects if they are not branches or block effects, - // with the exception of template effects, which are technically block effects but also - // have a special flag `TEMPLATE_EFFECT` that we can use to identify them - var skip_suspended = - suspended && + // Inside a boundary, defer everything except block/branch effects + var defer = + boundary !== undefined && (flags & BRANCH_EFFECT) === 0 && ((flags & BLOCK_EFFECT) === 0 || (flags & TEMPLATE_EFFECT) !== 0); - if ((flags & RENDER_EFFECT) !== 0) { + if (defer) { + add_boundary_effect(/** @type {Effect} */ (boundary), current_effect); + } else if ((flags & BOUNDARY_EFFECT) !== 0) { + process_effects(current_effect, collected_effects, current_effect); + + if ((current_effect.f & BOUNDARY_SUSPENDED) === 0) { + // no more async work to happen + release_boundary(current_effect); + } + } else if ((flags & RENDER_EFFECT) !== 0) { if ((flags & BOUNDARY_EFFECT) !== 0) { - suspended = (flags & BOUNDARY_SUSPENDED) !== 0; + // TODO do we need to do anything here? } else if (is_branch) { - if (!suspended) { - current_effect.f ^= CLEAN; - } - } else if (!skip_suspended) { + current_effect.f ^= CLEAN; + } else { // Ensure we set the effect to be the active reaction // to ensure that unowned deriveds are correctly tracked // because we're flushing the current effect @@ -860,7 +867,7 @@ function process_effects(effect, collected_effects) { current_effect = child; continue; } - } else if ((flags & EFFECT) !== 0 && !skip_suspended) { + } else if ((flags & EFFECT) !== 0) { effects.push(current_effect); } } @@ -873,15 +880,6 @@ function process_effects(effect, collected_effects) { break main_loop; } - if ((parent.f & BOUNDARY_EFFECT) !== 0) { - let boundary = parent.parent; - while (boundary !== null && (boundary.f & BOUNDARY_EFFECT) === 0) { - boundary = boundary.parent; - } - - suspended = boundary === null ? false : (boundary.f & BOUNDARY_SUSPENDED) !== 0; - } - var parent_sibling = parent.next; if (parent_sibling !== null) { current_effect = parent_sibling;