Skip to content

feat: improve ssr html mismatch validation #10658

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 5 commits into from
Feb 28, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions .changeset/hungry-singers-share.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
"svelte": patch
---

feat: improve ssr html mismatch validation
41 changes: 1 addition & 40 deletions packages/svelte/src/compiler/phases/1-parse/utils/html.js
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
import { interactive_elements } from '../../../../constants.js';
import entities from './entities.js';

const windows_1252 = [
Expand Down Expand Up @@ -121,16 +122,6 @@ function validate_code(code) {

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

// while `input` is also an interactive element, it is never moved by the browser, so we don't need to check for it
export const interactive_elements = new Set([
'a',
'button',
'iframe',
'embed',
'select',
'textarea'
]);

/** @type {Record<string, Set<string>>} */
const disallowed_contents = {
li: new Set(['li']),
Expand All @@ -153,36 +144,6 @@ const disallowed_contents = {
th: new Set(['td', 'th', 'tr'])
};

export const disallowed_parapgraph_contents = [
'address',
'article',
'aside',
'blockquote',
'details',
'div',
'dl',
'fieldset',
'figcapture',
'figure',
'footer',
'form',
'h1',
'h2',
'h3',
'h4',
'h5',
'h6',
'header',
'hr',
'menu',
'nav',
'ol',
'pre',
'section',
'table',
'ul'
];

for (const interactive_element of interactive_elements) {
disallowed_contents[interactive_element] = interactive_elements;
}
Expand Down
127 changes: 5 additions & 122 deletions packages/svelte/src/compiler/phases/2-analyze/validation.js
Original file line number Diff line number Diff line change
@@ -1,3 +1,8 @@
import {
disallowed_parapgraph_contents,
interactive_elements,
is_tag_valid_with_parent
} from '../../../constants.js';
import { error } from '../../errors.js';
import {
extract_identifiers,
Expand All @@ -8,7 +13,6 @@ import {
} from '../../utils/ast.js';
import { warn } from '../../warnings.js';
import fuzzymatch from '../1-parse/utils/fuzzymatch.js';
import { disallowed_parapgraph_contents, interactive_elements } from '../1-parse/utils/html.js';
import { binding_properties } from '../bindings.js';
import { ContentEditableBindings, EventModifiers, SVGElements } from '../constants.js';
import { is_custom_element_node } from '../nodes.js';
Expand Down Expand Up @@ -226,127 +230,6 @@ function validate_slot_attribute(context, attribute) {
}
}

// https://html.spec.whatwg.org/multipage/syntax.html#generate-implied-end-tags
const implied_end_tags = ['dd', 'dt', 'li', 'option', 'optgroup', 'p', 'rp', 'rt'];

/**
* @param {string} tag
* @param {string} parent_tag
* @returns {boolean}
*/
function is_tag_valid_with_parent(tag, parent_tag) {
// First, let's check if we're in an unusual parsing mode...
switch (parent_tag) {
// https://html.spec.whatwg.org/multipage/syntax.html#parsing-main-inselect
case 'select':
return tag === 'option' || tag === 'optgroup' || tag === '#text';
case 'optgroup':
return tag === 'option' || tag === '#text';
// Strictly speaking, seeing an <option> doesn't mean we're in a <select>
// but
case 'option':
return tag === '#text';
// https://html.spec.whatwg.org/multipage/syntax.html#parsing-main-intd
// https://html.spec.whatwg.org/multipage/syntax.html#parsing-main-incaption
// No special behavior since these rules fall back to "in body" mode for
// all except special table nodes which cause bad parsing behavior anyway.

// https://html.spec.whatwg.org/multipage/syntax.html#parsing-main-intr
case 'tr':
return (
tag === 'th' || tag === 'td' || tag === 'style' || tag === 'script' || tag === 'template'
);
// https://html.spec.whatwg.org/multipage/syntax.html#parsing-main-intbody
case 'tbody':
case 'thead':
case 'tfoot':
return tag === 'tr' || tag === 'style' || tag === 'script' || tag === 'template';
// https://html.spec.whatwg.org/multipage/syntax.html#parsing-main-incolgroup
case 'colgroup':
return tag === 'col' || tag === 'template';
// https://html.spec.whatwg.org/multipage/syntax.html#parsing-main-intable
case 'table':
return (
tag === 'caption' ||
tag === 'colgroup' ||
tag === 'tbody' ||
tag === 'tfoot' ||
tag === 'thead' ||
tag === 'style' ||
tag === 'script' ||
tag === 'template'
);
// https://html.spec.whatwg.org/multipage/syntax.html#parsing-main-inhead
case 'head':
return (
tag === 'base' ||
tag === 'basefont' ||
tag === 'bgsound' ||
tag === 'link' ||
tag === 'meta' ||
tag === 'title' ||
tag === 'noscript' ||
tag === 'noframes' ||
tag === 'style' ||
tag === 'script' ||
tag === 'template'
);
// https://html.spec.whatwg.org/multipage/semantics.html#the-html-element
case 'html':
return tag === 'head' || tag === 'body' || tag === 'frameset';
case 'frameset':
return tag === 'frame';
case '#document':
return tag === 'html';
}

// Probably in the "in body" parsing mode, so we outlaw only tag combos
// where the parsing rules cause implicit opens or closes to be added.
// https://html.spec.whatwg.org/multipage/syntax.html#parsing-main-inbody
switch (tag) {
case 'h1':
case 'h2':
case 'h3':
case 'h4':
case 'h5':
case 'h6':
return (
parent_tag !== 'h1' &&
parent_tag !== 'h2' &&
parent_tag !== 'h3' &&
parent_tag !== 'h4' &&
parent_tag !== 'h5' &&
parent_tag !== 'h6'
);

case 'rp':
case 'rt':
return implied_end_tags.indexOf(parent_tag) === -1;

case 'body':
case 'caption':
case 'col':
case 'colgroup':
case 'frameset':
case 'frame':
case 'head':
case 'html':
case 'tbody':
case 'td':
case 'tfoot':
case 'th':
case 'thead':
case 'tr':
// These tags are only valid with a few parents that have special child
// parsing rules -- if we're down here, then none of those matched and
// so we allow it only if we don't know what the parent is, as all other
// cases are invalid.
return parent_tag == null;
}

return true;
}

/**
* @type {import('zimmerframe').Visitors<import('#compiler').SvelteNode, import('./types.js').AnalysisState>}
*/
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1207,6 +1207,12 @@ const template_visitors = {
inner_context.visit(node, state);
}

if (context.state.options.dev) {
context.state.template.push(
t_statement(b.stmt(b.call('$.push_element', b.literal(node.name), b.id('$$payload'))))
);
}

process_children(trimmed, node, inner_context);

if (body_expression !== null) {
Expand Down Expand Up @@ -1239,6 +1245,9 @@ const template_visitors = {
if (!VoidElements.includes(node.name) && metadata.namespace !== 'foreign') {
context.state.template.push(t_string(`</${node.name}>`));
}
if (context.state.options.dev) {
context.state.template.push(t_statement(b.stmt(b.call('$.pop_element'))));
}
},
SvelteElement(node, context) {
let tag = /** @type {import('estree').Expression} */ (context.visit(node.tag));
Expand Down Expand Up @@ -1281,6 +1290,12 @@ const template_visitors = {

serialize_element_attributes(node, inner_context);

if (context.state.options.dev) {
context.state.template.push(
t_statement(b.stmt(b.call('$.push_element', tag, b.id('$$payload'))))
);
}

context.state.template.push(
t_statement(
b.if(
Expand All @@ -1304,6 +1319,9 @@ const template_visitors = {
),
t_expression(anchor_id)
);
if (context.state.options.dev) {
context.state.template.push(t_statement(b.stmt(b.call('$.pop_element'))));
}
},
EachBlock(node, context) {
const state = context.state;
Expand Down
Loading