From 7bd853b1a8a952d44368b68bb7f40f25f5e56376 Mon Sep 17 00:00:00 2001 From: Rich Harris Date: Sat, 30 Mar 2024 11:34:06 -0400 Subject: [PATCH] fix: hydrate HTML with surrounding whitespace (#10996) * fix: hydrate HTML with surrounding whitespace * add test * fix a few more short comments * tidy up * avoid magic strings * avoid magic strings * fix * oops --- .changeset/smart-cherries-leave.md | 5 ++++ .prettierignore | 6 ++--- .../3-transform/server/transform-server.js | 6 ++--- packages/svelte/src/constants.js | 3 +++ .../src/internal/client/dom/blocks/each.js | 8 +++++-- .../internal/client/dom/blocks/svelte-head.js | 3 ++- .../src/internal/client/dom/hydration.js | 10 ++++---- packages/svelte/src/internal/client/render.js | 21 ++++++++++++---- .../svelte/src/internal/server/hydration.js | 4 ++++ packages/svelte/src/internal/server/index.js | 11 +++++---- .../samples/surrounding-whitespace/_config.js | 3 +++ .../surrounding-whitespace/_override.html | 2 ++ .../surrounding-whitespace/main.svelte | 1 + packages/svelte/tests/hydration/test.ts | 24 ++++++------------- .../samples/comment-preserve/_expected.html | 2 +- .../_expected/server/index.svelte.js | 4 ++-- .../_expected/server/index.svelte.js | 8 +++---- .../_expected/server/index.svelte.js | 4 ++-- .../_expected/server/index.svelte.js | 4 ++-- 19 files changed, 77 insertions(+), 52 deletions(-) create mode 100644 .changeset/smart-cherries-leave.md create mode 100644 packages/svelte/src/internal/server/hydration.js create mode 100644 packages/svelte/tests/hydration/samples/surrounding-whitespace/_config.js create mode 100644 packages/svelte/tests/hydration/samples/surrounding-whitespace/_override.html create mode 100644 packages/svelte/tests/hydration/samples/surrounding-whitespace/main.svelte diff --git a/.changeset/smart-cherries-leave.md b/.changeset/smart-cherries-leave.md new file mode 100644 index 0000000000..d844352c7f --- /dev/null +++ b/.changeset/smart-cherries-leave.md @@ -0,0 +1,5 @@ +--- +'svelte': patch +--- + +fix: hydrate HTML with surrounding whitespace diff --git a/.prettierignore b/.prettierignore index 45e12c6e3f..dc6385309b 100644 --- a/.prettierignore +++ b/.prettierignore @@ -10,10 +10,8 @@ packages/svelte/tests/**/_actual* packages/svelte/tests/**/expected* packages/svelte/tests/**/_output packages/svelte/tests/**/shards/*.test.js -packages/svelte/tests/hydration/samples/*/_before.html -packages/svelte/tests/hydration/samples/*/_before_head.html -packages/svelte/tests/hydration/samples/*/_after.html -packages/svelte/tests/hydration/samples/*/_after_head.html +packages/svelte/tests/hydration/samples/*/_expected.html +packages/svelte/tests/hydration/samples/*/_override.html packages/svelte/types packages/svelte/compiler.cjs playgrounds/demo/src 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 5b40e32606..ac71edfd01 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 @@ -24,11 +24,11 @@ import { create_attribute, is_custom_element_node, is_element_node } from '../.. import { error } from '../../../errors.js'; import { binding_properties } from '../../bindings.js'; import { regex_starts_with_newline, regex_whitespaces_strict } from '../../patterns.js'; -import { DOMBooleanAttributes } from '../../../../constants.js'; +import { DOMBooleanAttributes, HYDRATION_END, HYDRATION_START } from '../../../../constants.js'; import { sanitize_template_string } from '../../../utils/sanitize_template_string.js'; -const block_open = t_string(''); -const block_close = t_string(''); +export const block_open = t_string(``); +export const block_close = t_string(``); /** * @param {string} value diff --git a/packages/svelte/src/constants.js b/packages/svelte/src/constants.js index 28e6d8d117..5c9408d314 100644 --- a/packages/svelte/src/constants.js +++ b/packages/svelte/src/constants.js @@ -19,6 +19,9 @@ export const TRANSITION_GLOBAL = 1 << 2; export const TEMPLATE_FRAGMENT = 1; export const TEMPLATE_USE_IMPORT_NODE = 1 << 1; +export const HYDRATION_START = '['; +export const HYDRATION_END = ']'; + export const UNINITIALIZED = Symbol(); /** List of Element events that will be delegated */ diff --git a/packages/svelte/src/internal/client/dom/blocks/each.js b/packages/svelte/src/internal/client/dom/blocks/each.js index 2cca222051..8ad815a7a7 100644 --- a/packages/svelte/src/internal/client/dom/blocks/each.js +++ b/packages/svelte/src/internal/client/dom/blocks/each.js @@ -4,7 +4,8 @@ import { EACH_IS_CONTROLLED, EACH_IS_STRICT_EQUALS, EACH_ITEM_REACTIVE, - EACH_KEYED + EACH_KEYED, + HYDRATION_START } from '../../../../constants.js'; import { hydrate_anchor, hydrate_nodes, hydrating, set_hydrating } from '../hydration.js'; import { empty } from '../operations.js'; @@ -117,7 +118,10 @@ function each(anchor, flags, get_collection, get_key, render_fn, fallback_fn, re var child_anchor = hydrate_nodes[0]; for (var i = 0; i < length; i++) { - if (child_anchor.nodeType !== 8 || /** @type {Comment} */ (child_anchor).data !== '[') { + if ( + child_anchor.nodeType !== 8 || + /** @type {Comment} */ (child_anchor).data !== HYDRATION_START + ) { // If `nodes` is null, then that means that the server rendered fewer items than what // expected, so break out and continue appending non-hydrated items mismatch = true; diff --git a/packages/svelte/src/internal/client/dom/blocks/svelte-head.js b/packages/svelte/src/internal/client/dom/blocks/svelte-head.js index 07d02d2c06..70606cd32f 100644 --- a/packages/svelte/src/internal/client/dom/blocks/svelte-head.js +++ b/packages/svelte/src/internal/client/dom/blocks/svelte-head.js @@ -1,6 +1,7 @@ import { hydrate_anchor, hydrate_nodes, hydrating, set_hydrate_nodes } from '../hydration.js'; import { empty } from '../operations.js'; import { block } from '../../reactivity/effects.js'; +import { HYDRATION_START } from '../../../../constants.js'; /** * @param {(anchor: Node) => import('#client').Dom | void} render_fn @@ -19,7 +20,7 @@ export function head(render_fn) { previous_hydrate_nodes = hydrate_nodes; let anchor = /** @type {import('#client').TemplateNode} */ (document.head.firstChild); - while (anchor.nodeType !== 8 || /** @type {Comment} */ (anchor).data !== '[') { + while (anchor.nodeType !== 8 || /** @type {Comment} */ (anchor).data !== HYDRATION_START) { anchor = /** @type {import('#client').TemplateNode} */ (anchor.nextSibling); } diff --git a/packages/svelte/src/internal/client/dom/hydration.js b/packages/svelte/src/internal/client/dom/hydration.js index 6404aa78f0..ef157f8344 100644 --- a/packages/svelte/src/internal/client/dom/hydration.js +++ b/packages/svelte/src/internal/client/dom/hydration.js @@ -1,3 +1,5 @@ +import { HYDRATION_END, HYDRATION_START } from '../../../constants.js'; + /** * Use this variable to guard everything related to hydration code so it can be treeshaken out * if the user doesn't use the `hydrate` method and these code paths are therefore not needed. @@ -23,7 +25,7 @@ export function set_hydrate_nodes(nodes) { } /** - * This function is only called when `hydrating` is true. If passed a `` opening + * This function is only called when `hydrating` is true. If passed a `` opening * hydration marker, it finds the corresponding closing marker and sets `hydrate_nodes` * to everything between the markers, before returning the closing marker. * @param {Node} node @@ -37,7 +39,7 @@ export function hydrate_anchor(node) { var current = /** @type {Node | null} */ (node); // TODO this could have false positives, if a user comment consisted of `[`. need to tighten that up - if (/** @type {Comment} */ (current)?.data !== '[') { + if (/** @type {Comment} */ (current)?.data !== HYDRATION_START) { return node; } @@ -49,9 +51,9 @@ export function hydrate_anchor(node) { if (current.nodeType === 8) { var data = /** @type {Comment} */ (current).data; - if (data === '[') { + if (data === HYDRATION_START) { depth += 1; - } else if (data === ']') { + } else if (data === HYDRATION_END) { if (depth === 0) { hydrate_nodes = /** @type {import('#client').TemplateNode[]} */ (nodes); return current; diff --git a/packages/svelte/src/internal/client/render.js b/packages/svelte/src/internal/client/render.js index aa2c0b4d55..2f28137163 100644 --- a/packages/svelte/src/internal/client/render.js +++ b/packages/svelte/src/internal/client/render.js @@ -6,7 +6,7 @@ import { empty, init_operations } from './dom/operations.js'; -import { PassiveDelegatedEvents } from '../../constants.js'; +import { HYDRATION_START, PassiveDelegatedEvents } from '../../constants.js'; import { flush_sync, push, pop, current_component_context, untrack } from './runtime.js'; import { effect_root, branch } from './reactivity/effects.js'; import { @@ -121,8 +121,7 @@ export function mount(component, options) { * @returns {Exports} */ export function hydrate(component, options) { - const container = options.target; - const first_child = /** @type {ChildNode} */ (container.firstChild); + const target = options.target; const previous_hydrate_nodes = hydrate_nodes; let hydrated = false; @@ -132,7 +131,19 @@ export function hydrate(component, options) { return flush_sync(() => { set_hydrating(true); - const anchor = hydrate_anchor(first_child); + var node = target.firstChild; + while ( + node && + (node.nodeType !== 8 || /** @type {Comment} */ (node).data !== HYDRATION_START) + ) { + node = node.nextSibling; + } + + if (!node) { + throw new Error('Missing hydration marker'); + } + + const anchor = hydrate_anchor(node); const instance = _mount(component, { ...options, anchor }); // flush_sync will run this callback and then synchronously run any pending effects, @@ -153,7 +164,7 @@ export function hydrate(component, options) { error ); - clear_text_content(container); + clear_text_content(target); set_hydrating(false); return mount(component, options); diff --git a/packages/svelte/src/internal/server/hydration.js b/packages/svelte/src/internal/server/hydration.js new file mode 100644 index 0000000000..941918456b --- /dev/null +++ b/packages/svelte/src/internal/server/hydration.js @@ -0,0 +1,4 @@ +import { HYDRATION_END, HYDRATION_START } from '../../constants.js'; + +export const BLOCK_OPEN = ``; +export const BLOCK_CLOSE = ``; diff --git a/packages/svelte/src/internal/server/index.js b/packages/svelte/src/internal/server/index.js index f163bbfb6d..1ecd0c991d 100644 --- a/packages/svelte/src/internal/server/index.js +++ b/packages/svelte/src/internal/server/index.js @@ -9,6 +9,7 @@ import { } from '../../constants.js'; import { DEV } from 'esm-env'; import { current_component, pop, push } from './context.js'; +import { BLOCK_CLOSE, BLOCK_OPEN } from './hydration.js'; /** * @typedef {{ @@ -161,11 +162,11 @@ export function element(payload, tag, attributes_fn, children_fn) { if (!VoidElements.has(tag)) { if (tag !== 'textarea') { - payload.out += ''; + payload.out += BLOCK_OPEN; } children_fn(); if (tag !== 'textarea') { - payload.out += ''; + payload.out += BLOCK_CLOSE; } payload.out += ``; } @@ -187,7 +188,7 @@ export function render(component, options) { const prev_on_destroy = on_destroy; on_destroy = []; - payload.out += ''; + payload.out += BLOCK_OPEN; if (options.context) { push(); @@ -200,14 +201,14 @@ export function render(component, options) { pop(); } - payload.out += ''; + payload.out += BLOCK_CLOSE; for (const cleanup of on_destroy) cleanup(); on_destroy = prev_on_destroy; return { head: payload.head.out || payload.head.title - ? payload.head.title + '' + payload.head.out + '' + ? payload.head.title + BLOCK_OPEN + payload.head.out + BLOCK_CLOSE : '', html: payload.out }; diff --git a/packages/svelte/tests/hydration/samples/surrounding-whitespace/_config.js b/packages/svelte/tests/hydration/samples/surrounding-whitespace/_config.js new file mode 100644 index 0000000000..f47bee71df --- /dev/null +++ b/packages/svelte/tests/hydration/samples/surrounding-whitespace/_config.js @@ -0,0 +1,3 @@ +import { test } from '../../test'; + +export default test({}); diff --git a/packages/svelte/tests/hydration/samples/surrounding-whitespace/_override.html b/packages/svelte/tests/hydration/samples/surrounding-whitespace/_override.html new file mode 100644 index 0000000000..a4b8d9e17c --- /dev/null +++ b/packages/svelte/tests/hydration/samples/surrounding-whitespace/_override.html @@ -0,0 +1,2 @@ + + hello diff --git a/packages/svelte/tests/hydration/samples/surrounding-whitespace/main.svelte b/packages/svelte/tests/hydration/samples/surrounding-whitespace/main.svelte new file mode 100644 index 0000000000..7d6fe33113 --- /dev/null +++ b/packages/svelte/tests/hydration/samples/surrounding-whitespace/main.svelte @@ -0,0 +1 @@ +{#if true}hello{/if} diff --git a/packages/svelte/tests/hydration/test.ts b/packages/svelte/tests/hydration/test.ts index 460d589695..6884504718 100644 --- a/packages/svelte/tests/hydration/test.ts +++ b/packages/svelte/tests/hydration/test.ts @@ -34,17 +34,11 @@ interface HydrationTest extends BaseTest { after_test?: () => void; } -const { test, run } = suite(async (config, cwd) => { - /** - * Read file and remove whitespace between ssr comments - */ - function read_html(path: string, fallback?: string): string { - const html = fs.readFileSync(fallback && !fs.existsSync(path) ? fallback : path, 'utf-8'); - return config.trim_whitespace !== false - ? html.replace(/()[ \t\n\r\f]+()/g, '$1$2') - : html; - } +function read(path: string): string | void { + return fs.existsSync(path) ? fs.readFileSync(path, 'utf-8') : undefined; +} +const { test, run } = suite(async (config, cwd) => { if (!config.load_compiled) { await compile_directory(cwd, 'client', { accessors: true, ...config.compileOptions }); await compile_directory(cwd, 'server', config.compileOptions); @@ -58,7 +52,7 @@ const { test, run } = suite(async (config, cwd) => { }); fs.writeFileSync(`${cwd}/_output/body.html`, rendered.html + '\n'); - target.innerHTML = rendered.html; + target.innerHTML = read(`${cwd}/_override.html`) ?? rendered.html; if (rendered.head) { fs.writeFileSync(`${cwd}/_output/head.html`, rendered.head + '\n'); @@ -97,15 +91,11 @@ const { test, run } = suite(async (config, cwd) => { assert.ok(!got_hydration_error, 'Unexpected hydration error'); } - const expected = fs.existsSync(`${cwd}/_expected.html`) - ? read_html(`${cwd}/_expected.html`) - : rendered.html; + const expected = read(`${cwd}/_expected.html`) ?? rendered.html; assert_html_equal(target.innerHTML, expected); if (rendered.head) { - const expected = fs.existsSync(`${cwd}/_expected_head.html`) - ? read_html(`${cwd}/_expected_head.html`) - : rendered.head; + const expected = read(`${cwd}/_expected_head.html`) ?? rendered.head; assert_html_equal(head.innerHTML, expected); } diff --git a/packages/svelte/tests/server-side-rendering/samples/comment-preserve/_expected.html b/packages/svelte/tests/server-side-rendering/samples/comment-preserve/_expected.html index 20b49122af..3eb11afc34 100644 --- a/packages/svelte/tests/server-side-rendering/samples/comment-preserve/_expected.html +++ b/packages/svelte/tests/server-side-rendering/samples/comment-preserve/_expected.html @@ -1 +1 @@ -

before

after

+

before

after

diff --git a/packages/svelte/tests/snapshot/samples/bind-this/_expected/server/index.svelte.js b/packages/svelte/tests/snapshot/samples/bind-this/_expected/server/index.svelte.js index 5627a82d5c..bd1d1ec711 100644 --- a/packages/svelte/tests/snapshot/samples/bind-this/_expected/server/index.svelte.js +++ b/packages/svelte/tests/snapshot/samples/bind-this/_expected/server/index.svelte.js @@ -4,8 +4,8 @@ import * as $ from "svelte/internal/server"; export default function Bind_this($$payload, $$props) { $.push(false); - $$payload.out += ``; + $$payload.out += ``; Foo($$payload, {}); - $$payload.out += ``; + $$payload.out += ``; $.pop(); } \ No newline at end of file diff --git a/packages/svelte/tests/snapshot/samples/each-string-template/_expected/server/index.svelte.js b/packages/svelte/tests/snapshot/samples/each-string-template/_expected/server/index.svelte.js index 759705d3f2..baabeb63e0 100644 --- a/packages/svelte/tests/snapshot/samples/each-string-template/_expected/server/index.svelte.js +++ b/packages/svelte/tests/snapshot/samples/each-string-template/_expected/server/index.svelte.js @@ -7,16 +7,16 @@ export default function Each_string_template($$payload, $$props) { const each_array = $.ensure_array_like(['foo', 'bar', 'baz']); - $$payload.out += ``; + $$payload.out += ``; for (let $$index = 0; $$index < each_array.length; $$index++) { const thing = each_array[$$index]; - $$payload.out += ""; + $$payload.out += ""; $$payload.out += `${$.escape(thing)}, `; - $$payload.out += ""; + $$payload.out += ""; } - $$payload.out += ``; + $$payload.out += ``; $.pop(); } \ No newline at end of file diff --git a/packages/svelte/tests/snapshot/samples/function-prop-no-getter/_expected/server/index.svelte.js b/packages/svelte/tests/snapshot/samples/function-prop-no-getter/_expected/server/index.svelte.js index cec3ef17ce..1ae4cc7613 100644 --- a/packages/svelte/tests/snapshot/samples/function-prop-no-getter/_expected/server/index.svelte.js +++ b/packages/svelte/tests/snapshot/samples/function-prop-no-getter/_expected/server/index.svelte.js @@ -13,7 +13,7 @@ export default function Function_prop_no_getter($$payload, $$props) { const plusOne = (num) => num + 1; - $$payload.out += ``; + $$payload.out += ``; Button($$payload, { onmousedown: () => count += 1, @@ -24,6 +24,6 @@ export default function Function_prop_no_getter($$payload, $$props) { } }); - $$payload.out += ``; + $$payload.out += ``; $.pop(); } \ No newline at end of file diff --git a/packages/svelte/tests/snapshot/samples/svelte-element/_expected/server/index.svelte.js b/packages/svelte/tests/snapshot/samples/svelte-element/_expected/server/index.svelte.js index a1ffeff067..2bd6400a5c 100644 --- a/packages/svelte/tests/snapshot/samples/svelte-element/_expected/server/index.svelte.js +++ b/packages/svelte/tests/snapshot/samples/svelte-element/_expected/server/index.svelte.js @@ -7,8 +7,8 @@ export default function Svelte_element($$payload, $$props) { let { tag = 'hr' } = $$props; - $$payload.out += ``; + $$payload.out += ``; if (tag) $.element($$payload, tag, () => {}, () => {}); - $$payload.out += ``; + $$payload.out += ``; $.pop(); } \ No newline at end of file