Skip to content

Commit

Permalink
chore: tweak html tree validation
Browse files Browse the repository at this point in the history
- 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
  • Loading branch information
dummdidumm committed Jul 26, 2024
1 parent 3515776 commit 78ed8e4
Show file tree
Hide file tree
Showing 15 changed files with 375 additions and 281 deletions.
8 changes: 8 additions & 0 deletions packages/svelte/messages/compile-errors/template.md
Original file line number Diff line number Diff line change
Expand Up @@ -192,6 +192,14 @@

> %thing% is invalid inside <%parent%>
HTML has some restrictions where certain elements can appear. For example, a `<div>` inside a `<p>` is invalid. Some violations "only" result in invalid HTML, others result in the HTML being repaired by the browser, resulting in content shifting around. Some examples:

- `<p>hello <div>world</div></p>` will result in `<p>hello </p><div>world</div><p></p>` for example (the `<div>` autoclosed the `<p>`)
- `<option><div>option a</div></select>` will result in `<option>option a</option>` (the `<div>` is removed)
- `<table><tr><td>cell</td></tr></table>` will result in `<table><tbody><tr><td>cell</td></tr></tbody></table>` (a `<tbody>` is auto-inserted)

Svelte throws a compiler error when it detects that it will generate the HTML in such a way that it will always be repaired and result in the runtime code not finding the nodes at the expected locations.

## render_tag_invalid_call_expression

> Calling a snippet function using apply, bind or call is not allowed
Expand Down
12 changes: 12 additions & 0 deletions packages/svelte/messages/compile-warnings/template.md
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,18 @@

> Using `on:%name%` to listen to the %name% event is deprecated. Use the event attribute `on%name%` instead
## 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
HTML has some restrictions where certain elements can appear. For example, a `<div>` inside a `<p>` is invalid. Some violations "only" result in invalid HTML, others result in the HTML being repaired by the browser, resulting in content shifting around. Some examples:

- `<p>hello <div>world</div></p>` will result in `<p>hello </p><div>world</div><p></p>` for example (the `<div>` autoclosed the `<p>`)
- `<option><div>option a</div></select>` will result in `<option>option a</option>` (the `<div>` is removed)
- `<table><tr><td>cell</td></tr></table>` will result in `<table><tbody><tr><td>cell</td></tr></tbody></table>` (a `<tbody>` is auto-inserted)

Svelte issues a compiler warning when it detects that it will generate the HTML in such a way that it will work on the client, but always fail when using server side rendering, because the resulting HTML will be repaired and result in the client runtime not finding the nodes at the expected locations when hydrating the DOM.

## slot_element_deprecated

> Using `<slot>` to render parent content is deprecated. Use `{@render ...}` tags instead
Expand Down
3 changes: 2 additions & 1 deletion packages/svelte/src/compiler/phases/1-parse/state/element.js
Original file line number Diff line number Diff line change
Expand Up @@ -5,12 +5,13 @@ import { is_void } from '../../../../constants.js';
import read_expression from '../read/expression.js';
import { read_script } from '../read/script.js';
import read_style from '../read/style.js';
import { closing_tag_omitted, decode_character_references } from '../utils/html.js';
import { decode_character_references } from '../utils/html.js';
import * as e from '../../../errors.js';
import * as w from '../../../warnings.js';
import { create_fragment } from '../utils/create.js';
import { create_attribute } from '../../nodes.js';
import { get_attribute_expression, is_expression_attribute } from '../../../utils/ast.js';
import { closing_tag_omitted } from '../../../../html-tree-validation.js';

// eslint-disable-next-line no-useless-escape
const valid_tag_name = /^\!?[a-zA-Z]{1,}:?[a-zA-Z0-9\-]*/;
Expand Down
46 changes: 0 additions & 46 deletions packages/svelte/src/compiler/phases/1-parse/utils/html.js
Original file line number Diff line number Diff line change
@@ -1,4 +1,3 @@
import { interactive_elements } from '../../../../constants.js';
import entities from './entities.js';

const windows_1252 = [
Expand Down Expand Up @@ -119,48 +118,3 @@ function validate_code(code) {

return NUL;
}

// based on http://developers.whatwg.org/syntax.html#syntax-tag-omission

/** @type {Record<string, Set<string>>} */
const disallowed_contents = {
li: new Set(['li']),
dt: new Set(['dt', 'dd']),
dd: new Set(['dt', 'dd']),
p: new Set(
'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(
' '
)
),
rt: new Set(['rt', 'rp']),
rp: new Set(['rt', 'rp']),
optgroup: new Set(['optgroup']),
option: new Set(['option', 'optgroup']),
thead: new Set(['tbody', 'tfoot']),
tbody: new Set(['tbody', 'tfoot']),
tfoot: new Set(['tbody']),
tr: new Set(['tr', 'tbody']),
td: new Set(['td', 'th', 'tr']),
th: new Set(['td', 'th', 'tr'])
};

for (const interactive_element of interactive_elements) {
disallowed_contents[interactive_element] = interactive_elements;
}

// can this be a child of the parent element, or does it implicitly
// close it, like `<li>one<li>two`?

/**
* @param {string} current
* @param {string} [next]
*/
export function closing_tag_omitted(current, next) {
if (disallowed_contents[current]) {
if (!next || disallowed_contents[current].has(next)) {
return true;
}
}

return false;
}
78 changes: 45 additions & 33 deletions packages/svelte/src/compiler/phases/2-analyze/validation.js
Original file line number Diff line number Diff line change
Expand Up @@ -3,11 +3,6 @@
/** @import { NodeLike } from '../../errors.js' */
/** @import { AnalysisState, Context, Visitors } from './types.js' */
import is_reference from 'is-reference';
import {
disallowed_paragraph_contents,
interactive_elements,
is_tag_valid_with_parent
} from '../../../constants.js';
import * as e from '../../errors.js';
import {
extract_identifiers,
Expand Down Expand Up @@ -37,6 +32,10 @@ import {
import { Scope, get_rune } from '../scope.js';
import { merge } from '../visitors.js';
import { a11y_validators } from './a11y.js';
import {
is_tag_valid_with_ancestor,
is_tag_valid_with_parent
} from '../../../html-tree-validation.js';

/**
* @param {Attribute} attribute
Expand Down Expand Up @@ -602,40 +601,53 @@ const validation = {
validate_element(node, context);

if (context.state.parent_element) {
if (!is_tag_valid_with_parent(node.name, context.state.parent_element)) {
e.node_invalid_placement(node, `<${node.name}>`, context.state.parent_element);
}
}
let past_parent = false;
let only_warn = false;

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

if (interactive_elements.has(node.name)) {
const path = context.path;
for (let parent of path) {
if (
parent.type === 'RegularElement' &&
parent.name === node.name &&
interactive_elements.has(parent.name)
ancestor.type === 'IfBlock' ||
ancestor.type === 'EachBlock' ||
ancestor.type === 'AwaitBlock' ||
ancestor.type === 'KeyBlock'
) {
e.node_invalid_placement(node, `<${node.name}>`, parent.name);
// We're creating a separate template string inside blocks, which means client-side this would work
only_warn = true;
}
}
}

if (disallowed_paragraph_contents.includes(node.name)) {
const path = context.path;
for (let parent of path) {
if (parent.type === 'RegularElement' && parent.name === 'p') {
e.node_invalid_placement(node, `<${node.name}>`, parent.name);
if (!past_parent) {
if (
ancestor.type === 'RegularElement' &&
ancestor.name === context.state.parent_element
) {
if (!is_tag_valid_with_parent(node.name, context.state.parent_element)) {
if (only_warn) {
w.node_invalid_placement_ssr(node, `<${node.name}>`, context.state.parent_element);
} else {
e.node_invalid_placement(node, `<${node.name}>`, context.state.parent_element);
}
}

past_parent = true;
}
} else if (ancestor.type === 'RegularElement') {
if (!is_tag_valid_with_ancestor(node.name, ancestor.name)) {
if (only_warn) {
w.node_invalid_placement_ssr(node, `<${node.name}>`, ancestor.name);
} else {
e.node_invalid_placement(node, `<${node.name}>`, ancestor.name);
}
}
} else if (
ancestor.type === 'Component' ||
ancestor.type === 'SvelteComponent' ||
ancestor.type === 'SvelteElement' ||
ancestor.type === 'SvelteSelf' ||
ancestor.type === 'SnippetBlock'
) {
break;
}
}
}
Expand Down
11 changes: 11 additions & 0 deletions packages/svelte/src/compiler/warnings.js
Original file line number Diff line number Diff line change
Expand Up @@ -114,6 +114,7 @@ export const codes = [
"component_name_lowercase",
"element_invalid_self_closing_tag",
"event_directive_deprecated",
"node_invalid_placement_ssr",
"slot_element_deprecated",
"svelte_element_invalid_this"
];
Expand Down Expand Up @@ -739,6 +740,16 @@ export function event_directive_deprecated(node, name) {
w(node, "event_directive_deprecated", `Using \`on:${name}\` to listen to the ${name} event is deprecated. Use the event attribute \`on${name}\` instead`);
}

/**
* %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
* @param {null | NodeLike} node
* @param {string} thing
* @param {string} parent
*/
export function node_invalid_placement_ssr(node, thing, parent) {
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`);
}

/**
* Using `<slot>` to render parent content is deprecated. Use `{@render ...}` tags instead
* @param {null | NodeLike} node
Expand Down
Loading

0 comments on commit 78ed8e4

Please sign in to comment.