Skip to content

Conversation

@tanhauhau
Copy link
Member

Fixes sveltejs/kit#2977

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

@tanhauhau tanhauhau force-pushed the tanhauhau/fix-2977 branch from c780564 to 09ff6f9 Compare April 19, 2022 15:53
@tanhauhau tanhauhau force-pushed the tanhauhau/fix-2977 branch from 09ff6f9 to 4a473e3 Compare April 19, 2022 16:28
@baseballyama
Copy link
Member

According to the repl of sveltejs/kit#2977, now it works OK.
But if {@html} has multiple root nodes like <div>foo</div><div>bar</div>, always if (ssr_html !== html) returns false now.
So I think we should handle this case also.

Should we do like this way?

let ssr_html = '';
for (const n of claimed_nodes) {
  ssr_html += n.outerHTML || '';
}

let normalized_html = '';
const temp_div = document.createElement('div');
temp_div.innerHTML = html;
for (const n of normalized_html.childNodes) {
  normalized_html += n.outerHTML || '';
}
if (ssr_html !== normalized_html) {
  for (const n of claimed_nodes) {
    detach(n);
  }
  return new HtmlTagHydration(undefined, is_svg);
}

@tanhauhau
Copy link
Member Author

tanhauhau commented Apr 25, 2022

i hvnt tried, but what was the reason that multiple root nodes causes the ssr_html !== html?

one thing i can think of that this approach has potential bug is that, the parent element cannot always be div. i remember that elements like <td> need to be added into <tr>

@tanhauhau
Copy link
Member Author

I realise if there's text nodes, will need to ssr_html += node.outerHTML || node.data || '' since text nodes does not have outerHTML

@baseballyama
Copy link
Member

I have good specific example!

If I have this svelte file.

<script context="module">
	export const hydrate = true;
	export const prerender = true;
</script>
<script>
  const content = `<div>xxxxxxx</div>
		<p>yyyy<b>yyyy</b>yyyy</p>
		<p>zzzzzz</p>`;
</script>

<h1>Hello World</h1>
{@html content}
<p id="p">Bye World</p>

ssr_html is:

<div>xxxxxxx</div>undefined<p>yyyy<b>yyyy</b>yyyy</p>undefined<p>zzzzzz</p>

html is:

<div>xxxxxxx</div>
		<p>yyyy<b>yyyy</b>yyyy</p>
		<p>zzzzzz</p>

DIfferent points are undefined and break-line / indent.

one thing i can think of that this approach has potential bug is that, the parent element cannot always be div. i remember that elements like need to be added into

Ah yes. But in any way, I think we need to handle this in some way.

@baseballyama
Copy link
Member

I realise if there's text nodes, will need to ssr_html += node.outerHTML || node.data || '' since text nodes does not have outerHTML

Yeah I also forgot node.data but this is required!

And after adding this, now ssr_html and html are same in my above example!

@tanhauhau
Copy link
Member Author

oh really, so the extra indent, newline, spaces are captured nicely within textNode.data?

@baseballyama
Copy link
Member

oh really, so the extra indent, newline, spaces are captured nicely within textNode.data?

Yes.


I found one more issue which is boolean attr issue.
If {@html content}'s content is <button class="abc" disabled>xxxxxxx</button>,
ssr_html is <button class="abc" disabled="">xxxxxxx</button>.
But html is <button class="abc" disabled>xxxxxxx</button>.


And regarding SVG.
At least in my local, it works fine with my suggested idea.


And when I check SVG, I realized the below pattern.

<script context="module">
  export const hydrate = true;
  export const prerender = true;
</script>

<script>
  const content = `<svg><rect x="0" y="0" width="100" height="60" fill="#ddd" />
  <polygon points="50 10, 70 30, 50 50, 30 30" fill="#99f" /></svg>`;
</script>

{@html content}

ssr_html is:

<svg><rect x="0" y="0" width="100" height="60" fill="#ddd"></rect>
  <polygon points="50 10, 70 30, 50 50, 30 30" fill="#99f"></polygon></svg>

but html is

<svg><rect x="0" y="0" width="100" height="60" fill="#ddd" />
  <polygon points="50 10, 70 30, 50 50, 30 30" fill="#99f" /></svg>

@benmccann benmccann changed the title [fix] do not reuse claimed nodes if the html does not match fix: do not reuse claimed nodes if the html does not match Mar 14, 2023
@dummdidumm
Copy link
Member

What's the status of this? Haven't followed the conversation closely, but it sounds like there's some improvements to be made to this PR?

@benmccann benmccann added the bug label Apr 19, 2023
@benmccann benmccann marked this pull request as draft April 19, 2023 17:23
@dummdidumm dummdidumm added this to the one day milestone May 4, 2023
@Rich-Harris
Copy link
Member

Closing, as Svelte 5 has a more robust hydration mechanism

@Rich-Harris Rich-Harris closed this Apr 1, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

@html and export const hydrate = false; issue

5 participants