Skip to content

fix: insert empty text nodes during hydration, where necessary #9729

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

Merged
merged 18 commits into from
Jan 31, 2024
Merged
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
5 changes: 5 additions & 0 deletions .changeset/forty-dolls-wave.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
'svelte': patch
---

fix: insert empty text nodes while hydrating, if necessary
Original file line number Diff line number Diff line change
Expand Up @@ -1099,50 +1099,58 @@ function create_block(parent, name, nodes, context) {
body.push(...state.init);
} else if (trimmed.length > 0) {
const id = b.id(context.state.scope.generate('fragment'));
const node_id = b.id(context.state.scope.generate('node'));

process_children(trimmed, node_id, {
...context,
state
});
const use_space_template =
trimmed.some((node) => node.type === 'ExpressionTag') &&
trimmed.every((node) => node.type === 'Text' || node.type === 'ExpressionTag');

if (use_space_template) {
// special case — we can use `$.space` instead of creating a unique template
const id = b.id(context.state.scope.generate('text'));

process_children(trimmed, () => id, false, {
...context,
state
});

body.push(b.var(id, b.call('$.space', b.id('$$anchor'))), ...state.init);
close = b.stmt(b.call('$.close', b.id('$$anchor'), id));
} else {
/** @type {(is_text: boolean) => import('estree').Expression} */
const expression = (is_text) =>
is_text ? b.call('$.child_frag', id, b.true) : b.call('$.child_frag', id);

process_children(trimmed, expression, false, { ...context, state });

const template = state.template[0];
const use_comment_template = state.template.length === 1 && state.template[0] === '<!>';

if (state.template.length === 1 && (template === ' ' || template === '<!>')) {
if (template === ' ') {
body.push(b.var(node_id, b.call('$.space', b.id('$$anchor'))), ...state.init);
close = b.stmt(b.call('$.close', b.id('$$anchor'), node_id));
if (use_comment_template) {
// special case — we can use `$.comment` instead of creating a unique template
body.push(b.var(id, b.call('$.comment', b.id('$$anchor'))));
} else {
const callee = namespace === 'svg' ? '$.svg_template' : '$.template';

state.hoisted.push(
b.var(
template_name,
b.call(callee, b.template([b.quasi(state.template.join(''), true)], []), b.true)
)
);

body.push(
b.var(id, b.call('$.comment', b.id('$$anchor'))),
b.var(node_id, b.call('$.child_frag', id)),
...state.init
b.var(
id,
b.call(
'$.open_frag',
b.id('$$anchor'),
b.literal(!state.metadata.template_needs_import_node),
template_name
)
)
);
close = b.stmt(b.call('$.close_frag', b.id('$$anchor'), id));
}
} else {
const callee = namespace === 'svg' ? '$.svg_template' : '$.template';

state.hoisted.push(
b.var(
template_name,
b.call(callee, b.template([b.quasi(state.template.join(''), true)], []), b.true)
)
);

body.push(
b.var(
id,
b.call(
'$.open_frag',
b.id('$$anchor'),
b.literal(!state.metadata.template_needs_import_node),
template_name
)
),
b.var(node_id, b.call('$.child_frag', id)),
...state.init
);
body.push(...state.init);

close = b.stmt(b.call('$.close_frag', b.id('$$anchor'), id));
}
Expand Down Expand Up @@ -1418,39 +1426,36 @@ function serialize_event_attribute(node, context) {
* (e.g. `{a} b {c}`) into a single update function. Along the way it creates
* corresponding template node references these updates are applied to.
* @param {import('#compiler').SvelteNode[]} nodes
* @param {import('estree').Expression} parent
* @param {(is_text: boolean) => import('estree').Expression} expression
* @param {boolean} is_element
* @param {import('../types.js').ComponentContext} context
*/
function process_children(nodes, parent, { visit, state }) {
function process_children(nodes, expression, is_element, { visit, state }) {
const within_bound_contenteditable = state.metadata.bound_contenteditable;

/** @typedef {Array<import('#compiler').Text | import('#compiler').ExpressionTag>} Sequence */

/** @type {Sequence} */
let sequence = [];

let expression = parent;

/**
* @param {Sequence} sequence
* @param {boolean} in_fragment
*/
function flush_sequence(sequence, in_fragment) {
function flush_sequence(sequence) {
if (sequence.length === 1) {
const node = sequence[0];

if ((in_fragment && node.type === 'ExpressionTag') || node.type === 'Text') {
expression = b.call('$.sibling', expression);
}

if (node.type === 'Text') {
let prev = expression;
expression = () => b.call('$.sibling', prev(true));
state.template.push(node.raw);
return;
}

state.template.push(' ');

const text_id = get_node_id(expression, state, 'text');
const text_id = get_node_id(expression(true), state, 'text');

const singular = b.stmt(
b.call(
'$.text_effect',
Expand Down Expand Up @@ -1487,50 +1492,47 @@ function process_children(nodes, parent, { visit, state }) {
);
}

return;
}
expression = (is_text) =>
is_text ? b.call('$.sibling', text_id, b.true) : b.call('$.sibling', text_id);
} else {
const text_id = get_node_id(expression(true), state, 'text');

state.template.push(' ');
state.template.push(' ');

const text_id = get_node_id(expression, state, 'text');
const contains_call_expression = sequence.some(
(n) => n.type === 'ExpressionTag' && n.metadata.contains_call_expression
);
const assignment = serialize_template_literal(sequence, visit, state)[1];
const init = b.stmt(b.assignment('=', b.member(text_id, b.id('nodeValue')), assignment));
const singular = b.stmt(b.call('$.text_effect', text_id, b.thunk(assignment)));
const contains_call_expression = sequence.some(
(n) => n.type === 'ExpressionTag' && n.metadata.contains_call_expression
);
const assignment = serialize_template_literal(sequence, visit, state)[1];
const init = b.stmt(b.assignment('=', b.member(text_id, b.id('nodeValue')), assignment));
const singular = b.stmt(b.call('$.text_effect', text_id, b.thunk(assignment)));

if (contains_call_expression && !within_bound_contenteditable) {
state.update_effects.push(singular);
} else if (
sequence.some((node) => node.type === 'ExpressionTag' && node.metadata.dynamic) &&
!within_bound_contenteditable
) {
state.update.push({
singular,
grouped: b.stmt(b.call('$.text', text_id, assignment))
});
} else {
state.init.push(init);
}
if (contains_call_expression && !within_bound_contenteditable) {
state.update_effects.push(singular);
} else if (
sequence.some((node) => node.type === 'ExpressionTag' && node.metadata.dynamic) &&
!within_bound_contenteditable
) {
state.update.push({
singular,
grouped: b.stmt(b.call('$.text', text_id, assignment))
});
} else {
state.init.push(init);
}

expression = b.call('$.sibling', text_id);
expression = (is_text) =>
is_text ? b.call('$.sibling', text_id, b.true) : b.call('$.sibling', text_id);
}
}

let is_fragment = false;
for (let i = 0; i < nodes.length; i += 1) {
const node = nodes[i];

if (node.type === 'Text' || node.type === 'ExpressionTag') {
sequence.push(node);
} else {
if (sequence.length > 0) {
flush_sequence(sequence, is_fragment);
// Ensure we move to the next sibling for the case where we move reference within a fragment
if (!is_fragment && sequence.length === 1 && sequence[0].type === 'ExpressionTag') {
expression = b.call('$.sibling', expression);
is_fragment = true;
}
flush_sequence(sequence);
sequence = [];
}

Expand All @@ -1544,23 +1546,18 @@ function process_children(nodes, parent, { visit, state }) {
// get hoisted inside clean_nodes?
visit(node, state);
} else {
if (
node.type === 'EachBlock' &&
nodes.length === 1 &&
parent.type === 'CallExpression' &&
parent.callee.type === 'Identifier' &&
parent.callee.name === '$.child'
) {
if (node.type === 'EachBlock' && nodes.length === 1 && is_element) {
node.metadata.is_controlled = true;
visit(node, state);
} else {
const id = get_node_id(
expression,
expression(false),
state,
node.type === 'RegularElement' ? node.name : 'node'
);

expression = b.call('$.sibling', id);
expression = (is_text) =>
is_text ? b.call('$.sibling', id, b.true) : b.call('$.sibling', id);

visit(node, {
...state,
Expand All @@ -1572,7 +1569,7 @@ function process_children(nodes, parent, { visit, state }) {
}

if (sequence.length > 0) {
flush_sequence(sequence, false);
flush_sequence(sequence);
}
}

Expand Down Expand Up @@ -2041,12 +2038,14 @@ export const template_visitors = {

process_children(
trimmed,
b.call(
'$.child',
node.name === 'template'
? b.member(context.state.node, b.id('content'))
: context.state.node
),
() =>
b.call(
'$.child',
node.name === 'template'
? b.member(context.state.node, b.id('content'))
: context.state.node
),
true,
{ ...context, state }
);

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -167,7 +167,7 @@ function process_children(nodes, parent, { visit, state }) {
}

const expression = b.call(
'$.escape_text',
'$.escape',
/** @type {import('estree').Expression} */ (visit(node.expression))
);
state.template.push(t_expression(expression));
Expand Down
43 changes: 38 additions & 5 deletions packages/svelte/src/internal/client/operations.js
Original file line number Diff line number Diff line change
Expand Up @@ -182,30 +182,63 @@ export function child(node) {
/**
* @template {Node | Node[]} N
* @param {N} node
* @param {boolean} is_text
* @returns {Node | null}
*/
/*#__NO_SIDE_EFFECTS__*/
export function child_frag(node) {
export function child_frag(node, is_text) {
if (current_hydration_fragment !== null) {
const first_node = /** @type {Node[]} */ (node)[0];
if (current_hydration_fragment !== null && first_node !== null) {

// if an {expression} is empty during SSR, there might be no
// text node to hydrate — we must therefore create one
if (is_text && first_node?.nodeType !== 3) {
const text = document.createTextNode('');
current_hydration_fragment.unshift(text);
if (first_node) {
/** @type {DocumentFragment} */ (first_node.parentNode).insertBefore(text, first_node);
}
return text;
}

if (first_node !== null) {
return capture_fragment_from_node(first_node);
}

return first_node;
}

return first_child_get.call(/** @type {Node} */ (node));
}

/**
* @template {Node} N
* @param {N} node
* @param {boolean} is_text
* @returns {Node | null}
*/
/*#__NO_SIDE_EFFECTS__*/
export function sibling(node) {
export function sibling(node, is_text = false) {
const next_sibling = next_sibling_get.call(node);
if (current_hydration_fragment !== null && next_sibling !== null) {
return capture_fragment_from_node(next_sibling);
if (current_hydration_fragment !== null) {
if (is_text && next_sibling?.nodeType !== 3) {
const text = document.createTextNode('');
if (next_sibling) {
const index = current_hydration_fragment.indexOf(
/** @type {Text | Comment | Element} */ (next_sibling)
);
current_hydration_fragment.splice(index, 0, text);
/** @type {DocumentFragment} */ (next_sibling.parentNode).insertBefore(text, next_sibling);
} else {
current_hydration_fragment.push(text);
}

return text;
}

if (next_sibling !== null) {
return capture_fragment_from_node(next_sibling);
}
}
return next_sibling;
}
Expand Down
11 changes: 0 additions & 11 deletions packages/svelte/src/internal/server/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -149,17 +149,6 @@ export function escape(value, is_attr = false) {
return escaped + str.substring(last);
}

/**
* @template V
* @param {V} value
* @returns {string}
*/
export function escape_text(value) {
const escaped = escape(value);
// If the value is empty, then ensure we put a space so that it creates a text node on the client
return escaped === '' ? ' ' : escaped;
}

/**
* @param {Payload} payload
* @param {(head_payload: Payload['head']) => void} fn
Expand Down
Original file line number Diff line number Diff line change
@@ -1,2 +1 @@
<!--ssr:0--><p></p>
<p></p><!--ssr:0-->
<!--ssr:0--><hr><hr> <p></p> <p></p><!--ssr:0-->
Original file line number Diff line number Diff line change
Expand Up @@ -3,5 +3,6 @@
let maybeUndefined = undefined;
</script>

{maybeNull}<hr>{maybeUndefined}<hr>
<p>{maybeNull}</p>
<p>{maybeUndefined}</p>
Loading