Skip to content

fix: make hydration more html whitespace tolerant #16243

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.

Sign up for GitHub

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

Open
wants to merge 5 commits into
base: main
Choose a base branch
from
+46 −11
Open

fix: make hydration more html whitespace tolerant #16243

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
f7ce3f6
Add failing test for #16242
Jun 25, 2025
b4200fa
Increase expectations for fix to #15819
Jun 25, 2025
a28a11e
make hydration less whitespace sensitive
Jun 25, 2025
e9bd782
fix await blocks
Jun 26, 2025
94c4784
prevent nix flake and direnv setup to leak
Jun 26, 2025
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/curvy-colts-occur.md
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
'svelte': patch
---

fix: make hydration less whitespace sensitive
4 changes: 4 additions & 0 deletions .gitignore
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,9 +14,13 @@ coverage
# dotenv environment variables file
.env
.env.test
flake.nix
flake.lock
.envrc

# build output
.vercel
.direnv

# OS-specific
.DS_Store
Expand Down
2 changes: 1 addition & 1 deletion packages/svelte/src/internal/client/dom/blocks/await.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,7 @@ const CATCH = 2;
*/
export function await_block(node, get_input, pending_fn, then_fn, catch_fn) {
if (hydrating) {
hydrate_next();
hydrate_next(true);
}

var anchor = node;
Expand Down
28 changes: 25 additions & 3 deletions packages/svelte/src/internal/client/dom/hydration.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
/** @import { TemplateNode } from '#client' */

import { COMMENT_NODE } from '#client/constants';
import { COMMENT_NODE, TEXT_NODE } from '#client/constants';
import {
HYDRATION_END,
HYDRATION_ERROR,
Expand Down Expand Up @@ -40,8 +40,30 @@ export function set_hydrate_node(node) {
return (hydrate_node = node);
}

export function hydrate_next() {
return set_hydrate_node(/** @type {TemplateNode} */ (get_next_sibling(hydrate_node)));
/**
* Moove to the next node to be hydrated. Empty text nodes will be skipped,
* unless `allow_text` is set to true.
*
* Skipping whitespace helps to sucessful hydrate even if some middleware added
* arbitrary whitespace into the html. This was at least twice an issue:
*
* - https://github.com/sveltejs/svelte/issues/15819
* - https://github.com/sveltejs/svelte/issues/16242
*
* Removing empty text nodes should be finde, as required text nodes will be
* added on demand. Doing so is necessary because an empty text on the server
* side will result in a missing text nodes as well.
*
* @param {boolean} allow_text
*/
export function hydrate_next(allow_text = false) {
var node = set_hydrate_node(/** @type {TemplateNode} */(get_next_sibling(hydrate_node)));
while (!allow_text && node.nodeType === TEXT_NODE && !node.nodeValue?.trim()) {
Copy link
Contributor

@7nik 7nik Jun 26, 2025 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Trimming "breaks" things - this is why you did hydrate_next(true) for the await block, but similar thing can be in any block. Use

import { test } from '../../test';

export default test({
    before_test() {
        window.document.body.insertBefore(window.document.createTextNode(''), window.document.querySelector('main'))
    }
});

to insert empty text node during testing.

Copy link
Author

@janvogt janvogt Jun 26, 2025 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, I realized it when running the full test-suite. However, the await case was the only one in the entire test suite that seem to rely on whitespace within html. So, allowed for the new logic to be disabled for that case. Notably, this isn't even a hydration test.

On a more conceptual level it is a bad idea to rely on whitespace within html to be preserved. There are countless tools - internal and external - that do non-semantic whitespace changes in html.

Unfortunately, I don't know enough about why exactly preserving whitespace is currently needed, to propose a more principled solution. All I can say is that the new test I added should at least fail gracefully and not crash the entire app resulting in a blank page after hydration.

Copy link
Author

@janvogt janvogt Jun 26, 2025 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Also browsers mostly ignore whitespace, so as a developer I didn't expect the observed crash to blank page of a complex production app to be caused by hydration not able to handle modified white space. Depending on it, will certainly lead to a terrible dev experience for many more. This would be very incompatible with the svelte experience, you awesome people got me hooked on (pun very much intended). Further reading: https://developer.mozilla.org/en-US/docs/Web/API/Document_Object_Model/Whitespace

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Unfortunately, the tests are a far from covering all possible cases.

The case is the following nodes:

  1. hydration mark;
  2. text node (user content + " ", so it's guarantied it is a non-empty text);
  3. other nodes.

and when the text node happens to be just a whitespace, you remove it, that cases the shift of nodes order.

Copy link
Author

@janvogt janvogt Jun 26, 2025 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Unfortunately, the tests are a far from covering all possible cases.

Ok, I feared that being the case. That makes it much harder for me to contribute a fix.

The case is the following nodes:

  1. hydration mark;

  2. text node (user content + " ", so it's guarantied it is a non-empty text);

  3. other nodes.

If you could provide tests for the relevant cases, I cloud try to find a more general solution.

If not, I suggest we just prevent the failing call to appendChild and handle most html whitespace manipulation as hydration mismatches. I can change my PR to do that. The unfortunate thing is, that I believe pre svelte 5 the hydration was much more robust in that regard.

and when the text node happens to be just a whitespace, you remove it, that cases the shift of nodes order.
Yes, that was expected.

In the test suite it only resulted in a gracefully handled hydration mismatch for the await tests whilst making more hydration tests actually hydrate. So it seemed like a good compromise. To make await work as well I allowed to opt out of text node skipping.

If I understand correctly now, you're saying that relying on text nodes to carry relevance happens far more than the existing tests suggest. In that case I am asking myself if that information could be transmitted in a more reliable way. Thoughts?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

See the REPL (check console) in my first message: this branch is causes hydration_mismatch, on 5.34.8 - all good.
More advanced case where this PR causes crash.

Oh, I seem to misread the issue - I thought GT inserts empty text nodes.

We'll make a more general hydration fix.

Copy link
Author

@janvogt janvogt Jun 26, 2025 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We'll make a more general hydration fix.

Awesome. Should I change this commit to only contain the crashing test case?

Oh, I seem to misread the issue - I thought GT inserts empty text nodes.

Sorry I don't follow. What does GT stand for here?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Google Translate

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh, that makes sense 😅

As empty is ambiguous: GT does insert whitespace text nodes. So in HTML they are semantically empty but literally they are not as they contain newline and space characters, which causes the issue in my case.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

BTW. I tried to reproduce the issue within the REPL. Unfortunately without success. So the test in f7ce3f6a9 is the only way to show the crash.

var next_sibling = get_next_sibling(hydrate_node)
hydrate_node.parentElement?.removeChild(hydrate_node)
node = set_hydrate_node(/** @type {TemplateNode} */(next_sibling))
}
return node
}

/** @param {TemplateNode} node */
Expand Down
6 changes: 0 additions & 6 deletions packages/svelte/tests/hydration/samples/cloudflare-mirage-borking-2/_config.js
View file
Open in desktop

This file was deleted.

1 change: 0 additions & 1 deletion packages/svelte/tests/hydration/samples/cloudflare-mirage-borking-2/_expected.html
View file
Open in desktop

This file was deleted.

1 change: 1 addition & 0 deletions packages/svelte/tests/hydration/samples/cloudflare-mirage-borking-2/main.svelte
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
<script>
// https://github.com/sveltejs/svelte/issues/15819
const cond = true;
</script>

Expand Down
1 change: 1 addition & 0 deletions packages/svelte/tests/hydration/samples/whitespace-before-child/Nested.svelte
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
<p>nested</p>
2 changes: 2 additions & 0 deletions packages/svelte/tests/hydration/samples/whitespace-before-child/_override.html
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
<!--[-->
<main><p>nested</p><!----></main><!--]-->
7 changes: 7 additions & 0 deletions packages/svelte/tests/hydration/samples/whitespace-before-child/main.svelte
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
<script>
// https://github.com/sveltejs/svelte/issues/16242
import Nested from './Nested.svelte';
</script>
<main>
<Nested />
</main>