From 432c0235c941e9c19b10377e363a2580638075a9 Mon Sep 17 00:00:00 2001 From: paoloricciuti Date: Mon, 30 Mar 2026 08:43:11 +0200 Subject: [PATCH] fix: push and pop renderer when needed --- .../internal/client/custom-renderer/state.js | 7 +++ .../internal/client/dom/blocks/boundary.js | 38 ++++++++++--- .../internal/client/dom/blocks/branches.js | 37 +++++++++--- .../src/internal/client/dom/blocks/each.js | 57 +++++++++++++++---- .../src/internal/client/reactivity/async.js | 5 ++ .../src/internal/client/reactivity/effects.js | 10 +++- 6 files changed, 127 insertions(+), 27 deletions(-) diff --git a/packages/svelte/src/internal/client/custom-renderer/state.js b/packages/svelte/src/internal/client/custom-renderer/state.js index 2f0bf8b174..90e8f21372 100644 --- a/packages/svelte/src/internal/client/custom-renderer/state.js +++ b/packages/svelte/src/internal/client/custom-renderer/state.js @@ -7,6 +7,13 @@ */ export let renderer = null; +/** + * @param {Renderer | null} $renderer + */ +export function set_renderer($renderer) { + renderer = $renderer; +} + /** * * @param {Renderer} $renderer diff --git a/packages/svelte/src/internal/client/dom/blocks/boundary.js b/packages/svelte/src/internal/client/dom/blocks/boundary.js index b440bb3ba4..22ec1e7830 100644 --- a/packages/svelte/src/internal/client/dom/blocks/boundary.js +++ b/packages/svelte/src/internal/client/dom/blocks/boundary.js @@ -39,9 +39,16 @@ import { Batch, current_batch, schedule_effect } from '../../reactivity/batch.js import { internal_set, source } from '../../reactivity/sources.js'; import { tag } from '../../dev/tracing.js'; import { createSubscriber } from '../../../../reactivity/create-subscriber.js'; -import { create_text } from '../operations.js'; +import { + create_text, + create_fragment, + append_child, + insert_before, + get_node_value +} from '../operations.js'; import { defer_effect } from '../../reactivity/utils.js'; import { set_signal_status } from '../../reactivity/status.js'; +import { push_renderer } from '../../custom-renderer/state.js'; /** * @typedef {{ @@ -164,13 +171,17 @@ export class Boundary { const comment = /** @type {Comment} */ (this.#hydrate_open); hydrate_next(); - const server_rendered_pending = comment.data === HYDRATION_START_ELSE; - const server_rendered_failed = comment.data.startsWith(HYDRATION_START_FAILED); + const server_rendered_pending = get_node_value(comment) === HYDRATION_START_ELSE; + const server_rendered_failed = (get_node_value(comment) ?? '').startsWith( + HYDRATION_START_FAILED + ); if (server_rendered_failed) { // Server rendered the failed snippet - hydrate it. // The serialized error is embedded in the comment: - const serialized_error = JSON.parse(comment.data.slice(HYDRATION_START_FAILED.length)); + const serialized_error = JSON.parse( + (get_node_value(comment) ?? '').slice(HYDRATION_START_FAILED.length) + ); this.#hydrate_failed_content(serialized_error); } else if (server_rendered_pending) { this.#hydrate_pending_content(); @@ -219,17 +230,19 @@ export class Boundary { this.#pending_effect = branch(() => pending(this.#anchor)); queue_micro_task(() => { - var fragment = (this.#offscreen_fragment = document.createDocumentFragment()); + var pop_renderer = this.#effect.r !== null ? push_renderer(this.#effect.r) : null; + + var fragment = (this.#offscreen_fragment = create_fragment()); var anchor = create_text(); - fragment.append(anchor); + append_child(fragment, anchor); this.#main_effect = this.#run(() => { return branch(() => this.#children(anchor)); }); if (this.#pending_count === 0) { - this.#anchor.before(fragment); + insert_before(this.#anchor, fragment); this.#offscreen_fragment = null; pause_effect(/** @type {Effect} */ (this.#pending_effect), () => { @@ -238,6 +251,8 @@ export class Boundary { this.#resolve(/** @type {Batch} */ (current_batch)); } + + pop_renderer?.(); }); } @@ -252,7 +267,7 @@ export class Boundary { }); if (this.#pending_count > 0) { - var fragment = (this.#offscreen_fragment = document.createDocumentFragment()); + var fragment = (this.#offscreen_fragment = create_fragment()); move_effect(this.#main_effect, fragment); const pending = /** @type {(anchor: Node) => void} */ (this.#props.pending); @@ -309,6 +324,8 @@ export class Boundary { set_active_reaction(this.#effect); set_component_context(this.#effect.ctx); + var pop_renderer = this.#effect.r !== null ? push_renderer(this.#effect.r) : null; + try { Batch.ensure(); return fn(); @@ -319,6 +336,7 @@ export class Boundary { set_active_effect(previous_effect); set_active_reaction(previous_reaction); set_component_context(previous_ctx); + pop_renderer?.(); } } @@ -350,8 +368,10 @@ export class Boundary { } if (this.#offscreen_fragment) { - this.#anchor.before(this.#offscreen_fragment); + var pop_renderer = this.#effect.r !== null ? push_renderer(this.#effect.r) : null; + insert_before(this.#anchor, this.#offscreen_fragment); this.#offscreen_fragment = null; + pop_renderer?.(); } } } diff --git a/packages/svelte/src/internal/client/dom/blocks/branches.js b/packages/svelte/src/internal/client/dom/blocks/branches.js index a8096e0a58..ee0efd43f5 100644 --- a/packages/svelte/src/internal/client/dom/blocks/branches.js +++ b/packages/svelte/src/internal/client/dom/blocks/branches.js @@ -1,4 +1,5 @@ /** @import { Effect, TemplateNode } from '#client' */ +/** @import { Renderer } from '../../custom-renderer' */ import { Batch, current_batch } from '../../reactivity/batch.js'; import { branch, @@ -8,7 +9,16 @@ import { resume_effect } from '../../reactivity/effects.js'; import { hydrate_node, hydrating } from '../hydration.js'; -import { create_text, should_defer_append } from '../operations.js'; +import { + create_text, + should_defer_append, + create_fragment, + append_child, + insert_before, + remove_node, + get_last_child +} from '../operations.js'; +import { push_renderer, renderer as current_renderer } from '../../custom-renderer/state.js'; /** * @typedef {{ effect: Effect, fragment: DocumentFragment }} Branch @@ -59,6 +69,14 @@ export class BranchManager { */ #transition = true; + /** + * The renderer that was active when this BranchManager was created. + * Needed so that #commit can push the correct renderer when doing DOM operations + * outside of an effect context (e.g. as a batch commit callback). + * @type {Renderer | null} + */ + #renderer = null; + /** * @param {TemplateNode} anchor * @param {boolean} transition @@ -66,6 +84,7 @@ export class BranchManager { constructor(anchor, transition = true) { this.anchor = anchor; this.#transition = transition; + this.#renderer = current_renderer; } /** @@ -75,6 +94,8 @@ export class BranchManager { // if this batch was made obsolete, bail if (!this.#batches.has(batch)) return; + var pop_renderer = this.#renderer !== null ? push_renderer(this.#renderer) : null; + var key = /** @type {Key} */ (this.#batches.get(batch)); var onscreen = this.#onscreen.get(key); @@ -92,10 +113,10 @@ export class BranchManager { this.#offscreen.delete(key); // remove the anchor... - /** @type {TemplateNode} */ (offscreen.fragment.lastChild).remove(); + remove_node(/** @type {ChildNode} */ (get_last_child(offscreen.fragment))); // ...and append the fragment - this.anchor.before(offscreen.fragment); + insert_before(this.anchor, offscreen.fragment); onscreen = offscreen.effect; } } @@ -129,10 +150,10 @@ export class BranchManager { if (keys.includes(k)) { // keep the effect offscreen, as another batch will need it - var fragment = document.createDocumentFragment(); + var fragment = create_fragment(); move_effect(effect, fragment); - fragment.append(create_text()); // TODO can we avoid this? + append_child(fragment, create_text()); // TODO can we avoid this? this.#offscreen.set(k, { effect, fragment }); } else { @@ -150,6 +171,8 @@ export class BranchManager { on_destroy(); } } + + pop_renderer?.(); }; /** @@ -179,10 +202,10 @@ export class BranchManager { if (fn && !this.#onscreen.has(key) && !this.#offscreen.has(key)) { if (defer) { - var fragment = document.createDocumentFragment(); + var fragment = create_fragment(); var target = create_text(); - fragment.append(target); + append_child(fragment, target); this.#offscreen.set(key, { effect: branch(() => fn(target)), diff --git a/packages/svelte/src/internal/client/dom/blocks/each.js b/packages/svelte/src/internal/client/dom/blocks/each.js index 3acdf80d84..bee21178ae 100644 --- a/packages/svelte/src/internal/client/dom/blocks/each.js +++ b/packages/svelte/src/internal/client/dom/blocks/each.js @@ -23,7 +23,13 @@ import { create_text, get_first_child, get_next_sibling, - should_defer_append + should_defer_append, + get_parent_node, + append_child, + create_fragment, + insert_before, + node_type, + get_node_value } from '../operations.js'; import { block, @@ -43,6 +49,8 @@ import { derived_safe_equal } from '../../reactivity/deriveds.js'; import { current_batch } from '../../reactivity/batch.js'; import * as e from '../../errors.js'; +import { push_renderer, renderer } from '../../custom-renderer/state.js'; + // When making substantive changes to this file, validate them with the each block stress test: // https://svelte.dev/playground/1972b2cf46564476ad8c8c6405b23b7b // This test also exists in this repo, as `packages/svelte/tests/manual/each-stress-test` @@ -107,10 +115,10 @@ function pause_effects(state, to_destroy, controlled_anchor) { if (fast_path) { var anchor = /** @type {Element} */ (controlled_anchor); - var parent_node = /** @type {Element} */ (anchor.parentNode); + var parent_node = /** @type {Element} */ (get_parent_node(anchor)); clear_text_content(parent_node); - parent_node.append(anchor); + append_child(parent_node, anchor); state.items.clear(); } @@ -152,7 +160,7 @@ function destroy_effects(state, to_destroy, remove_dom = true) { if (preserved_effects?.has(e)) { e.f |= EFFECT_OFFSCREEN; - const fragment = document.createDocumentFragment(); + const fragment = create_fragment(); move_effect(e, fragment); } else { destroy_effect(to_destroy[i], remove_dom); @@ -163,6 +171,26 @@ function destroy_effects(state, to_destroy, remove_dom = true) { /** @type {TemplateNode} */ var offscreen_anchor; +/** + * Returns an anchor node suitable for offscreen rendering. + * When a custom renderer is active, the anchor must have a parent + * (so that `getParent()` works), so we place the text node inside + * a fragment. The result is cached just like the non-renderer path. + * @returns {TemplateNode} + */ +function get_offscreen_anchor() { + if (offscreen_anchor !== undefined) return offscreen_anchor; + + offscreen_anchor = create_text(); + + if (renderer) { + var fragment = create_fragment(); + append_child(fragment, offscreen_anchor); + } + + return offscreen_anchor; +} + /** * @template V * @param {Element | Comment} node The next sibling node, or the parent node if this is a 'controlled' block @@ -181,12 +209,17 @@ export function each(node, flags, get_collection, get_key, render_fn, fallback_f var is_controlled = (flags & EACH_IS_CONTROLLED) !== 0; + // Capture the renderer that was active when this each block was created. + // Needed so that the commit callback can push the correct renderer when doing + // DOM operations outside of an effect context (e.g. as a batch commit callback). + var captured_renderer = renderer; + if (is_controlled) { var parent_node = /** @type {Element} */ (node); anchor = hydrating ? set_hydrate_node(get_first_child(parent_node)) - : parent_node.appendChild(create_text()); + : /** @type {Text} */ (append_child(parent_node, create_text())); } if (hydrating) { @@ -221,6 +254,8 @@ export function each(node, flags, get_collection, get_key, render_fn, fallback_f return; } + var pop_renderer = captured_renderer !== null ? push_renderer(captured_renderer) : null; + state.pending.delete(batch); state.fallback = fallback; @@ -243,6 +278,8 @@ export function each(node, flags, get_collection, get_key, render_fn, fallback_f }); } } + + pop_renderer?.(); } /** @@ -279,8 +316,8 @@ export function each(node, flags, get_collection, get_key, render_fn, fallback_f for (var index = 0; index < length; index += 1) { if ( hydrating && - hydrate_node.nodeType === COMMENT_NODE && - /** @type {Comment} */ (hydrate_node).data === HYDRATION_END + node_type(hydrate_node) === COMMENT_NODE && + get_node_value(hydrate_node) === HYDRATION_END ) { // The server rendered fewer items than expected, // so break out and continue appending non-hydrated items @@ -313,7 +350,7 @@ export function each(node, flags, get_collection, get_key, render_fn, fallback_f } else { item = create_item( items, - first_run ? anchor : (offscreen_anchor ??= create_text()), + first_run ? anchor : get_offscreen_anchor(), value, key, index, @@ -336,7 +373,7 @@ export function each(node, flags, get_collection, get_key, render_fn, fallback_f if (first_run) { fallback = branch(() => fallback_fn(anchor)); } else { - fallback = branch(() => fallback_fn((offscreen_anchor ??= create_text()))); + fallback = branch(() => fallback_fn(get_offscreen_anchor())); fallback.f |= EFFECT_OFFSCREEN; } } @@ -708,7 +745,7 @@ function move(effect, next, anchor) { while (node !== null) { var next_node = /** @type {TemplateNode} */ (get_next_sibling(node)); - dest.before(node); + insert_before(dest, node); if (node === end) { return; diff --git a/packages/svelte/src/internal/client/reactivity/async.js b/packages/svelte/src/internal/client/reactivity/async.js index 7b0b108e4c..fe8e63084b 100644 --- a/packages/svelte/src/internal/client/reactivity/async.js +++ b/packages/svelte/src/internal/client/reactivity/async.js @@ -9,6 +9,7 @@ import { set_dev_stack } from '../context.js'; import { Boundary } from '../dom/blocks/boundary.js'; +import { renderer, set_renderer } from '../custom-renderer/state.js'; import { invoke_error_boundary } from '../error-handling.js'; import { active_effect, @@ -113,6 +114,7 @@ export function capture() { var previous_reaction = active_reaction; var previous_component_context = component_context; var previous_batch = /** @type {Batch} */ (current_batch); + var previous_renderer = renderer; if (DEV) { var previous_dev_stack = dev_stack; @@ -123,6 +125,8 @@ export function capture() { set_active_reaction(previous_reaction); set_component_context(previous_component_context); + set_renderer(previous_renderer); + if (activate_batch && (previous_effect.f & DESTROYED) === 0) { // TODO we only need optional chaining here because `{#await ...}` blocks // are anomalous. Once we retire them we can get rid of it @@ -221,6 +225,7 @@ export function unset_context(deactivate_batch = true) { set_active_effect(null); set_active_reaction(null); set_component_context(null); + set_renderer(null); if (deactivate_batch) current_batch?.deactivate(); if (DEV) { diff --git a/packages/svelte/src/internal/client/reactivity/effects.js b/packages/svelte/src/internal/client/reactivity/effects.js index 6139e22438..a33f03fd96 100644 --- a/packages/svelte/src/internal/client/reactivity/effects.js +++ b/packages/svelte/src/internal/client/reactivity/effects.js @@ -46,7 +46,7 @@ import { Batch, collected_effects } from './batch.js'; import { flatten, increment_pending } from './async.js'; import { without_reactive_context } from '../dom/elements/bindings/shared.js'; import { set_signal_status } from './status.js'; -import { renderer } from '../custom-renderer/state.js'; +import { push_renderer, renderer } from '../custom-renderer/state.js'; /** * @param {'$effect' | '$effect.pre' | '$inspect'} rune @@ -514,6 +514,8 @@ export function destroy_block_effect_children(signal) { export function destroy_effect(effect, remove_dom = true) { var removed = false; + var pop_renderer = effect.r !== null ? push_renderer(effect.r) : null; + if ( (remove_dom || (effect.f & HEAD_EFFECT) !== 0) && effect.nodes !== null && @@ -564,6 +566,8 @@ export function destroy_effect(effect, remove_dom = true) { effect.b = effect.r = null; + + pop_renderer?.(); } /** @@ -729,6 +733,8 @@ export function aborted(effect = /** @type {Effect} */ (active_effect)) { export function move_effect(effect, fragment) { if (!effect.nodes) return; + var pop_renderer = effect.r !== null ? push_renderer(effect.r) : null; + /** @type {TemplateNode | null} */ var node = effect.nodes.start; var end = effect.nodes.end; @@ -740,4 +746,6 @@ export function move_effect(effect, fragment) { append_child(fragment, node); node = next; } + + pop_renderer?.(); }