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

Dev-time check for {@html ...} mismatch #12362

Closed
Rich-Harris opened this issue Jul 9, 2024 · 10 comments · Fixed by #12396
Closed

Dev-time check for {@html ...} mismatch #12362

Rich-Harris opened this issue Jul 9, 2024 · 10 comments · Fixed by #12396
Milestone

Comments

@Rich-Harris
Copy link
Member

Describe the problem

In a case like this...

<div>{browser ? 'browser' : 'server'}</div>

...Svelte will update the text node upon hydration. It does not update {@html ...} tags however:

<div>{@html browser ? 'browser' : 'server'}</div>

This is deliberate — these often contain large chunks of HTML rendered from a CMS or a markdown file, and as such it's very rare that there's a discrepancy between server and client. It would be expensive to either a) check that everything matches or b) remove it and reinsert it.

We make a similar trade-off in the case of things like src attributes, since we don't want to reload iframes or kick off additional network requests. But in those cases we warn about the discrepancy at dev time. It would be nice to do the same thing for {@html ...} tags.

Describe the proposed solution

At dev time, hash the content and put the hash in the opening hydration boundary comment. In the browser, repeat, and check that the hashes match.

Importance

nice to have

@Rich-Harris Rich-Harris added this to the 5.x milestone Jul 9, 2024
@Conduitry
Copy link
Member

I'm hoping that we don't need to worry about browsers normalizing/messing with HTML when we do this? It will just be a simple string comparison (via hashes) of literally what the server sent and what the client thinks it should be?

@Rich-Harris
Copy link
Member Author

Exactly, yeah, the browser wouldn't be involved. Literally just hashing the foo in {@html foo}

@paoloricciuti
Copy link
Member

This sounds really fun to work on...can i take a shot at it?

In case the answer is yes should i use the native sha256 implementation from crypto to hash or something else that is faster/produces smaller hashes?

@Rich-Harris
Copy link
Member Author

You can use this — it just needs to be moved to a place where it can be imported by both compiler and runtime without a lot of ../../

export function hash(str) {
str = str.replace(regex_return_characters, '');
let hash = 5381;
let i = str.length;
while (i--) hash = ((hash << 5) - hash) ^ str.charCodeAt(i);
return (hash >>> 0).toString(36);
}

@paoloricciuti
Copy link
Member

You can use this — it just needs to be moved to a place where it can be imported by both compiler and runtime without a lot of ../../

export function hash(str) {
str = str.replace(regex_return_characters, '');
let hash = 5381;
let i = str.length;
while (i--) hash = ((hash << 5) - hash) ^ str.charCodeAt(i);
return (hash >>> 0).toString(36);
}

Great i will take a shot at this likely this evening.

@paoloricciuti
Copy link
Member

Also just for clarification

At dev time, hash the content and put the hash in the opening hydration boundary comment. In the browser, repeat, and check that the hashes match.

At dev time i suppose means "on the server" here right?

@Rich-Harris
Copy link
Member Author

On the server it means context.state.options.dev, on the client it means DEV

@paoloricciuti
Copy link
Member

On the server it means context.state.options.dev, on the client it means DEV

Oh ok got it now!

@paoloricciuti
Copy link
Member

Also the output of this should just be a warning?

@Rich-Harris
Copy link
Member Author

yep. see check_src_in_dev_hydration for prior art

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

Successfully merging a pull request may close this issue.

3 participants