Svelte 5: only block root tags are checked for being closed

[7nik]

Describe the bug

~The error of the non-closed tag isn't emitted for the content of the implicit children snippet.~ Only block (component root or any {#*} block) root tags are checked for being closed, while nested tags aren't checked. Components seem to be excluded from the "block" definition here.

Reproduction

REPL

Logs

No response

System Info

Svelte 5

Severity

annoyance

[dummdidumm]

What error are you expecting? If I do

<div>
    <progress max=10 value={count}>
    <button onclick={increment}>inc</button>
</div>

I also don't get an error

[brunnerh]

If you move the code into an explicit snippet you currently get:

Unexpected block closing tag

If the code is outside of a component you get:

<progress> was left open

The former is a bit non-descriptive as it does not point towards the element that is the actual problem. Maybe that could be improved as well.

[7nik]

What error are you expecting? If I do

<div>
  <progress max=10 value={count}>
  <button onclick={increment}>inc</button>
</div>

I also don't get an error

It seems that the error about a non-closed tag is emitted only for the block root tags, and nested tags aren't checked. Note, <progress> requires the closing tag!

[dummdidumm]

This behavior mirrors what's done in Svelte 4, and I'm a little scared also erroring on the implicit closing tag. I haven't been around when the compiler error was originally introduced, but I can imagine this being deliberate, in the sense of "in case of a parent element you definitely know that all inner elements need to implicitly be closed, but in case of Svelte blocks or top level you don't"

[7nik]

I'm curious how desirable this behaviour is because it can cause the DOM structure to be different from the expected one, like in the example (the button goes inside the progress where it isn't visible).

[Rich-Harris]

I think this is by design, insofar as Svelte is mimicking HTML here — this...

document.body.innerHTML = `
<div>
  <progress>
  <button>inc</button>
</div>
`;

...results in this HTML (note the inserted </progress> immediately before the </div>:

<div>
  <progress>
  <button>inc</button>
</progress></div>

(Bizarrely, phrasing content is permitted inside a progress element, so it wouldn't make sense to approach this by warning on invalid content.)

I do think it would be reasonable to expect a little more consistency here. It's tricky though — we presumably want to support valid HTML like this...

<ul>
  <li>one
  <li>two
  <li>three
</ul>

...but that's not really possible without also treating this as valid:

<div>
  <progress>
  <button>inc</button>
</div>

If anything I think this is a case where consistency would require us to become less strict. I don't think we should start treating components like we treat explicit snippets today — even though they're conceptually the same, visually they're more like elements — it'd probably be weird if this (turning <ul> into a <Ul> component) didn't work:

<Ul>
  <li>one
  <li>two
  <li>three
</Ul>

But that makes me think that this should also work — that a closing block should close any currently-open elements inside the block:

<Ul>
  {#snippet children()}
    <li>one
    <li>two
    <li>three
  {/snippet}
</Ul>

In other words the only time we'd complain about an unclosed element is at the top level. (Maybe even that's unnecessary?)

Unfortunately, none of this helps with the original case with the <progress>. But I'm not sure what a sensible fix for that could look like, short of always insisting on balanced tags (like in JSX!) for non-void elements, which would be a breaking change that would probably annoy a bunch of people.

[dummdidumm]

FWIW Svelte 4 was able to deal with unclosed tags within control flow blocks, there's a skipped test in our test suite because of that. This must have regressed while reworking the parser. It did error on unclosed tags at the top level though

[7nik]

valid HTML

Technically, invalid HTML is impossible. Any arbitrary text can be rendered as HTML. Including <div/> (though it isn't treated as a self-closing tag), which causes a warning in Svelte 5 and will(?) an error in Svelte 6. However, non-closed non-void tags don't exist in the repaired HTML, and it's good practice to write such a markup where there is nothing to repair, especially in Svelte 5.

For me, omitting a closing tag is like omitting the semicolon in JS: it has no effect in 99% of cases, but one day, it will shoot in your leg:

// bad - raises exception
const luke = {}
const leia = {}
[luke, leia].forEach(jedi => jedi.father = 'vader')

// bad - raises exception
const reaction = "No! That's impossible!"
(async function meanwhileOnTheFalcon() {
  // some async code
}())

And I've checked other frameworks with SFC: Vue - forbids Ember - forbids Angular (Analog + Astro) - allows <li> but throws runtime error on <progress>, maybe I missed something in my setup

What about a warning if you think non-closed tags should be allowed?

[Rich-Harris]

It's a question of semantics of course, but I think it's more correct to say that browsers will render invalid HTML than to say there's no such thing as invalid HTML. Lots of HTML is invalid, otherwise the W3C Validator would have to be called the W3C Persnickety Complainer.

I agree that omitting closing tags is bad, and the comparison with semi-colons is apt. I also think that the fewer ways in which we diverge from HTML parsers, the better. Arguably this is a similar case to #11052, and being stricter than the platform is justifiable.

If we were to become stricter, it would be a case of warning today and erroring in 6.0, same as we did for self-closing non-void elements.