feat: improve hydration, claim static html elements using innerHTML instead of deopt to claiming every nodes

[tanhauhau]

Related #7341, #7226

1. We have optimsiation to use innerHTML to create static element nodes, however, this is de-optimised, when the hydratable flag is on.
   This PR allows the element nodes to be claimed using innerHTML. It will create a data-svelte hash on the element that can be optimised this way, and if the data-svelte hash does not match, it will fallback to use innerHTML to "fix" the elements.
2. when {@html xxx} is the only child of an element, we "optimised" by setting parent.innerHTML = xxx on mount, for both CSR and hydration. However, for hydration, this will destroy and recreate existing elements from the SSR.
   When we have {@html xxx} as the only child of an element, we add data-svelte hash and only do .innerHTML if the hash does not match.
3. while adding tests for above, found there's some differences in the HTML from CSR and SSR, namely, when there's space between sibling elements, \s\t\r\n, CSR collapse them into 1 space element ' ', however SSR maintains the original spaces. this PR fix the consistency by collapsing spaces in SSR into 1 space ' '. since however many space between the sibling elements, as long as they are not within <pre> or <textarea>, browser will treat them as only 1 space element. This behavior however can be disabled through preserveWhitespace compiler option.

Before submitting the PR, please make sure you do the following

• Prefix your PR title with [feat], [fix], [chore], or [docs].
• This message body should clearly illustrate what problems it solves.
• Ideally, include a test that fails without this PR but passes with it.

Tests

• Run the tests with npm test and lint the project with npm run lint
• combined test/server-side-rendering/samples/{textarea-content,pre-tag,preserve-whitespaces} into test/runtime/samples/*

[dummdidumm]

I thinks this fixes #7226

[tanhauhau]

will wait for #7280

[benmccann]

If we want to use data-svelte we might have to tweak SvelteKit to avoid any conflicts since it currently uses that

[tanhauhau]

If we want to use data-svelte we might have to tweak SvelteKit to avoid any conflicts since it currently uses that

hmm.. but sveltekit targets style[data-svelte] right? probably unlikely that it conflicts?

currently data-svelte is also used in head tags for hydration too

<svelte:head>
   <meta name="xxx" content="yyy" />
</svelte:head>

will ssr render to

<meta name="xxx" content="yyy" data-svelte="hash" />

[benmccann]

oh, yes, right you are. please forgive my sleep deprived comment! though it's probably not a bad idea for SvelteKit to use data-sveltekit rather than data-svelte anyway

[benmccann]

just fyi, it looks like there's a new merge conflict

[benmccann]

I didn't review the code, but I did test it manually and it gets a thumbs up from me.

I tested it on c3.ventures which is written in Svelte. It created 29 instances of data-svelte, which were removed upon hydration.

The entire page in my test is static content and is prerendered. I was hoping there'd just be one data-svelte attribute at the top of the page. However, it got broken up by each image since I use an image component backed by vite-imagetools. Maybe in Svelte 5 we can figure out a way for the compiler to recognize that the image is a const so it can be inlined. Sadly it looks like Prepack is no longer being worked on.

On this one site, it added 250 bytes of common runtime code, but saved 6524 bytes from my main page reducing the size of that file by over 10%.

Without PR:

-rw-rw-r-- 1 benmccann benmccann    97 Oct  5 14:58 0-30f7245e.js
-rw-rw-r-- 1 benmccann benmccann    89 Oct  5 14:58 1-6591d108.js
-rw-rw-r-- 1 benmccann benmccann   147 Oct  5 14:58 2-352fcf64.js
-rw-rw-r-- 1 benmccann benmccann 10381 Oct  5 14:58 index-af9f9aa3.js
-rw-rw-r-- 1 benmccann benmccann   140 Oct  5 14:58 _page-d86dbbba.js
-rw-rw-r-- 1 benmccann benmccann  1855 Oct  5 14:58 singletons-597a2518.js
-rw-rw-r-- 1 benmccann benmccann   546 Oct  5 15:25 _layout.svelte-a8e218a3.js
-rw-rw-r-- 1 benmccann benmccann 60951 Oct  5 15:25 _page.svelte-6fa880c2.js

With PR:

-rw-rw-r-- 1 benmccann benmccann    97 Oct  5 14:59 0-1343795c.js
-rw-rw-r-- 1 benmccann benmccann    89 Oct  5 14:59 1-f3abefb6.js
-rw-rw-r-- 1 benmccann benmccann   147 Oct  5 14:59 2-1bd10837.js
-rw-rw-r-- 1 benmccann benmccann 10631 Oct  5 14:59 index-f37845c4.js
-rw-rw-r-- 1 benmccann benmccann   140 Oct  5 14:59 _page-d86dbbba.js
-rw-rw-r-- 1 benmccann benmccann  1855 Oct  5 14:59 singletons-7a3df618.js
-rw-rw-r-- 1 benmccann benmccann   546 Oct  5 15:27 _layout.svelte-f0b0e2f3.js
-rw-rw-r-- 1 benmccann benmccann 54427 Oct  5 15:27 _page.svelte-70716db4.js

[baseballyama]

[ask]

Why did you remove a few tests?

[tanhauhau]

i've combined them into the test/runtime/samples/* tests

[f-elix]

This is awesome! Very eager for this to be merged :)

[benmccann]

@tanhauhau just a small heads up before our PR bash that there's a merge conflict on this one

[dummdidumm]

@tanhauhau I reverted the @html changes and changed it from data-svelte to data-svelte-h - could you have another look if the changes look good?

[tanhauhau]

meaning right now for static nodes in hydration you would always do parent.innerHTML = '...' ?

this would destroy the SSR-ed html elements, and replace it with the .innerHTML-ed HTML string.

i see u removed some of the tests assertions over here 718a25b that tries to assert the same element instance in hydration test

i wonder would it be possible or worthwhile to check if the parent.innerHTML !== '...' before we decided to wipe it away with parent.innerHTML = '...', eg:

if (parent.innerHTML !== '...') {
  parent.innerHTML = '...';
}

[dummdidumm]

meaning right now for static nodes in hydration you would always do parent.innerHTML = '...'?

No, only for {@html ..} blocks. The other truly static content is checked against data-svelte-h as you implemented it, I just removed that logic for {@html ..} blocks because content could've changed; we can't know.

i see u removed some of the tests assertions over here 718a25b that tries to assert the same element instance in hydration test

Yes, because the test _before.html is different than _after.html, so the whole thing is replaced with .innerHTML per this PR. It didn't occur before because comments for the DOM were only implemented recently, and this is an effect of that now being different.

In general, this got my thinking - shouldn't all the tests in hydrate have data-svelte-h comments? Rather, are most of these tests now obsolete? Many of them check static content before/after, which by this PR is now done by comparing the data-svelte-h and doing nothing if it matches, and replacing everything with .innerHTML = .. if it doesn't - and since none of the tests set it, all the static content is recreated, right? I think we need to update the tests?

i wonder would it be possible or worthwhile to check if the parent.innerHTML !== '...' before we decided to wipe it away with parent.innerHTML = '...', eg:

How would we write code that compares dynamic content before replacing it? I don't see how that's possible.

[tanhauhau]

ok that make sense.
the PR has been a while, i've totally forgotten what i've done over here. 🙈

anyway, i think the changes looks good to me

[dummdidumm]

One thing I'm worried about is that with this change everyt little bit of HTML will have a data-svelte-h attribute associated with it. Even <div>hi</div> will have it. What does this mean for the HTML size - is it ok or do we need to special-case the "one element with text in it"-case to be less consuming? Idea would be to compare textContent with what should be there and set it if it doesn't match. Like

function hydate_single_node_with_text(node, text) {
  if (node.textContent !== text) {
   node.textContent = text;
  }
}

// .. inside hydration block:
hydate_single_node_with_text(div, 'the text that should appear');

[benmccann]

One thing I'm worried about is that with this change every little bit of HTML will have a data-svelte-h attribute associated with it. Even <div>hi</div> will have it. What does this mean for the HTML size

I think we come out very far ahead. I ran it on a real site earlier up above (#7426 (review)) and it created 29 instances of data-svelte-h. That's not going to add too much size. Meanwhile it saved 6.5k on the main JS.

It's possible this could be further optimized, but I'm not sure it's worth sinking time into honestly as I'd rather spend time thinking about how to optimize perf for Svelte 5. This PR is still nice as it gives us a good starting point for benchmarking. Anything we come up with for Svelte 5 that's a very different approach like should have to beat this method by a good margin if we want to pursue it.

[doceazedo]

I also think data-svelte might be too generic

Yeah, but data-svelte-h sounds quite unintuitive for me. Maybe data-svelte-static or something more appropriate and readable?