Skip to content

Commit 3bff87a

Browse files
chore: tweak html tree validation (#12618)
* chore: tweak html tree validation - relax validation in some places where we know the HTML will not break or only break when using SSR - consolidate validation in one place and for better reuse, which results in more cases getting caught at runtime closes #11941 * move more of the validation into more descriptive record * obselete / incorrect (those are not autoclosed, and the invalid ones handled later) * typo * backticks * update tests * Update packages/svelte/messages/compile-errors/template.md Co-authored-by: Simon H <[email protected]> * Update packages/svelte/messages/compile-warnings/template.md Co-authored-by: Simon H <[email protected]> --------- Co-authored-by: Rich Harris <[email protected]>
1 parent 2e8a205 commit 3bff87a

File tree

19 files changed

+327
-288
lines changed

19 files changed

+327
-288
lines changed

packages/svelte/messages/compile-errors/template.md

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -190,7 +190,13 @@
190190
191191
## node_invalid_placement
192192

193-
> %thing% is invalid inside <%parent%>
193+
> %thing% is invalid inside `<%parent%>`
194+
195+
HTML restricts where certain elements can appear. In case of a violation the browser will 'repair' the HTML in a way that breaks Svelte's assumptions about the structure of your components. Some examples:
196+
197+
- `<p>hello <div>world</div></p>` will result in `<p>hello </p><div>world</div><p></p>` for example (the `<div>` autoclosed the `<p>` because `<p>` cannot contain block-level elements)
198+
- `<option><div>option a</div></option>` will result in `<option>option a</option>` (the `<div>` is removed)
199+
- `<table><tr><td>cell</td></tr></table>` will result in `<table><tbody><tr><td>cell</td></tr></tbody></table>` (a `<tbody>` is auto-inserted)
194200

195201
## render_tag_invalid_call_expression
196202

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

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -38,6 +38,18 @@
3838

3939
> Using `on:%name%` to listen to the %name% event is deprecated. Use the event attribute `on%name%` instead
4040
41+
## node_invalid_placement_ssr
42+
43+
> %thing% is invalid inside `<%parent%>`. When rendering this component on the server, the resulting HTML will be modified by the browser, likely resulting in a `hydration_mismatch` warning
44+
45+
HTML restricts where certain elements can appear. In case of a violation the browser will 'repair' the HTML in a way that breaks Svelte's assumptions about the structure of your components. Some examples:
46+
47+
- `<p>hello <div>world</div></p>` will result in `<p>hello </p><div>world</div><p></p>` for example (the `<div>` autoclosed the `<p>` because `<p>` cannot contain block-level elements)
48+
- `<option><div>option a</div></option>` will result in `<option>option a</option>` (the `<div>` is removed)
49+
- `<table><tr><td>cell</td></tr></table>` will result in `<table><tbody><tr><td>cell</td></tr></tbody></table>` (a `<tbody>` is auto-inserted)
50+
51+
This code will work when the component is rendered on the client (which is why this is a warning rather than an error), but if you use server rendering it will cause hydration to fail.
52+
4153
## slot_element_deprecated
4254

4355
> Using `<slot>` to render parent content is deprecated. Use `{@render ...}` tags instead

packages/svelte/src/compiler/errors.js

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -978,14 +978,14 @@ export function mixed_event_handler_syntaxes(node, name) {
978978
}
979979

980980
/**
981-
* %thing% is invalid inside <%parent%>
981+
* %thing% is invalid inside `<%parent%>`
982982
* @param {null | number | NodeLike} node
983983
* @param {string} thing
984984
* @param {string} parent
985985
* @returns {never}
986986
*/
987987
export function node_invalid_placement(node, thing, parent) {
988-
e(node, "node_invalid_placement", `${thing} is invalid inside <${parent}>`);
988+
e(node, "node_invalid_placement", `${thing} is invalid inside \`<${parent}>\``);
989989
}
990990

991991
/**

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

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -5,12 +5,13 @@ import { is_void } from '../../../../constants.js';
55
import read_expression from '../read/expression.js';
66
import { read_script } from '../read/script.js';
77
import read_style from '../read/style.js';
8-
import { closing_tag_omitted, decode_character_references } from '../utils/html.js';
8+
import { decode_character_references } from '../utils/html.js';
99
import * as e from '../../../errors.js';
1010
import * as w from '../../../warnings.js';
1111
import { create_fragment } from '../utils/create.js';
1212
import { create_attribute } from '../../nodes.js';
1313
import { get_attribute_expression, is_expression_attribute } from '../../../utils/ast.js';
14+
import { closing_tag_omitted } from '../../../../html-tree-validation.js';
1415

1516
// eslint-disable-next-line no-useless-escape
1617
const valid_tag_name = /^\!?[a-zA-Z]{1,}:?[a-zA-Z0-9\-]*/;

packages/svelte/src/compiler/phases/1-parse/utils/html.js

Lines changed: 0 additions & 46 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,3 @@
1-
import { interactive_elements } from '../../../../constants.js';
21
import entities from './entities.js';
32

43
const windows_1252 = [
@@ -119,48 +118,3 @@ function validate_code(code) {
119118

120119
return NUL;
121120
}
122-
123-
// based on http://developers.whatwg.org/syntax.html#syntax-tag-omission
124-
125-
/** @type {Record<string, Set<string>>} */
126-
const disallowed_contents = {
127-
li: new Set(['li']),
128-
dt: new Set(['dt', 'dd']),
129-
dd: new Set(['dt', 'dd']),
130-
p: new Set(
131-
'address article aside blockquote div dl fieldset footer form h1 h2 h3 h4 h5 h6 header hgroup hr main menu nav ol p pre section table ul'.split(
132-
' '
133-
)
134-
),
135-
rt: new Set(['rt', 'rp']),
136-
rp: new Set(['rt', 'rp']),
137-
optgroup: new Set(['optgroup']),
138-
option: new Set(['option', 'optgroup']),
139-
thead: new Set(['tbody', 'tfoot']),
140-
tbody: new Set(['tbody', 'tfoot']),
141-
tfoot: new Set(['tbody']),
142-
tr: new Set(['tr', 'tbody']),
143-
td: new Set(['td', 'th', 'tr']),
144-
th: new Set(['td', 'th', 'tr'])
145-
};
146-
147-
for (const interactive_element of interactive_elements) {
148-
disallowed_contents[interactive_element] = interactive_elements;
149-
}
150-
151-
// can this be a child of the parent element, or does it implicitly
152-
// close it, like `<li>one<li>two`?
153-
154-
/**
155-
* @param {string} current
156-
* @param {string} [next]
157-
*/
158-
export function closing_tag_omitted(current, next) {
159-
if (disallowed_contents[current]) {
160-
if (!next || disallowed_contents[current].has(next)) {
161-
return true;
162-
}
163-
}
164-
165-
return false;
166-
}

packages/svelte/src/compiler/phases/2-analyze/validation.js

Lines changed: 50 additions & 34 deletions
Original file line numberDiff line numberDiff line change
@@ -3,11 +3,6 @@
33
/** @import { NodeLike } from '../../errors.js' */
44
/** @import { AnalysisState, Context, Visitors } from './types.js' */
55
import is_reference from 'is-reference';
6-
import {
7-
disallowed_paragraph_contents,
8-
interactive_elements,
9-
is_tag_valid_with_parent
10-
} from '../../../constants.js';
116
import * as e from '../../errors.js';
127
import {
138
extract_identifiers,
@@ -37,6 +32,10 @@ import {
3732
import { Scope, get_rune } from '../scope.js';
3833
import { merge } from '../visitors.js';
3934
import { a11y_validators } from './a11y.js';
35+
import {
36+
is_tag_valid_with_ancestor,
37+
is_tag_valid_with_parent
38+
} from '../../../html-tree-validation.js';
4039

4140
/**
4241
* @param {Attribute} attribute
@@ -602,40 +601,57 @@ const validation = {
602601
validate_element(node, context);
603602

604603
if (context.state.parent_element) {
605-
if (!is_tag_valid_with_parent(node.name, context.state.parent_element)) {
606-
e.node_invalid_placement(node, `<${node.name}>`, context.state.parent_element);
607-
}
608-
}
604+
let past_parent = false;
605+
let only_warn = false;
609606

610-
// can't add form to interactive elements because those are also used by the parser
611-
// to check for the last auto-closing parent.
612-
if (node.name === 'form') {
613-
const path = context.path;
614-
for (let parent of path) {
615-
if (parent.type === 'RegularElement' && parent.name === 'form') {
616-
e.node_invalid_placement(node, `<${node.name}>`, parent.name);
617-
}
618-
}
619-
}
607+
for (let i = context.path.length - 1; i >= 0; i--) {
608+
const ancestor = context.path[i];
620609

621-
if (interactive_elements.has(node.name)) {
622-
const path = context.path;
623-
for (let parent of path) {
624610
if (
625-
parent.type === 'RegularElement' &&
626-
parent.name === node.name &&
627-
interactive_elements.has(parent.name)
611+
ancestor.type === 'IfBlock' ||
612+
ancestor.type === 'EachBlock' ||
613+
ancestor.type === 'AwaitBlock' ||
614+
ancestor.type === 'KeyBlock'
628615
) {
629-
e.node_invalid_placement(node, `<${node.name}>`, parent.name);
616+
// We're creating a separate template string inside blocks, which means client-side this would work
617+
only_warn = true;
630618
}
631-
}
632-
}
633619

634-
if (disallowed_paragraph_contents.includes(node.name)) {
635-
const path = context.path;
636-
for (let parent of path) {
637-
if (parent.type === 'RegularElement' && parent.name === 'p') {
638-
e.node_invalid_placement(node, `<${node.name}>`, parent.name);
620+
if (!past_parent) {
621+
if (
622+
ancestor.type === 'RegularElement' &&
623+
ancestor.name === context.state.parent_element
624+
) {
625+
if (!is_tag_valid_with_parent(node.name, context.state.parent_element)) {
626+
if (only_warn) {
627+
w.node_invalid_placement_ssr(
628+
node,
629+
`\`<${node.name}>\``,
630+
context.state.parent_element
631+
);
632+
} else {
633+
e.node_invalid_placement(node, `\`<${node.name}>\``, context.state.parent_element);
634+
}
635+
}
636+
637+
past_parent = true;
638+
}
639+
} else if (ancestor.type === 'RegularElement') {
640+
if (!is_tag_valid_with_ancestor(node.name, ancestor.name)) {
641+
if (only_warn) {
642+
w.node_invalid_placement_ssr(node, `\`<${node.name}>\``, ancestor.name);
643+
} else {
644+
e.node_invalid_placement(node, `\`<${node.name}>\``, ancestor.name);
645+
}
646+
}
647+
} else if (
648+
ancestor.type === 'Component' ||
649+
ancestor.type === 'SvelteComponent' ||
650+
ancestor.type === 'SvelteElement' ||
651+
ancestor.type === 'SvelteSelf' ||
652+
ancestor.type === 'SnippetBlock'
653+
) {
654+
break;
639655
}
640656
}
641657
}
@@ -818,7 +834,7 @@ const validation = {
818834
if (!node.parent) return;
819835
if (context.state.parent_element) {
820836
if (!is_tag_valid_with_parent('#text', context.state.parent_element)) {
821-
e.node_invalid_placement(node, '{expression}', context.state.parent_element);
837+
e.node_invalid_placement(node, '`{expression}`', context.state.parent_element);
822838
}
823839
}
824840
}

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
"component_name_lowercase",
115115
"element_invalid_self_closing_tag",
116116
"event_directive_deprecated",
117+
"node_invalid_placement_ssr",
117118
"slot_element_deprecated",
118119
"svelte_element_invalid_this"
119120
];
@@ -739,6 +740,16 @@ export function event_directive_deprecated(node, name) {
739740
w(node, "event_directive_deprecated", `Using \`on:${name}\` to listen to the ${name} event is deprecated. Use the event attribute \`on${name}\` instead`);
740741
}
741742

743+
/**
744+
* %thing% is invalid inside `<%parent%>`. When rendering this component on the server, the resulting HTML will be modified by the browser, likely resulting in a `hydration_mismatch` warning
745+
* @param {null | NodeLike} node
746+
* @param {string} thing
747+
* @param {string} parent
748+
*/
749+
export function node_invalid_placement_ssr(node, thing, parent) {
750+
w(node, "node_invalid_placement_ssr", `${thing} is invalid inside \`<${parent}>\`. When rendering this component on the server, the resulting HTML will be modified by the browser, likely resulting in a \`hydration_mismatch\` warning`);
751+
}
752+
742753
/**
743754
* Using `<slot>` to render parent content is deprecated. Use `{@render ...}` tags instead
744755
* @param {null | NodeLike} node

0 commit comments

Comments
 (0)