sveltejs / kit

web development, streamlined
https://svelte.dev/docs/kit
MIT License
18.73k stars 1.94k forks source link

Nesting `<button>` tags causes `%sveltekit.body%` to render under the wrong target #8345

Open alexiglesias93 opened 1 year ago

alexiglesias93 commented 1 year ago

Describe the bug

I don't know if this issue is just specific to nesting <button> tags (which is wrong syntax) or if it is caused whenever invalid HTML syntax is written.

When something like this is written (notice the invalid nested <button> tags):

<button class="card" on:click={handle_card_click}>
    <div>Card Title</div>

    <div class="card-controls">
        <button class="card-control" on:click|stopPropagation={handle_edit_card_click}>Edit</button>
        <button class="card-control" on:click|stopPropagation={handle_remove_card_click}>Remove</button>
    </div>
</button>

SvelteKit renders %sveltekit.body% directly under <body>, ignoring the HTML structure defined in app.html and displays this warning:

Placing %sveltekit.body% directly inside <body> is not recommended, as your app may break for users who have certain browser extensions installed.

Consider wrapping it in an element:

<div style="display: contents">
  %sveltekit.body%
</div>

Fixing this is as simple as fixing the invalid HTML, but a more descriptive error would've helped me a lot when debugging my project when I encountered this.

Reproduction

Stackblitz with reproduction steps: https://stackblitz.com/edit/sveltejs-kit-template-default-ylq8rn?file=src/routes/+page.svelte

Logs

No response

System Info

(Info from Stackblitz, but the issue happens in regular local projects too)

  System:
    OS: Linux 5.0 undefined
    CPU: (8) x64 Intel(R) Core(TM) i9-9880H CPU @ 2.30GHz
    Memory: 0 Bytes / 0 Bytes
    Shell: 1.0 - /bin/jsh
  Binaries:
    Node: 16.14.2 - /usr/local/bin/node
    Yarn: 1.22.19 - /usr/local/bin/yarn
    npm: 7.17.0 - /usr/local/bin/npm
  npmPackages:
    @sveltejs/adapter-auto: ^1.0.0 => 1.0.0 
    @sveltejs/kit: ^1.0.0 => 1.0.1 
    svelte: ^3.54.0 => 3.55.0 
    vite: ^4.0.0 => 4.0.4

Severity

annoyance

Additional Information

No response

Rich-Harris commented 1 year ago

Any ideas about what sort of error message would have helped diagnose this? Malformed HTML is rather hard to detect (even if the Svelte compiler had an exhaustive list of rules like 'a button can only include phrasing content', it only operates on a single component at a time, meaning that if a <button> contained a component that also contained a <button>, so we'd need to validate the rendered HTML which is costly/tricky and the developer would have to figure out where the offending component code lived), so realistically it would probably be something like this...

Placing %sveltekit.body% directly inside <body> is not recommended, as your app may break for users who have certain browser extensions installed.

Consider wrapping it in an element:

<div style="display: contents">
  %sveltekit.body%
</div>

+If the body is already wrapped, it may indicate that you're generating malformed HTML

...which doesn't seem like it would be that helpful. Thoughts?

alexiglesias93 commented 1 year ago

Oh, that extra line would've given me a good head start, as initially I was clueless of why I was getting that error and it wasn't until I decided to start enabling/disabling each component in my project that I was able to narrow down the issue to that specific HTML snippet. So it would definitely be helpful!

Maybe it could be possible to identify the specific logic in SvelteKit that causes the %sveltekit.body% to be misplaced? Or instead of analyzing each component separately, check app.html to see if effectively %sveltekit.body% is not a direct child of <body> before throwing the error.

Rich-Harris commented 1 year ago

i guess analysing app.html would be the most robust approach. the (major) downside is that to do it reliably we might need to introduce a dependency on an HTML parser, which would be unfortunate. though maybe given the scope of the problem we can hand-roll a solution

aubergene commented 1 year ago

Just a thought which probably isn't helpful but might inspire something. If you serve a page as xhtml (i.e. with "Content-Type": "application/xhtml+xml") then Chrome will do DOM validation. So in a simple experiment where I try to add invalid content I get and error like this

index.html:11 Uncaught DOMException: Failed to set the 'innerHTML' property on 'Element': The provided markup is invalid XML, and therefore cannot be inserted into an XML document.

myieye commented 8 months ago

I almost created a new issue, but I'm assuming my problem is essentially the same thing.

I've discovered that nesting <button> or <a> breaks the SSR result and then CSR fixes it up. On SSR the first button seems to get closed when the second button starts.

Here's my repro: https://stackblitz.com/edit/sveltejs-kit-template-default-ga3fw5?file=src%2Fapp.d.ts image