sveltejs / svelte

Cybernetically enhanced web apps
https://svelte.dev
MIT License
78.42k stars 4.11k forks source link

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

Open Rich-Harris opened 2 years ago

Rich-Harris commented 2 years ago

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: https://github.com/sveltejs/kit/issues/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 commented 2 years ago

fixed

<div bind:clientHeight={h} /><main>{@html html}{''}</main>
benmccann commented 2 years ago

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 commented 2 years ago

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 commented 2 years ago

@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 commented 2 years ago

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

Herdubreid commented 2 years ago

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)!