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

{@html ...} tag is populated too late #7341

Open
Rich-Harris opened this issue Mar 4, 2022 · 6 comments
Open

{@html ...} tag is populated too late #7341

Rich-Harris opened this issue Mar 4, 2022 · 6 comments
Labels
compiler Changes relating to the compiler

Comments

@Rich-Harris
Copy link
Member

Describe the bug

Bear with me, this is a bit of an edge case.

If you have code like this...

<script>
	let h = 0;

	const html = '<div style="height: 50vh"></div><div style="height: 100vh; background: salmon"><h3 id="foo">this should be at the top of the page</h3></div>';
</script>

<div bind:clientHeight={h} /><main>{@html html}</main>

it will compile with hydratable: true to this:

		/* omitted */
		l(nodes) {
			div = claim_element(nodes, "DIV", {});
			children(div).forEach(detach);
			main = claim_element(nodes, "MAIN", {});
			var main_nodes = children(main);
			main_nodes.forEach(detach);
			this.h();
		},
		h() {
			add_render_callback(() => /*div_elementresize_handler*/ ctx[1].call(div));
		},
		m(target, anchor) {
			insert_hydration(target, div, anchor);
			div_resize_listener = add_resize_listener(div, /*div_elementresize_handler*/ ctx[1].bind(div));
			insert_hydration(target, main, anchor);
			main.innerHTML = html;
		},
		/* omitted */

Between main_nodes.forEach(detach) and main.innerHTML = html, the addition of the resize listener causes a reflow that, long story short, results in you losing your scroll position if you were previously scrolled to #foo and there's not much content on the page other than the contents of {@html ...}. This is currently visible on the SvelteKit docs: sveltejs/kit#4216. (Only on Chrome, not Firefox.)

Removing the bind:clientHeight (or placing it below the {@html ...}) prevents the bug, as would using a ResizeObserver instead of the resize listener hack. But the biggest mystery is why we're waiting until the mount phase (m) to do innerHTML = html instead of the claim phase (l).

(Aside: it would be nice if there was a way to say 'you don't need to replace me, I promise my contents won't have changed since SSR' — {@html:leavemealone post.html} or whatever.)

Reproduction

Open this StackBlitz repro and go to the standalone version: https://sveltejs-kit-template-default-pm2sdq--3000.local.webcontainer.io/

Logs

No response

System Info

System:
    OS: macOS 12.0.1
    CPU: (10) arm64 Apple M1 Max
    Memory: 90.30 MB / 32.00 GB
    Shell: 5.8 - /bin/zsh
  Binaries:
    Node: 16.13.1 - ~/.nvm/versions/node/v16.13.1/bin/node
    Yarn: 1.22.17 - ~/.nvm/versions/node/v16.13.1/bin/yarn
    npm: 8.1.2 - ~/.nvm/versions/node/v16.13.1/bin/npm
  Browsers:
    Chrome: 98.0.4758.109
    Firefox: 97.0.1
    Safari: 15.1
  npmPackages:
    svelte: ^3.43.0 => 3.44.2

Severity

annoyance

@lidlanca
Copy link
Contributor

lidlanca commented Mar 4, 2022

fixed

<div bind:clientHeight={h} /><main>{@html html}{''}</main>

@benmccann
Copy link
Member

Aside: it would be nice if there was a way to say 'you don't need to replace me, I promise my contents won't have changed since SSR'

+1 this is easily my geatest desire for Svelte. I would love to be able to turn off repair mode

@Herdubreid
Copy link

This is a bit more than annoyance, it actually broke my code.

I've created a REPL to replicate my issue:
https://svelte.dev/repl/6398f723b642481da32c1c52903fd88e?version=3.46.4

The "display" is shifted right and down compared to the cursor, but aligns perfectly in 3.46.3:
https://svelte.dev/repl/6398f723b642481da32c1c52903fd88e?version=3.46.3

@mrkishi
Copy link
Member

mrkishi commented Mar 17, 2022

@Herdubreid That doesn't seem related to this issue, but to #6990 instead. Svelte's behavior was changed because it didn't respect whitespace inside <pre>, even though it should to align with browsers.

@Herdubreid
Copy link

That makes sense @mrkishi, changing {@html code} to {code} makes no difference.

@Herdubreid
Copy link

This fix is quite simple.

Change:

<div>
  <textarea bind:value={text} style:height={height} spellcheck="false" />
  <pre>
    <code id="display">
      {@html code}
    </code>
  </pre>
</div>

To:

<div>
  <textarea bind:value={text} style:height={height} spellcheck="false" />
  <pre><code id="display">{@html code}</code></pre>
</div>

Basically remove unwanted whitespaces within the <pre> tag.

Thanks for pointing me in the right direction @mrkishi (this has been doing my head in for days)!

@dummdidumm dummdidumm added the compiler Changes relating to the compiler label Jul 26, 2022
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>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
compiler Changes relating to the compiler
Projects
None yet
Development

No branches or pull requests

6 participants