Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

chore: Reduce hydration comment for {:else if} #15250

Open
wants to merge 5 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down
Original file line number Diff line number Diff line change
@@ -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';
Expand All @@ -19,28 +19,29 @@ 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([
b.if(
/** @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
)
])
Expand Down Expand Up @@ -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)));
Expand Down
Original file line number Diff line number Diff line change
@@ -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';
Expand All @@ -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(`<!--[${index++}-->`)))
);
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);
}
41 changes: 30 additions & 11 deletions packages/svelte/src/internal/client/dom/blocks/if.js
Original file line number Diff line number Diff line change
Expand Up @@ -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();
}

Expand All @@ -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.
Expand All @@ -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
}
}

Expand All @@ -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) {
Expand Down