runem / lit-analyzer

Monorepository for tools that analyze lit-html templates
MIT License
317 stars 36 forks source link

Wrongly reports "This tag isn't closed" on <p> <div></div></p> #283

Open nmattia opened 1 year ago

nmattia commented 1 year ago

The CLI version of lit-analyzer will incorrectly report a <p> tag starting with a space as unclosed:

./src/file.ts

    This tag isn't closed.
    92:  <p> <div></div></p>
    no-unclosed-tag

Removing the whitespace fixes the issue.

AndrewJakubowicz commented 7 months ago

I think the issue here is that the workaround of removing the whitespace should not resolve the issue. The error message can also be made clearer.

Regarding the example html provided, both with and without the whitespace results in DOM being created that doesn't reflect what was authored.

Due to the paragraph tag allowing "tag omission", the <div> will result in the <p> being self-closed. When parsed by the browser (and parse5), the html: <p> <div></div></p> or <p><div></div></p> creates the following DOM:

<p> </p><div></div><p></p>

or

<p></p><div></div><p></p>

In both cases, with or without the space, the html as written won't reflect what is rendered in the DOM.

The specification reference for why this happens is described here: https://html.spec.whatwg.org/multipage/grouping-content.html#the-p-element

Relevant excerpt (with large list of tags omitted):

"A p element's end tag can be omitted if the p element is immediately followed by a ... div ... element"

I think the action item here is that both cases should raise the This tag isn't closed error. However maybe a message can be added to warn that this diagnostic is raised in the case where closing tags were automatically added by parse5/browser that were not authored?

Edit: Although maybe this belongs in a different category of error.