Skip to content

Commit 72fab2b

Browse files
edoardocavazzaedoardocavazza
committed
fix: improve custom element checks in html validation
1 parent ba37bbc commit 72fab2b

File tree

8 files changed

+115
-39
lines changed

8 files changed

+115
-39
lines changed

‎packages/svelte/src/compiler/phases/2-analyze/types.d.ts‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -10,10 +10,10 @@ export interface AnalysisState {
1010
options: ValidatedCompileOptions;
1111
ast_type: 'instance' | 'template' | 'module';
1212
/**
13-
* Tag name of the parent element. `null` if the parent is `svelte:element`, `#snippet`, a component or the root.
13+
* The parent element. `null` if the parent is `svelte:element`, `#snippet`, a component or the root.
1414
* Parent doesn't necessarily mean direct path predecessor because there could be `#each`, `#if` etc in-between.
1515
*/
16-
parent_element: string | null;
16+
parent_element: AST.RegularElement | null;
1717
has_props_rune: boolean;
1818
/** Which slots the current parent component has */
1919
component_slots: Set<string>;

‎packages/svelte/src/compiler/phases/2-analyze/visitors/ExpressionTag.js‎

Lines changed: 11 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@
22
/** @import { Context } from '../types' */
33
import { is_tag_valid_with_parent } from '../../../../html-tree-validation.js';
44
import * as e from '../../../errors.js';
5+
import { is_custom_element_node } from '../../nodes.js';
56
import { mark_subtree_dynamic } from './shared/fragment.js';
67

78
/**
@@ -12,7 +13,16 @@ export function ExpressionTag(node, context) {
1213
const in_template = context.path.at(-1)?.type === 'Fragment';
1314

1415
if (in_template && context.state.parent_element) {
15-
const message = is_tag_valid_with_parent('#text', context.state.parent_element);
16+
const message = is_tag_valid_with_parent(
17+
{
18+
tag: '#text',
19+
custom_element: false
20+
},
21+
{
22+
tag: context.state.parent_element.name,
23+
custom_element: is_custom_element_node(context.state.parent_element)
24+
}
25+
);
1626
if (message) {
1727
e.node_invalid_placement(node, message);
1828
}

‎packages/svelte/src/compiler/phases/2-analyze/visitors/RegularElement.js‎

Lines changed: 33 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -116,7 +116,12 @@ export function RegularElement(node, context) {
116116
if (context.state.parent_element) {
117117
let past_parent = false;
118118
let only_warn = false;
119-
const ancestors = [context.state.parent_element];
119+
const ancestors = [
120+
{
121+
tag: context.state.parent_element.name,
122+
custom_element: is_custom_element_node(context.state.parent_element)
123+
}
124+
];
120125

121126
for (let i = context.path.length - 1; i >= 0; i--) {
122127
const ancestor = context.path[i];
@@ -132,8 +137,20 @@ export function RegularElement(node, context) {
132137
}
133138

134139
if (!past_parent) {
135-
if (ancestor.type === 'RegularElement' && ancestor.name === context.state.parent_element) {
136-
const message = is_tag_valid_with_parent(node.name, context.state.parent_element);
140+
if (
141+
ancestor.type === 'RegularElement' &&
142+
ancestor.name === context.state.parent_element.name
143+
) {
144+
const message = is_tag_valid_with_parent(
145+
{
146+
tag: node.name,
147+
custom_element: is_custom_element_node(node)
148+
},
149+
{
150+
tag: context.state.parent_element.name,
151+
custom_element: is_custom_element_node(context.state.parent_element)
152+
}
153+
);
137154
if (message) {
138155
if (only_warn) {
139156
w.node_invalid_placement_ssr(node, message);
@@ -145,9 +162,18 @@ export function RegularElement(node, context) {
145162
past_parent = true;
146163
}
147164
} else if (ancestor.type === 'RegularElement') {
148-
ancestors.push(ancestor.name);
149-
150-
const message = is_tag_valid_with_ancestor(node.name, ancestors);
165+
ancestors.push({
166+
tag: ancestor.name,
167+
custom_element: is_custom_element_node(ancestor)
168+
});
169+
170+
const message = is_tag_valid_with_ancestor(
171+
{
172+
tag: node.name,
173+
custom_element: is_custom_element_node(node)
174+
},
175+
ancestors
176+
);
151177
if (message) {
152178
if (only_warn) {
153179
w.node_invalid_placement_ssr(node, message);
@@ -178,7 +204,7 @@ export function RegularElement(node, context) {
178204
w.element_invalid_self_closing_tag(node, node.name);
179205
}
180206

181-
context.next({ ...context.state, parent_element: node.name });
207+
context.next({ ...context.state, parent_element: node });
182208

183209
// Special case: <a> tags are valid in both the SVG and HTML namespace.
184210
// If there's no parent, look downwards to see if it's the parent of a SVG or HTML element.

‎packages/svelte/src/compiler/phases/2-analyze/visitors/Text.js‎

Lines changed: 11 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@
33
import { is_tag_valid_with_parent } from '../../../../html-tree-validation.js';
44
import { regex_not_whitespace } from '../../patterns.js';
55
import * as e from '../../../errors.js';
6+
import { is_custom_element_node } from '../../nodes.js';
67

78
/**
89
* @param {AST.Text} node
@@ -12,7 +13,16 @@ export function Text(node, context) {
1213
const in_template = context.path.at(-1)?.type === 'Fragment';
1314

1415
if (in_template && context.state.parent_element && regex_not_whitespace.test(node.data)) {
15-
const message = is_tag_valid_with_parent('#text', context.state.parent_element);
16+
const message = is_tag_valid_with_parent(
17+
{
18+
tag: '#text',
19+
custom_element: false
20+
},
21+
{
22+
tag: context.state.parent_element.name,
23+
custom_element: is_custom_element_node(context.state.parent_element)
24+
}
25+
);
1626
if (message) {
1727
e.node_invalid_placement(node, message);
1828
}

‎packages/svelte/src/compiler/phases/3-transform/server/visitors/RegularElement.js‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@
55
import { is_void } from '../../../../../utils.js';
66
import { dev, locator } from '../../../../state.js';
77
import * as b from '../../../../utils/builders.js';
8+
import { is_custom_element_node } from '../../../nodes.js';
89
import { clean_nodes, determine_namespace_for_children } from '../../utils.js';
910
import { build_element_attributes } from './shared/element.js';
1011
import { process_children, build_template } from './shared/utils.js';
@@ -62,6 +63,7 @@ export function RegularElement(node, context) {
6263
'$.push_element',
6364
b.id('$$payload'),
6465
b.literal(node.name),
66+
b.literal(is_custom_element_node(node)),
6567
b.literal(location.line),
6668
b.literal(location.column)
6769
)

‎packages/svelte/src/compiler/phases/3-transform/server/visitors/SvelteElement.js‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -47,6 +47,7 @@ export function SvelteElement(node, context) {
4747
'$.push_element',
4848
b.id('$$payload'),
4949
tag,
50+
b.literal(false),
5051
b.literal(location.line),
5152
b.literal(location.column)
5253
)

‎packages/svelte/src/html-tree-validation.js‎

Lines changed: 31 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,10 @@
1+
/**
2+
* @typedef {{
3+
* tag: string;
4+
* custom_element: boolean;
5+
* }} Element
6+
*/
7+
18
/**
29
* Map of elements that have certain elements that are not allowed inside them, in the sense that they will auto-close the parent/ancestor element.
310
* Theoretically one could take advantage of it but most of the time it will just result in confusing behavior and break when SSR'd.
@@ -137,33 +144,33 @@ const disallowed_children = {
137144
/**
138145
* Returns an error message if the tag is not allowed inside the ancestor tag (which is grandparent and above) such that it will result
139146
* in the browser repairing the HTML, which will likely result in an error during hydration.
140-
* @param {string} child_tag
141-
* @param {string[]} ancestors All nodes starting with the parent, up until the ancestor, which means two entries minimum
147+
* @param {Element} child_node
148+
* @param {Element[]} ancestors All nodes starting with the parent, up until the ancestor, which means two entries minimum
142149
* @param {string} [child_loc]
143150
* @param {string} [ancestor_loc]
144151
* @returns {string | null}
145152
*/
146-
export function is_tag_valid_with_ancestor(child_tag, ancestors, child_loc, ancestor_loc) {
147-
if (child_tag.includes('-')) return null; // custom elements can be anything
153+
export function is_tag_valid_with_ancestor(child_node, ancestors, child_loc, ancestor_loc) {
154+
if (child_node.custom_element) return null; // custom elements can be anything
148155

149-
const ancestor_tag = ancestors[ancestors.length - 1];
156+
const ancestor_tag = ancestors[ancestors.length - 1].tag;
150157
const disallowed = disallowed_children[ancestor_tag];
151158
if (!disallowed) return null;
152159

153160
if ('reset_by' in disallowed && disallowed.reset_by) {
154161
for (let i = ancestors.length - 2; i >= 0; i--) {
155162
const ancestor = ancestors[i];
156-
if (ancestor.includes('-')) return null; // custom elements can be anything
163+
if (ancestor.custom_element) return null; // custom elements can be anything
157164

158165
// A reset means that forbidden descendants are allowed again
159-
if (disallowed.reset_by.includes(ancestors[i])) {
166+
if (disallowed.reset_by.includes(ancestors[i].tag)) {
160167
return null;
161168
}
162169
}
163170
}
164171

165-
if ('descendant' in disallowed && disallowed.descendant.includes(child_tag)) {
166-
const child = child_loc ? `\`<${child_tag}>\` (${child_loc})` : `\`<${child_tag}>\``;
172+
if ('descendant' in disallowed && disallowed.descendant.includes(child_node.tag)) {
173+
const child = child_loc ? `\`<${child_node.tag}>\` (${child_loc})` : `\`<${child_node.tag}>\``;
167174
const ancestor = ancestor_loc
168175
? `\`<${ancestor_tag}>\` (${ancestor_loc})`
169176
: `\`<${ancestor_tag}>\``;
@@ -177,36 +184,38 @@ export function is_tag_valid_with_ancestor(child_tag, ancestors, child_loc, ance
177184
/**
178185
* Returns an error message if the tag is not allowed inside the parent tag such that it will result
179186
* in the browser repairing the HTML, which will likely result in an error during hydration.
180-
* @param {string} child_tag
181-
* @param {string} parent_tag
187+
* @param {Element} child_node
188+
* @param {Element} parent_node
182189
* @param {string} [child_loc]
183190
* @param {string} [parent_loc]
184191
* @returns {string | null}
185192
*/
186-
export function is_tag_valid_with_parent(child_tag, parent_tag, child_loc, parent_loc) {
187-
if (child_tag.includes('-') || parent_tag?.includes('-')) return null; // custom elements can be anything
193+
export function is_tag_valid_with_parent(child_node, parent_node, child_loc, parent_loc) {
194+
if (child_node.custom_element || parent_node?.custom_element) return null; // custom elements can be anything
188195

189-
if (parent_tag === 'template') return null; // no errors or warning should be thrown in immediate children of template tags
196+
if (parent_node.tag === 'template') return null; // no errors or warning should be thrown in immediate children of template tags
190197

191-
const disallowed = disallowed_children[parent_tag];
198+
const disallowed = disallowed_children[parent_node.tag];
192199

193-
const child = child_loc ? `\`<${child_tag}>\` (${child_loc})` : `\`<${child_tag}>\``;
194-
const parent = parent_loc ? `\`<${parent_tag}>\` (${parent_loc})` : `\`<${parent_tag}>\``;
200+
const child = child_loc ? `\`<${child_node.tag}>\` (${child_loc})` : `\`<${child_node.tag}>\``;
201+
const parent = parent_loc
202+
? `\`<${parent_node.tag}>\` (${parent_loc})`
203+
: `\`<${parent_node.tag}>\``;
195204

196205
if (disallowed) {
197-
if ('direct' in disallowed && disallowed.direct.includes(child_tag)) {
206+
if ('direct' in disallowed && disallowed.direct.includes(child_node.tag)) {
198207
return `${child} cannot be a direct child of ${parent}`;
199208
}
200209

201-
if ('descendant' in disallowed && disallowed.descendant.includes(child_tag)) {
210+
if ('descendant' in disallowed && disallowed.descendant.includes(child_node.tag)) {
202211
return `${child} cannot be a child of ${parent}`;
203212
}
204213

205214
if ('only' in disallowed && disallowed.only) {
206-
if (disallowed.only.includes(child_tag)) {
215+
if (disallowed.only.includes(child_node.tag)) {
207216
return null;
208217
} else {
209-
return `${child} cannot be a child of ${parent}. \`<${parent_tag}>\` only allows these children: ${disallowed.only.map((d) => `\`<${d}>\``).join(', ')}`;
218+
return `${child} cannot be a child of ${parent}. \`<${parent_node.tag}>\` only allows these children: ${disallowed.only.map((d) => `\`<${d}>\``).join(', ')}`;
210219
}
211220
}
212221
}
@@ -215,7 +224,7 @@ export function is_tag_valid_with_parent(child_tag, parent_tag, child_loc, paren
215224
// parsing rules - if we're down here, then none of those matched and
216225
// so we allow it only if we don't know what the parent is, as all other
217226
// cases are invalid (and we only get into this function if we know the parent).
218-
switch (child_tag) {
227+
switch (child_node.tag) {
219228
case 'body':
220229
case 'caption':
221230
case 'col':

‎packages/svelte/src/internal/server/dev.js‎

Lines changed: 24 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,7 @@ import { current_component } from './context.js';
99
/**
1010
* @typedef {{
1111
* tag: string;
12+
* custom_element: boolean;
1213
* parent: null | Element;
1314
* filename: null | string;
1415
* line: number;
@@ -60,32 +61,49 @@ export function reset_elements() {
6061
/**
6162
* @param {Payload} payload
6263
* @param {string} tag
64+
* @param {boolean} custom_element
6365
* @param {number} line
6466
* @param {number} column
6567
*/
66-
export function push_element(payload, tag, line, column) {
68+
export function push_element(payload, tag, custom_element, line, column) {
6769
var filename = /** @type {Component} */ (current_component).function[FILENAME];
68-
var child = { tag, parent, filename, line, column };
70+
var child = { tag, custom_element, parent, filename, line, column };
6971

7072
if (parent !== null) {
7173
var ancestor = parent.parent;
72-
var ancestors = [parent.tag];
74+
var ancestors = [parent];
7375

7476
const child_loc = filename ? `${filename}:${line}:${column}` : undefined;
7577
const parent_loc = parent.filename
7678
? `${parent.filename}:${parent.line}:${parent.column}`
7779
: undefined;
7880

79-
const message = is_tag_valid_with_parent(tag, parent.tag, child_loc, parent_loc);
81+
const message = is_tag_valid_with_parent(
82+
{
83+
tag,
84+
custom_element
85+
},
86+
parent,
87+
child_loc,
88+
parent_loc
89+
);
8090
if (message) print_error(payload, message);
8191

8292
while (ancestor != null) {
83-
ancestors.push(ancestor.tag);
93+
ancestors.push(ancestor);
8494
const ancestor_loc = ancestor.filename
8595
? `${ancestor.filename}:${ancestor.line}:${ancestor.column}`
8696
: undefined;
8797

88-
const message = is_tag_valid_with_ancestor(tag, ancestors, child_loc, ancestor_loc);
98+
const message = is_tag_valid_with_ancestor(
99+
{
100+
tag,
101+
custom_element
102+
},
103+
ancestors,
104+
child_loc,
105+
ancestor_loc
106+
);
89107
if (message) print_error(payload, message);
90108

91109
ancestor = ancestor.parent;

0 commit comments

Comments
 (0)