Skip to content

Commit c66a439

Browse files
7nikRich-Harris
7nik
and
Rich-Harris
authored
feat: warn on implicitly closed tags (#15932)
* warn on implicitly closed tags * lint * changeset * separate tests for parent/sibling case * account for attributes in opening tag * tweak * update message --------- Co-authored-by: 7nik <[email protected]> Co-authored-by: Rich Harris <[email protected]>
1 parent 7183886 commit c66a439

File tree

9 files changed

+131
-1
lines changed

9 files changed

+131
-1
lines changed

‎.changeset/blue-experts-tell.md‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
---
2+
'svelte': minor
3+
---
4+
5+
feat: warn on implicitly closed tags

‎documentation/docs/98-reference/.generated/compile-warnings.md‎

Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -632,6 +632,25 @@ In some situations a selector may target an element that is not 'visible' to the
632632
</style>
633633
```
634634

635+
### element_implicitly_closed
636+
637+
```
638+
This element is implicitly closed by the following `%tag%`, which can cause an unexpected DOM structure. Add an explicit `%closing%` to avoid surprises.
639+
```
640+
641+
In HTML, some elements are implicitly closed by another element. For example, you cannot nest a `<p>` inside another `<p>`:
642+
643+
```html
644+
<!-- this HTML... -->
645+
<p><p>hello</p>
646+
647+
<!-- results in this DOM structure -->
648+
<p></p>
649+
<p>hello</p>
650+
```
651+
652+
Similarly, a parent element's closing tag will implicitly close all child elements, even if the `</` was a typo and you meant to create a _new_ element. To avoid ambiguity, it's always a good idea to have an explicit closing tag.
653+
635654
### element_invalid_self_closing_tag
636655

637656
```

‎packages/svelte/messages/compile-warnings/template.md‎

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -30,6 +30,23 @@
3030

3131
> `<%name%>` will be treated as an HTML element unless it begins with a capital letter
3232
33+
## element_implicitly_closed
34+
35+
> This element is implicitly closed by the following `%tag%`, which can cause an unexpected DOM structure. Add an explicit `%closing%` to avoid surprises.
36+
37+
In HTML, some elements are implicitly closed by another element. For example, you cannot nest a `<p>` inside another `<p>`:
38+
39+
```html
40+
<!-- this HTML... -->
41+
<p><p>hello</p>
42+
43+
<!-- results in this DOM structure -->
44+
<p></p>
45+
<p>hello</p>
46+
```
47+
48+
Similarly, a parent element's closing tag will implicitly close all child elements, even if the `</` was a typo and you meant to create a _new_ element. To avoid ambiguity, it's always a good idea to have an explicit closing tag.
49+
3350
## element_invalid_self_closing_tag
3451

3552
> Self-closing HTML tags for non-void elements are ambiguous — use `<%name% ...></%name%>` rather than `<%name% ... />`

‎packages/svelte/src/compiler/phases/1-parse/state/element.js‎

Lines changed: 12 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -93,7 +93,16 @@ export default function element(parser) {
9393
}
9494
}
9595

96-
if (parent.type !== 'RegularElement' && !parser.loose) {
96+
if (parent.type === 'RegularElement') {
97+
if (!parser.last_auto_closed_tag || parser.last_auto_closed_tag.tag !== name) {
98+
const end = parent.fragment.nodes[0]?.start ?? start;
99+
w.element_implicitly_closed(
100+
{ start: parent.start, end },
101+
`</${name}>`,
102+
`</${parent.name}>`
103+
);
104+
}
105+
} else if (!parser.loose) {
97106
if (parser.last_auto_closed_tag && parser.last_auto_closed_tag.tag === name) {
98107
e.element_invalid_closing_tag_autoclosed(start, name, parser.last_auto_closed_tag.reason);
99108
} else {
@@ -186,6 +195,8 @@ export default function element(parser) {
186195
parser.allow_whitespace();
187196

188197
if (parent.type === 'RegularElement' && closing_tag_omitted(parent.name, name)) {
198+
const end = parent.fragment.nodes[0]?.start ?? start;
199+
w.element_implicitly_closed({ start: parent.start, end }, `<${name}>`, `</${parent.name}>`);
189200
parent.end = start;
190201
parser.pop();
191202
parser.last_auto_closed_tag = {

‎packages/svelte/src/compiler/warnings.js‎

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -114,6 +114,7 @@ export const codes = [
114114
'bind_invalid_each_rest',
115115
'block_empty',
116116
'component_name_lowercase',
117+
'element_implicitly_closed',
117118
'element_invalid_self_closing_tag',
118119
'event_directive_deprecated',
119120
'node_invalid_placement_ssr',
@@ -746,6 +747,16 @@ export function component_name_lowercase(node, name) {
746747
w(node, 'component_name_lowercase', `\`<${name}>\` will be treated as an HTML element unless it begins with a capital letter\nhttps://svelte.dev/e/component_name_lowercase`);
747748
}
748749

750+
/**
751+
* This element is implicitly closed by the following `%tag%`, which can cause an unexpected DOM structure. Add an explicit `%closing%` to avoid surprises.
752+
* @param {null | NodeLike} node
753+
* @param {string} tag
754+
* @param {string} closing
755+
*/
756+
export function element_implicitly_closed(node, tag, closing) {
757+
w(node, 'element_implicitly_closed', `This element is implicitly closed by the following \`${tag}\`, which can cause an unexpected DOM structure. Add an explicit \`${closing}\` to avoid surprises.\nhttps://svelte.dev/e/element_implicitly_closed`);
758+
}
759+
749760
/**
750761
* Self-closing HTML tags for non-void elements are ambiguous — use `<%name% ...></%name%>` rather than `<%name% ... />`
751762
* @param {null | NodeLike} node

‎packages/svelte/tests/validator/samples/implicitly-closed-by-parent/input.svelte‎

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,6 @@
1+
<main><div class="hello"></main>
2+
3+
<main>
4+
<div class="hello">
5+
<p>hello</p>
6+
</main>

‎packages/svelte/tests/validator/samples/implicitly-closed-by-parent/warnings.json‎

Lines changed: 26 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,26 @@
1+
[
2+
{
3+
"code": "element_implicitly_closed",
4+
"message": "This element is implicitly closed by the following `</main>`, which can cause an unexpected DOM structure. Add an explicit `</div>` to avoid surprises.",
5+
"start": {
6+
"line": 1,
7+
"column": 6
8+
},
9+
"end": {
10+
"line": 1,
11+
"column": 25
12+
}
13+
},
14+
{
15+
"code": "element_implicitly_closed",
16+
"message": "This element is implicitly closed by the following `</main>`, which can cause an unexpected DOM structure. Add an explicit `</div>` to avoid surprises.",
17+
"start": {
18+
"line": 4,
19+
"column": 1
20+
},
21+
"end": {
22+
"line": 4,
23+
"column": 20
24+
}
25+
}
26+
]

‎packages/svelte/tests/validator/samples/implicitly-closed-by-sibling/input.svelte‎

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,9 @@
1+
<div>
2+
<p class="hello">
3+
<span></span>
4+
<p></p>
5+
</div>
6+
7+
<div>
8+
<p class="hello"><p></p>
9+
</div>

‎packages/svelte/tests/validator/samples/implicitly-closed-by-sibling/warnings.json‎

Lines changed: 26 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,26 @@
1+
[
2+
{
3+
"code": "element_implicitly_closed",
4+
"message": "This element is implicitly closed by the following `<p>`, which can cause an unexpected DOM structure. Add an explicit `</p>` to avoid surprises.",
5+
"start": {
6+
"line": 2,
7+
"column": 1
8+
},
9+
"end": {
10+
"line": 2,
11+
"column": 18
12+
}
13+
},
14+
{
15+
"code": "element_implicitly_closed",
16+
"message": "This element is implicitly closed by the following `<p>`, which can cause an unexpected DOM structure. Add an explicit `</p>` to avoid surprises.",
17+
"start": {
18+
"line": 8,
19+
"column": 1
20+
},
21+
"end": {
22+
"line": 8,
23+
"column": 18
24+
}
25+
}
26+
]

0 commit comments

Comments
 (0)