From 0aedab8d5eaacca67e121e4b436ce47c586ad5ec Mon Sep 17 00:00:00 2001 From: Rich Harris Date: Wed, 31 Jan 2024 15:44:12 -0500 Subject: [PATCH] fix: insert empty text nodes during hydration, where necessary (#9729) * Revert "fix: improve template text node serialization (#9722)" This reverts commit 2fa06447cf51893715bfe776f753d1b1f0587d83. * regression test for #9722 * add back failing test * better test * create text nodes during hydration * partial fix * partial fix * remove nasty brittle logic * simplify * refactor * rename * thunkify * fix * add back changeset * changeset * lint --------- Co-authored-by: Rich Harris --- .changeset/forty-dolls-wave.md | 5 + .../3-transform/client/visitors/template.js | 187 +++++++++--------- .../3-transform/server/transform-server.js | 2 +- .../svelte/src/internal/client/operations.js | 43 +++- packages/svelte/src/internal/server/index.js | 11 -- .../samples/dynamic-text-nil/_before.html | 3 +- .../samples/dynamic-text-nil/main.svelte | 1 + .../event-attribute-template/_config.js | 16 +- .../samples/event-attribute-template/log.js | 2 - .../event-attribute-template/main.svelte | 10 +- .../_expected/client/main.svelte.js | 20 +- .../_expected/client/index.svelte.js | 6 +- .../_expected/client/index.svelte.js | 6 +- .../_expected/client/index.svelte.js | 14 +- 14 files changed, 171 insertions(+), 155 deletions(-) create mode 100644 .changeset/forty-dolls-wave.md delete mode 100644 packages/svelte/tests/runtime-runes/samples/event-attribute-template/log.js diff --git a/.changeset/forty-dolls-wave.md b/.changeset/forty-dolls-wave.md new file mode 100644 index 0000000000..6fccd773c0 --- /dev/null +++ b/.changeset/forty-dolls-wave.md @@ -0,0 +1,5 @@ +--- +'svelte': patch +--- + +fix: insert empty text nodes while hydrating, if necessary diff --git a/packages/svelte/src/compiler/phases/3-transform/client/visitors/template.js b/packages/svelte/src/compiler/phases/3-transform/client/visitors/template.js index e196a09370..a3555aabb9 100644 --- a/packages/svelte/src/compiler/phases/3-transform/client/visitors/template.js +++ b/packages/svelte/src/compiler/phases/3-transform/client/visitors/template.js @@ -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)); } @@ -1418,10 +1426,11 @@ 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} Sequence */ @@ -1429,28 +1438,24 @@ function process_children(nodes, parent, { visit, state }) { /** @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', @@ -1487,37 +1492,39 @@ 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]; @@ -1525,12 +1532,7 @@ function process_children(nodes, parent, { visit, state }) { 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 = []; } @@ -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, @@ -1572,7 +1569,7 @@ function process_children(nodes, parent, { visit, state }) { } if (sequence.length > 0) { - flush_sequence(sequence, false); + flush_sequence(sequence); } } @@ -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 } ); diff --git a/packages/svelte/src/compiler/phases/3-transform/server/transform-server.js b/packages/svelte/src/compiler/phases/3-transform/server/transform-server.js index b6e53a739b..1f0dcc9e1b 100644 --- a/packages/svelte/src/compiler/phases/3-transform/server/transform-server.js +++ b/packages/svelte/src/compiler/phases/3-transform/server/transform-server.js @@ -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)); diff --git a/packages/svelte/src/internal/client/operations.js b/packages/svelte/src/internal/client/operations.js index e8cbe22718..372b1e510a 100644 --- a/packages/svelte/src/internal/client/operations.js +++ b/packages/svelte/src/internal/client/operations.js @@ -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; } diff --git a/packages/svelte/src/internal/server/index.js b/packages/svelte/src/internal/server/index.js index aafbd8252a..12d9a3c130 100644 --- a/packages/svelte/src/internal/server/index.js +++ b/packages/svelte/src/internal/server/index.js @@ -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 diff --git a/packages/svelte/tests/hydration/samples/dynamic-text-nil/_before.html b/packages/svelte/tests/hydration/samples/dynamic-text-nil/_before.html index d3ae0ab9f1..7508e31db6 100644 --- a/packages/svelte/tests/hydration/samples/dynamic-text-nil/_before.html +++ b/packages/svelte/tests/hydration/samples/dynamic-text-nil/_before.html @@ -1,2 +1 @@ -

-

+

diff --git a/packages/svelte/tests/hydration/samples/dynamic-text-nil/main.svelte b/packages/svelte/tests/hydration/samples/dynamic-text-nil/main.svelte index 853faa635c..5e5fc5c82b 100644 --- a/packages/svelte/tests/hydration/samples/dynamic-text-nil/main.svelte +++ b/packages/svelte/tests/hydration/samples/dynamic-text-nil/main.svelte @@ -3,5 +3,6 @@ let maybeUndefined = undefined; +{maybeNull}
{maybeUndefined}

{maybeNull}

{maybeUndefined}

diff --git a/packages/svelte/tests/runtime-runes/samples/event-attribute-template/_config.js b/packages/svelte/tests/runtime-runes/samples/event-attribute-template/_config.js index c8d797b1a6..550fd05b47 100644 --- a/packages/svelte/tests/runtime-runes/samples/event-attribute-template/_config.js +++ b/packages/svelte/tests/runtime-runes/samples/event-attribute-template/_config.js @@ -1,20 +1,16 @@ import { flushSync } from 'svelte'; import { test } from '../../test'; -import { log } from './log.js'; export default test({ - before_test() { - log.length = 0; - }, + html: ``, async test({ assert, target }) { - const [b1] = target.querySelectorAll('button'); + const [b1, b2] = target.querySelectorAll('button'); - flushSync(() => { - b1?.click(); - }); + flushSync(() => b1?.click()); + assert.htmlEqual(target.innerHTML, ``); - await Promise.resolve(); - assert.deepEqual(log, ['onclick']); + flushSync(() => b2?.click()); + assert.htmlEqual(target.innerHTML, ``); } }); diff --git a/packages/svelte/tests/runtime-runes/samples/event-attribute-template/log.js b/packages/svelte/tests/runtime-runes/samples/event-attribute-template/log.js deleted file mode 100644 index d3df521f4d..0000000000 --- a/packages/svelte/tests/runtime-runes/samples/event-attribute-template/log.js +++ /dev/null @@ -1,2 +0,0 @@ -/** @type {any[]} */ -export const log = []; diff --git a/packages/svelte/tests/runtime-runes/samples/event-attribute-template/main.svelte b/packages/svelte/tests/runtime-runes/samples/event-attribute-template/main.svelte index 5976ab15f8..08f5f17d04 100644 --- a/packages/svelte/tests/runtime-runes/samples/event-attribute-template/main.svelte +++ b/packages/svelte/tests/runtime-runes/samples/event-attribute-template/main.svelte @@ -1,10 +1,6 @@ -{undefined}
- +{undefined}{undefined} diff --git a/packages/svelte/tests/snapshot/samples/dynamic-attributes-casing/_expected/client/main.svelte.js b/packages/svelte/tests/snapshot/samples/dynamic-attributes-casing/_expected/client/main.svelte.js index f0960dc93d..6494d2436a 100644 --- a/packages/svelte/tests/snapshot/samples/dynamic-attributes-casing/_expected/client/main.svelte.js +++ b/packages/svelte/tests/snapshot/samples/dynamic-attributes-casing/_expected/client/main.svelte.js @@ -13,25 +13,25 @@ export default function Main($$anchor, $$props) { let y = () => 'test'; /* Init */ var fragment = $.open_frag($$anchor, false, frag); - var node = $.child_frag(fragment); - var svg = $.sibling($.sibling(node)); - var custom_element = $.sibling($.sibling(svg)); - var div = $.sibling($.sibling(custom_element)); - var svg_1 = $.sibling($.sibling(div)); - var custom_element_1 = $.sibling($.sibling(svg_1)); + var div = $.child_frag(fragment); + var svg = $.sibling($.sibling(div, true)); + var custom_element = $.sibling($.sibling(svg, true)); + var div_1 = $.sibling($.sibling(custom_element, true)); + var svg_1 = $.sibling($.sibling(div_1, true)); + var custom_element_1 = $.sibling($.sibling(svg_1, true)); /* Update */ - $.attr_effect(div, "foobar", y); + $.attr_effect(div_1, "foobar", y); $.attr_effect(svg_1, "viewBox", y); $.set_custom_element_data_effect(custom_element_1, "fooBar", y); - var node_foobar; + var div_foobar; var svg_viewBox; var custom_element_fooBar; $.render_effect(() => { - if (node_foobar !== (node_foobar = x)) { - $.attr(node, "foobar", node_foobar); + if (div_foobar !== (div_foobar = x)) { + $.attr(div, "foobar", div_foobar); } if (svg_viewBox !== (svg_viewBox = x)) { diff --git a/packages/svelte/tests/snapshot/samples/each-string-template/_expected/client/index.svelte.js b/packages/svelte/tests/snapshot/samples/each-string-template/_expected/client/index.svelte.js index 5680d12e63..70746bcb36 100644 --- a/packages/svelte/tests/snapshot/samples/each-string-template/_expected/client/index.svelte.js +++ b/packages/svelte/tests/snapshot/samples/each-string-template/_expected/client/index.svelte.js @@ -16,11 +16,11 @@ export default function Each_string_template($$anchor, $$props) { 1, ($$anchor, thing, $$index) => { /* Init */ - var node_1 = $.space($$anchor); + var text = $.space($$anchor); /* Update */ - $.text_effect(node_1, () => `${$.stringify($.unwrap(thing))}, `); - $.close($$anchor, node_1); + $.text_effect(text, () => `${$.stringify($.unwrap(thing))}, `); + $.close($$anchor, text); }, null ); diff --git a/packages/svelte/tests/snapshot/samples/function-prop-no-getter/_expected/client/index.svelte.js b/packages/svelte/tests/snapshot/samples/function-prop-no-getter/_expected/client/index.svelte.js index b53ee2523d..2e984c1ff8 100644 --- a/packages/svelte/tests/snapshot/samples/function-prop-no-getter/_expected/client/index.svelte.js +++ b/packages/svelte/tests/snapshot/samples/function-prop-no-getter/_expected/client/index.svelte.js @@ -23,11 +23,11 @@ export default function Function_prop_no_getter($$anchor, $$props) { onmouseenter: () => $.set(count, $.proxy(plusOne($.get(count)))), children: ($$anchor, $$slotProps) => { /* Init */ - var node_1 = $.space($$anchor); + var text = $.space($$anchor); /* Update */ - $.text_effect(node_1, () => `clicks: ${$.stringify($.get(count))}`); - $.close($$anchor, node_1); + $.text_effect(text, () => `clicks: ${$.stringify($.get(count))}`); + $.close($$anchor, text); } }); diff --git a/packages/svelte/tests/snapshot/samples/state-proxy-literal/_expected/client/index.svelte.js b/packages/svelte/tests/snapshot/samples/state-proxy-literal/_expected/client/index.svelte.js index 321496e20b..3da681ea53 100644 --- a/packages/svelte/tests/snapshot/samples/state-proxy-literal/_expected/client/index.svelte.js +++ b/packages/svelte/tests/snapshot/samples/state-proxy-literal/_expected/client/index.svelte.js @@ -19,18 +19,18 @@ export default function State_proxy_literal($$anchor, $$props) { let tpl = $.source(``); /* Init */ var fragment = $.open_frag($$anchor, true, frag); - var node = $.child_frag(fragment); + var input = $.child_frag(fragment); - $.remove_input_attr_defaults(node); + $.remove_input_attr_defaults(input); - var input = $.sibling($.sibling(node)); + var input_1 = $.sibling($.sibling(input, true)); - $.remove_input_attr_defaults(input); + $.remove_input_attr_defaults(input_1); - var button = $.sibling($.sibling(input)); + var button = $.sibling($.sibling(input_1, true)); - $.bind_value(node, () => $.get(str), ($$value) => $.set(str, $$value)); - $.bind_value(input, () => $.get(tpl), ($$value) => $.set(tpl, $$value)); + $.bind_value(input, () => $.get(str), ($$value) => $.set(str, $$value)); + $.bind_value(input_1, () => $.get(tpl), ($$value) => $.set(tpl, $$value)); button.__click = [reset, str, tpl]; $.close_frag($$anchor, fragment); $.pop();