chore: improve SSR invalid element error message (#11585)

* chore: improve SSR invalid element error message

* move push_element and pop_element into new dev.js file

* pass location info, remove unnecessary if (DEV) block

* use full filename, basename is not very helpful. also, current_component is guaranteed to not be null

* current_element is guaranteed to not be null in pop_element

* tweaks

* remove message prefix - redundant when filenames are included

* add line/column

* make message more concise

* reduce indirection

* only print message once

* update test

---------

Co-authored-by: Rich Harris <rich.harris@vercel.com>
pull/11621/head
Dominic Gannaway 1 year ago committed by GitHub
parent e97bc79f02
commit 1087e6fb54
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194

@ -0,0 +1,5 @@
---
"svelte": patch
---
chore: improve SSR invalid element error message

@ -27,6 +27,7 @@ import { DOMBooleanAttributes, HYDRATION_END, HYDRATION_START } from '../../../.
import { escape_html } from '../../../../escaping.js';
import { sanitize_template_string } from '../../../utils/sanitize_template_string.js';
import { BLOCK_CLOSE, BLOCK_CLOSE_ELSE } from '../../../../internal/server/hydration.js';
import { locator } from '../../../state.js';
export const block_open = t_string(`<!--${HYDRATION_START}-->`);
export const block_close = t_string(`<!--${HYDRATION_END}-->`);
@ -1364,8 +1365,19 @@ const template_visitors = {
}
if (context.state.options.dev) {
const location = /** @type {import('locate-character').Location} */ (locator(node.start));
context.state.template.push(
t_statement(b.stmt(b.call('$.push_element', b.literal(node.name), b.id('$$payload'))))
t_statement(
b.stmt(
b.call(
'$.push_element',
b.id('$$payload'),
b.literal(node.name),
b.literal(location.line),
b.literal(location.column)
)
)
)
);
}
@ -2279,9 +2291,12 @@ export function server_component(analysis, options) {
// undefined to a binding that has a default value.
template.body.push(b.stmt(b.call('$.bind_props', b.id('$$props'), b.object(props))));
}
/** @type {import('estree').Expression[]} */
const push_args = [];
if (options.dev) push_args.push(b.id(analysis.name));
const component_block = b.block([
b.stmt(b.call('$.push', b.literal(analysis.runes))),
b.stmt(b.call('$.push', ...push_args)),
.../** @type {import('estree').Statement[]} */ (instance.body),
.../** @type {import('estree').Statement[]} */ (template.body),
b.stmt(b.call('$.pop'))
@ -2376,6 +2391,22 @@ export function server_component(analysis, options) {
body.push(b.export_default(component_function));
}
if (options.dev && options.filename) {
let filename = options.filename;
if (/(\/|\w:)/.test(options.filename)) {
// filename is absolute — truncate it
const parts = filename.split(/[/\\]/);
filename = parts.length > 3 ? ['...', ...parts.slice(-3)].join('/') : filename;
}
// add `App.filename = 'App.svelte'` so that we can print useful messages later
body.unshift(
b.stmt(
b.assignment('=', b.member(b.id(analysis.name), b.id('filename')), b.literal(filename))
)
);
}
return {
type: 'Program',
sourceType: 'module',

@ -53,8 +53,15 @@ function get_or_init_context_map(name) {
return (current_component.c ??= new Map(get_parent_context(current_component) || undefined));
}
export function push() {
/**
* @param {Function} [fn]
*/
export function push(fn) {
current_component = { p: current_component, c: null, d: null };
if (DEV) {
// component function
current_component.function = fn;
}
}
export function pop() {

@ -0,0 +1,93 @@
import {
disallowed_paragraph_contents,
interactive_elements,
is_tag_valid_with_parent
} from '../../constants.js';
import { current_component } from './context.js';
/**
* @typedef {{
* tag: string;
* parent: null | Element;
* filename: null | string;
* line: number;
* column: number;
* }} Element
*/
/**
* @type {Element | null}
*/
let parent = null;
/** @type {Set<string>} */
let seen;
/**
* @param {Element} element
*/
function stringify(element) {
if (element.filename === null) return `\`<${element.tag}>\``;
return `\`<${element.tag}>\` (${element.filename}:${element.line}:${element.column})`;
}
/**
* @param {import('#server').Payload} payload
* @param {Element} parent
* @param {Element} child
*/
function print_error(payload, parent, child) {
var message =
`${stringify(child)} cannot contain ${stringify(parent)}\n\n` +
'This can cause content to shift around as the browser repairs the HTML, and will likely result in a `hydration_mismatch` warning.';
if ((seen ??= new Set()).has(message)) return;
seen.add(message);
// eslint-disable-next-line no-console
console.error(message);
payload.head.out += `<script>console.error(${JSON.stringify(message)})</script>`;
}
/**
* @param {import('#server').Payload} payload
* @param {string} tag
* @param {number} line
* @param {number} column
*/
export function push_element(payload, tag, line, column) {
var filename = /** @type {import('#server').Component} */ (current_component).function.filename;
var child = { tag, parent, filename, line, column };
if (parent !== null && !is_tag_valid_with_parent(tag, parent.tag)) {
print_error(payload, parent, child);
}
if (interactive_elements.has(tag)) {
let element = parent;
while (element !== null) {
if (interactive_elements.has(element.tag)) {
print_error(payload, element, child);
break;
}
element = element.parent;
}
}
if (disallowed_paragraph_contents.includes(tag)) {
let element = parent;
while (element !== null) {
if (element.tag === 'p') {
print_error(payload, element, child);
break;
}
element = element.parent;
}
}
parent = child;
}
export function pop_element() {
parent = /** @type {Element} */ (parent).parent;
}

@ -1,26 +1,12 @@
import { is_promise, noop } from '../shared/utils.js';
import { subscribe_to_store } from '../../store/utils.js';
import {
UNINITIALIZED,
DOMBooleanAttributes,
RawTextElements,
disallowed_paragraph_contents,
interactive_elements,
is_tag_valid_with_parent
} from '../../constants.js';
import { UNINITIALIZED, DOMBooleanAttributes, RawTextElements } from '../../constants.js';
import { escape_html } from '../../escaping.js';
import { DEV } from 'esm-env';
import { current_component, pop, push } from './context.js';
import { BLOCK_CLOSE, BLOCK_OPEN } from './hydration.js';
import { validate_store } from '../shared/validate.js';
/**
* @typedef {{
* tag: string;
* parent: null | Element;
* }} Element
*/
/**
* @typedef {{
* head: string;
@ -28,18 +14,6 @@ import { validate_store } from '../shared/validate.js';
* }} RenderOutput
*/
/**
* @typedef {{
* out: string;
* anchor: number;
* head: {
* title: string;
* out: string;
* anchor: number;
* };
* }} Payload
*/
// https://html.spec.whatwg.org/multipage/syntax.html#attributes-2
// https://infra.spec.whatwg.org/#noncharacter
const INVALID_ATTR_NAME_CHAR_REGEX =
@ -64,19 +38,14 @@ export const VoidElements = new Set([
'wbr'
]);
/**
* @type {Element | null}
*/
let current_element = null;
/** @returns {Payload} */
/** @returns {import('#server').Payload} */
function create_payload() {
return { out: '', head: { title: '', out: '', anchor: 0 }, anchor: 0 };
}
/**
* @param {Payload} to_copy
* @returns {Payload}
* @param {import('#server').Payload} to_copy
* @returns {import('#server').Payload}
*/
export function copy_payload(to_copy) {
return {
@ -87,8 +56,8 @@ export function copy_payload(to_copy) {
/**
* Assigns second payload to first
* @param {Payload} p1
* @param {Payload} p2
* @param {import('#server').Payload} p1
* @param {import('#server').Payload} p2
* @returns {void}
*/
export function assign_payload(p1, p2) {
@ -98,59 +67,7 @@ export function assign_payload(p1, p2) {
}
/**
* @param {Payload} payload
* @param {string} message
*/
function error_on_client(payload, message) {
message =
`Svelte SSR validation error:\n\n${message}\n\n` +
'Ensure your components render valid HTML as the browser will try to repair invalid HTML, ' +
'which may result in content being shifted around and will likely result in a hydration mismatch.';
// eslint-disable-next-line no-console
console.error(message);
payload.head.out += `<script>console.error(\`${message}\`)</script>`;
}
/**
* @param {string} tag
* @param {Payload} payload
*/
export function push_element(tag, payload) {
if (current_element !== null && !is_tag_valid_with_parent(tag, current_element.tag)) {
error_on_client(payload, `<${tag}> is invalid inside <${current_element.tag}>`);
}
if (interactive_elements.has(tag)) {
let element = current_element;
while (element !== null) {
if (interactive_elements.has(element.tag)) {
error_on_client(payload, `<${tag}> is invalid inside <${element.tag}>`);
}
element = element.parent;
}
}
if (disallowed_paragraph_contents.includes(tag)) {
let element = current_element;
while (element !== null) {
if (element.tag === 'p') {
error_on_client(payload, `<${tag}> is invalid inside <p>`);
}
element = element.parent;
}
}
current_element = {
tag,
parent: current_element
};
}
export function pop_element() {
if (current_element !== null) {
current_element = current_element.parent;
}
}
/**
* @param {Payload} payload
* @param {import('#server').Payload} payload
* @param {string} tag
* @param {() => void} attributes_fn
* @param {() => void} children_fn
@ -214,8 +131,8 @@ export function render(component, options) {
}
/**
* @param {Payload} payload
* @param {(head_payload: Payload['head']) => void} fn
* @param {import('#server').Payload} payload
* @param {(head_payload: import('#server').Payload['head']) => void} fn
* @returns {void}
*/
export function head(payload, fn) {
@ -239,7 +156,7 @@ export function attr(name, value, boolean) {
}
/**
* @param {Payload} payload
* @param {import('#server').Payload} payload
* @param {boolean} is_html
* @param {Record<string, string>} props
* @param {() => void} component
@ -497,8 +414,8 @@ export async function value_or_fallback_async(value, fallback) {
}
/**
* @param {Payload} payload
* @param {void | ((payload: Payload, props: Record<string, unknown>) => void)} slot_fn
* @param {import('#server').Payload} payload
* @param {void | ((payload: import('#server').Payload, props: Record<string, unknown>) => void)} slot_fn
* @param {Record<string, unknown>} slot_props
* @param {null | (() => void)} fallback_fn
* @returns {void}
@ -621,6 +538,8 @@ export function once(get_value) {
export { push, pop } from './context.js';
export { push_element, pop_element } from './dev.js';
export {
add_snippet_symbol,
validate_component,

@ -5,4 +5,18 @@ export interface Component {
c: null | Map<unknown, unknown>;
/** ondestroy */
d: null | Array<() => void>;
/**
* dev mode only: the component function
*/
function?: any;
}
export interface Payload {
out: string;
anchor: number;
head: {
title: string;
out: string;
anchor: number;
};
}

@ -32,9 +32,8 @@ export default test({
if (variant === 'hydrate') {
assert.equal(
log[0],
`Svelte SSR validation error:\n\n<h1> is invalid inside <p>\n\n` +
'Ensure your components render valid HTML as the browser will try to repair invalid HTML, ' +
'which may result in content being shifted around and will likely result in a hydration mismatch.'
'`<h1>` (.../samples/invalid-html-ssr/Component.svelte:1:0) cannot contain `<p>` (.../samples/invalid-html-ssr/main.svelte:5:0)\n\n' +
'This can cause content to shift around as the browser repairs the HTML, and will likely result in a `hydration_mismatch` warning.'
);
}
}

@ -2,7 +2,7 @@ import * as $ from "svelte/internal/server";
import TextInput from './Child.svelte';
export default function Bind_component_snippet($$payload, $$props) {
$.push(true);
$.push();
let value = '';
const _snippet = snippet;

@ -1,7 +1,7 @@
import * as $ from "svelte/internal/server";
export default function Bind_this($$payload, $$props) {
$.push(false);
$.push();
$$payload.out += `<!--[-->`;
Foo($$payload, {});
$$payload.out += `<!--]-->`;

@ -1,7 +1,7 @@
import * as $ from "svelte/internal/server";
export default function Class_state_field_constructor_assignment($$payload, $$props) {
$.push(true);
$.push();
class Foo {
a;

@ -1,7 +1,7 @@
import * as $ from "svelte/internal/server";
export default function Main($$payload, $$props) {
$.push(true);
$.push();
// needs to be a snapshot test because jsdom does auto-correct the attribute casing
let x = 'test';

@ -1,7 +1,7 @@
import * as $ from "svelte/internal/server";
export default function Each_string_template($$payload, $$props) {
$.push(false);
$.push();
const each_array = $.ensure_array_like(['foo', 'bar', 'baz']);

@ -1,7 +1,7 @@
import * as $ from "svelte/internal/server";
export default function Function_prop_no_getter($$payload, $$props) {
$.push(true);
$.push();
let count = 0;

@ -1,7 +1,7 @@
import * as $ from "svelte/internal/server";
export default function Hello_world($$payload, $$props) {
$.push(false);
$.push();
$$payload.out += `<h1>hello world</h1>`;
$.pop();
}

@ -1,7 +1,7 @@
import * as $ from "svelte/internal/server";
export default function Hmr($$payload, $$props) {
$.push(false);
$.push();
$$payload.out += `<h1>hello world</h1>`;
$.pop();
}

@ -1,7 +1,7 @@
import * as $ from "svelte/internal/server";
export default function State_proxy_literal($$payload, $$props) {
$.push(true);
$.push();
let str = '';
let tpl = ``;

@ -1,7 +1,7 @@
import * as $ from "svelte/internal/server";
export default function Svelte_element($$payload, $$props) {
$.push(true);
$.push();
let { tag = 'hr' } = $$props;

Loading…
Cancel
Save