Skip to content
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.

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

Inefficient code created for hydration of static components #7226

Closed
benmccann opened this issue Feb 5, 2022 · 6 comments
Closed

Inefficient code created for hydration of static components #7226

benmccann opened this issue Feb 5, 2022 · 6 comments
Labels

Comments

@benmccann
Copy link
Member

benmccann commented Feb 5, 2022

Describe the bug

If there's a big block of static HTML or even the entire component is static, the Svelte compiler still creates a separate variable for every element when hydratable: true. Compare to when hydratable: false and it will just set the entire block with innerHTML. This results in worse time-to-interactive on blogs with large posts using MDSveX, which is a fairly common use case - though this example is a particularly large one.

This is yet another reason we would benefit from something like repair: false: #780 (comment)

Reproduction

https://svelte.dev/repl/a197647b669249f0a1d7a249b7c00044?version=3.46.4

Turn the hydratable flag to true and the output JS will have 10x as many lines. It both takes longer to transfer over the network and longer to run after it's been transferred

Logs

No response

System Info

Svelte 3.46.4

Severity

annoyance

@Conduitry
Copy link
Member

I opened #2152 for this ages ago but it got closed at some point.

@benmccann
Copy link
Member Author

Ah. And from there I see that @tanhauhau even had a PR at one point to implement the performance improvement: #4297

@Conduitry
Copy link
Member

Yeah, which was opening the can of worms about hydration trusting that is starting HTML was accurate, which isn't something we can assume generally. There's definitely work to do here, but there's planning first.

Will this be done in a compatible, opt-in way? Will it be a Svelte 4 thing? Will there be a way to tell the compiler to not trust certain bits of the DOM when hydrating?

@Rich-Harris
Copy link
Member

Recap of discussion earlier today: we could do this by changing the output of this...

<div>
	<p>some HTML</p>
</div>

...like so:

/* App.svelte generated by Svelte v3.46.4 */
import {
	SvelteComponent,
	append_hydration,
	children,
	claim_element,
	claim_text,
	detach,
	element,
	init,
	insert_hydration,
	noop,
	safe_not_equal,
	text
} from "svelte/internal";

function create_fragment(ctx) {
	let div;
	let p;
	let t;

	const innerHTML = "some HTML";

	return {
		c() {
			div = element("div");
-			p = element("p");
-			t = text("some HTML");
+			div.innerHTML = innerHTML;
		},
		l(nodes) {
			div = claim_element(nodes, "DIV", {});
-			var div_nodes = children(div);
-			p = claim_element(div_nodes, "P", {});
-			var p_nodes = children(p);
-			t = claim_text(p_nodes, "some HTML");
-			p_nodes.forEach(detach);
-			div_nodes.forEach(detach);
		},
		m(target, anchor) {
			insert_hydration(target, div, anchor);
			append_hydration(div, p);
			append_hydration(p, t);
		},
		p: noop,
		i: noop,
		o: noop,
		d(detaching) {
			if (detaching) detach(div);
		}
	};
}

class App extends SvelteComponent {
	constructor(options) {
		super();
		init(this, options, null, create_fragment, safe_not_equal, {});
	}
}

export default App;

(Note that the c() block is just what you get with hydratable: false.)

This assumes that the server and client renders match. In order for this to be a non-breaking change (and more generally, to deal with the possibility that they don't match), we could add a hash of the HTML during SSR so that it looks something like this...

<div svelte:hash="xyz123">
	<p>some HTML</p>
</div>

...and update the compiler output accordingly:

		l(nodes) {
			div = claim_element(nodes, "DIV", {});
+			if (div.getAttribute('svelte:hash') !== 'xyz123') div.innerHTML = innerHTML;

svelte:hash open to bikeshedding, obviously.

@benmccann
Copy link
Member Author

The biggest impact I've seen is actually shipping the hydration code over the wire. We could simply check if the contents are equal directly and then do div.innerHTML = innerHTML if they are not to get a big win by really shrinking the size of the hydration code

dummdidumm added a commit that referenced this issue Apr 20, 2023
…nstead of deopt to claiming every nodes (#7426)

Related: #7341, #7226

For purely static HTML, instead of walking the node tree and claiming every node/text etc, hydration now uses the same innerHTML optimization technique for hydration compared to normal create. It uses a new data-svelte-h attribute which is added upon server side rendering containing a hash (computed at build time), and then comparing that hash in the client to ensure it's the same node. If the hash is the same, the whole child content is expected to be the same. If the hash is different, the whole child content is replaced with innerHTML.

---------

Co-authored-by: Simon H <5968653+dummdidumm@users.noreply.github.com>
Co-authored-by: Ben McCann <322311+benmccann@users.noreply.github.com>
Co-authored-by: Simon Holthausen <simon.holthausen@vercel.com>
@Rich-Harris
Copy link
Member

In Svelte 5 we assume that the data is the same between SSR and hydration, and just skip over these. (We may eventually need a way to opt out of that, but for now it's filed under YAGNI.) In code size terms, hydration is a one-time cost in Svelte 5 (it doesn't add code per-component), and so I'm going to treat this as resolved

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

4 participants
@benmccann @Rich-Harris @Conduitry and others