From 2032049e47b97098c874195d837a16acea5bb42e Mon Sep 17 00:00:00 2001 From: adiGuba Date: Mon, 3 Mar 2025 17:38:41 +0100 Subject: [PATCH] chore: Reduce hydration comment for {:else if} (#15250) * rewrite else/if hydration * fix: bad index * reduce args on if_block() * restore the block * don't use isNan() * changeset --- .changeset/tough-steaks-travel.md | 5 +++ .../3-transform/client/visitors/Fragment.js | 4 +- .../3-transform/client/visitors/IfBlock.js | 25 +++++------ .../3-transform/server/visitors/IfBlock.js | 30 +++++++++----- .../src/internal/client/dom/blocks/if.js | 41 ++++++++++++++----- 5 files changed, 71 insertions(+), 34 deletions(-) create mode 100644 .changeset/tough-steaks-travel.md diff --git a/.changeset/tough-steaks-travel.md b/.changeset/tough-steaks-travel.md new file mode 100644 index 0000000000..6a9468b609 --- /dev/null +++ b/.changeset/tough-steaks-travel.md @@ -0,0 +1,5 @@ +--- +'svelte': minor +--- + +chore: Reduce hydration comment for {:else if} diff --git a/packages/svelte/src/compiler/phases/3-transform/client/visitors/Fragment.js b/packages/svelte/src/compiler/phases/3-transform/client/visitors/Fragment.js index e9cfd9c506..389a694741 100644 --- a/packages/svelte/src/compiler/phases/3-transform/client/visitors/Fragment.js +++ b/packages/svelte/src/compiler/phases/3-transform/client/visitors/Fragment.js @@ -48,7 +48,9 @@ export function Fragment(node, context) { const is_single_element = trimmed.length === 1 && trimmed[0].type === 'RegularElement'; const is_single_child_not_needing_template = trimmed.length === 1 && - (trimmed[0].type === 'SvelteFragment' || trimmed[0].type === 'TitleElement'); + (trimmed[0].type === 'SvelteFragment' || + trimmed[0].type === 'TitleElement' || + (trimmed[0].type === 'IfBlock' && trimmed[0].elseif)); const template_name = context.state.scope.root.unique('root'); // TODO infer name from parent diff --git a/packages/svelte/src/compiler/phases/3-transform/client/visitors/IfBlock.js b/packages/svelte/src/compiler/phases/3-transform/client/visitors/IfBlock.js index d658f9eaf8..0876fa30b6 100644 --- a/packages/svelte/src/compiler/phases/3-transform/client/visitors/IfBlock.js +++ b/packages/svelte/src/compiler/phases/3-transform/client/visitors/IfBlock.js @@ -1,4 +1,4 @@ -/** @import { BlockStatement, Expression } from 'estree' */ +/** @import { BlockStatement, Expression, Identifier } from 'estree' */ /** @import { AST } from '#compiler' */ /** @import { ComponentContext } from '../types' */ import * as b from '../../../../utils/builders.js'; @@ -19,14 +19,21 @@ export function IfBlock(node, context) { let alternate_id; if (node.alternate) { - const alternate = /** @type {BlockStatement} */ (context.visit(node.alternate)); alternate_id = context.state.scope.generate('alternate'); - statements.push(b.var(b.id(alternate_id), b.arrow([b.id('$$anchor')], alternate))); + const alternate = /** @type {BlockStatement} */ (context.visit(node.alternate)); + const nodes = node.alternate.nodes; + + let alternate_args = [b.id('$$anchor')]; + if (nodes.length === 1 && nodes[0].type === 'IfBlock' && nodes[0].elseif) { + alternate_args.push(b.id('$$elseif')); + } + + statements.push(b.var(b.id(alternate_id), b.arrow(alternate_args, alternate))); } /** @type {Expression[]} */ const args = [ - context.state.node, + node.elseif ? b.id('$$anchor') : context.state.node, b.arrow( [b.id('$$render')], b.block([ @@ -34,13 +41,7 @@ export function IfBlock(node, context) { /** @type {Expression} */ (context.visit(node.test)), b.stmt(b.call(b.id('$$render'), b.id(consequent_id))), alternate_id - ? b.stmt( - b.call( - b.id('$$render'), - b.id(alternate_id), - node.alternate ? b.literal(false) : undefined - ) - ) + ? b.stmt(b.call(b.id('$$render'), b.id(alternate_id), b.literal(false))) : undefined ) ]) @@ -69,7 +70,7 @@ export function IfBlock(node, context) { // ...even though they're logically equivalent. In the first case, the // transition will only play when `y` changes, but in the second it // should play when `x` or `y` change — both are considered 'local' - args.push(b.literal(true)); + args.push(b.id('$$elseif')); } statements.push(b.stmt(b.call('$.if', ...args))); diff --git a/packages/svelte/src/compiler/phases/3-transform/server/visitors/IfBlock.js b/packages/svelte/src/compiler/phases/3-transform/server/visitors/IfBlock.js index 4df09aa8b9..cbdd2cd8cc 100644 --- a/packages/svelte/src/compiler/phases/3-transform/server/visitors/IfBlock.js +++ b/packages/svelte/src/compiler/phases/3-transform/server/visitors/IfBlock.js @@ -1,4 +1,4 @@ -/** @import { BlockStatement, Expression } from 'estree' */ +/** @import { BlockStatement, Expression, IfStatement } from 'estree' */ /** @import { AST } from '#compiler' */ /** @import { ComponentContext } from '../types.js' */ import { BLOCK_OPEN_ELSE } from '../../../../../internal/server/hydration.js'; @@ -10,19 +10,29 @@ import { block_close, block_open } from './shared/utils.js'; * @param {ComponentContext} context */ export function IfBlock(node, context) { - const test = /** @type {Expression} */ (context.visit(node.test)); - const consequent = /** @type {BlockStatement} */ (context.visit(node.consequent)); + consequent.body.unshift(b.stmt(b.assignment('+=', b.id('$$payload.out'), block_open))); + let if_statement = b.if(/** @type {Expression} */ (context.visit(node.test)), consequent); - const alternate = node.alternate - ? /** @type {BlockStatement} */ (context.visit(node.alternate)) - : b.block([]); + context.state.template.push(if_statement, block_close); - consequent.body.unshift(b.stmt(b.assignment('+=', b.id('$$payload.out'), block_open))); + let index = 1; + let alt = node.alternate; + while (alt && alt.nodes.length === 1 && alt.nodes[0].type === 'IfBlock' && alt.nodes[0].elseif) { + const elseif = alt.nodes[0]; + const alternate = /** @type {BlockStatement} */ (context.visit(elseif.consequent)); + alternate.body.unshift( + b.stmt(b.assignment('+=', b.id('$$payload.out'), b.literal(``))) + ); + if_statement = if_statement.alternate = b.if( + /** @type {Expression} */ (context.visit(elseif.test)), + alternate + ); + alt = elseif.alternate; + } - alternate.body.unshift( + if_statement.alternate = alt ? /** @type {BlockStatement} */ (context.visit(alt)) : b.block([]); + if_statement.alternate.body.unshift( b.stmt(b.assignment('+=', b.id('$$payload.out'), b.literal(BLOCK_OPEN_ELSE))) ); - - context.state.template.push(b.if(test, consequent, alternate), block_close); } diff --git a/packages/svelte/src/internal/client/dom/blocks/if.js b/packages/svelte/src/internal/client/dom/blocks/if.js index 36790c05c1..423c436fe4 100644 --- a/packages/svelte/src/internal/client/dom/blocks/if.js +++ b/packages/svelte/src/internal/client/dom/blocks/if.js @@ -9,16 +9,16 @@ import { set_hydrating } from '../hydration.js'; import { block, branch, pause_effect, resume_effect } from '../../reactivity/effects.js'; -import { HYDRATION_START_ELSE, UNINITIALIZED } from '../../../../constants.js'; +import { HYDRATION_START, HYDRATION_START_ELSE, UNINITIALIZED } from '../../../../constants.js'; /** * @param {TemplateNode} node - * @param {(branch: (fn: (anchor: Node) => void, flag?: boolean) => void) => void} fn - * @param {boolean} [elseif] True if this is an `{:else if ...}` block rather than an `{#if ...}`, as that affects which transitions are considered 'local' + * @param {(branch: (fn: (anchor: Node, elseif?: [number,number]) => void, flag?: boolean) => void) => void} fn + * @param {[number,number]} [elseif] * @returns {void} */ -export function if_block(node, fn, elseif = false) { - if (hydrating) { +export function if_block(node, fn, [root_index, hydrate_index] = [0, 0]) { + if (hydrating && root_index === 0) { hydrate_next(); } @@ -33,26 +33,44 @@ export function if_block(node, fn, elseif = false) { /** @type {UNINITIALIZED | boolean | null} */ var condition = UNINITIALIZED; - var flags = elseif ? EFFECT_TRANSPARENT : 0; + var flags = root_index > 0 ? EFFECT_TRANSPARENT : 0; var has_branch = false; - const set_branch = (/** @type {(anchor: Node) => void} */ fn, flag = true) => { + const set_branch = ( + /** @type {(anchor: Node, elseif?: [number,number]) => void} */ fn, + flag = true + ) => { has_branch = true; update_branch(flag, fn); }; const update_branch = ( /** @type {boolean | null} */ new_condition, - /** @type {null | ((anchor: Node) => void)} */ fn + /** @type {null | ((anchor: Node, elseif?: [number,number]) => void)} */ fn ) => { if (condition === (condition = new_condition)) return; /** Whether or not there was a hydration mismatch. Needs to be a `let` or else it isn't treeshaken out */ let mismatch = false; - if (hydrating) { - const is_else = /** @type {Comment} */ (anchor).data === HYDRATION_START_ELSE; + if (hydrating && hydrate_index !== -1) { + if (root_index === 0) { + const data = /** @type {Comment} */ (anchor).data; + if (data === HYDRATION_START) { + hydrate_index = 0; + } else if (data === HYDRATION_START_ELSE) { + hydrate_index = Infinity; + } else { + hydrate_index = parseInt(data.substring(1)); + if (hydrate_index !== hydrate_index) { + // if hydrate_index is NaN + // we set an invalid index to force mismatch + hydrate_index = condition ? Infinity : -1; + } + } + } + const is_else = hydrate_index > root_index; if (!!condition === is_else) { // Hydration mismatch: remove everything inside the anchor and start fresh. @@ -62,6 +80,7 @@ export function if_block(node, fn, elseif = false) { set_hydrate_node(anchor); set_hydrating(false); mismatch = true; + hydrate_index = -1; // ignore hydration in next else if } } @@ -81,7 +100,7 @@ export function if_block(node, fn, elseif = false) { if (alternate_effect) { resume_effect(alternate_effect); } else if (fn) { - alternate_effect = branch(() => fn(anchor)); + alternate_effect = branch(() => fn(anchor, [root_index + 1, hydrate_index])); } if (consequent_effect) {