Skip to content

Commit 1087e6f

Browse files
trueadmRich-Harris
andauthored
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 <[email protected]>
1 parent e97bc79 commit 1087e6f

File tree

17 files changed

+179
-111
lines changed

17 files changed

+179
-111
lines changed

.changeset/bright-falcons-float.md

+5
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
---
2+
"svelte": patch
3+
---
4+
5+
chore: improve SSR invalid element error message

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

+33-2
Original file line numberDiff line numberDiff line change
@@ -27,6 +27,7 @@ import { DOMBooleanAttributes, HYDRATION_END, HYDRATION_START } from '../../../.
2727
import { escape_html } from '../../../../escaping.js';
2828
import { sanitize_template_string } from '../../../utils/sanitize_template_string.js';
2929
import { BLOCK_CLOSE, BLOCK_CLOSE_ELSE } from '../../../../internal/server/hydration.js';
30+
import { locator } from '../../../state.js';
3031

3132
export const block_open = t_string(`<!--${HYDRATION_START}-->`);
3233
export const block_close = t_string(`<!--${HYDRATION_END}-->`);
@@ -1364,8 +1365,19 @@ const template_visitors = {
13641365
}
13651366

13661367
if (context.state.options.dev) {
1368+
const location = /** @type {import('locate-character').Location} */ (locator(node.start));
13671369
context.state.template.push(
1368-
t_statement(b.stmt(b.call('$.push_element', b.literal(node.name), b.id('$$payload'))))
1370+
t_statement(
1371+
b.stmt(
1372+
b.call(
1373+
'$.push_element',
1374+
b.id('$$payload'),
1375+
b.literal(node.name),
1376+
b.literal(location.line),
1377+
b.literal(location.column)
1378+
)
1379+
)
1380+
)
13691381
);
13701382
}
13711383

@@ -2279,9 +2291,12 @@ export function server_component(analysis, options) {
22792291
// undefined to a binding that has a default value.
22802292
template.body.push(b.stmt(b.call('$.bind_props', b.id('$$props'), b.object(props))));
22812293
}
2294+
/** @type {import('estree').Expression[]} */
2295+
const push_args = [];
2296+
if (options.dev) push_args.push(b.id(analysis.name));
22822297

22832298
const component_block = b.block([
2284-
b.stmt(b.call('$.push', b.literal(analysis.runes))),
2299+
b.stmt(b.call('$.push', ...push_args)),
22852300
.../** @type {import('estree').Statement[]} */ (instance.body),
22862301
.../** @type {import('estree').Statement[]} */ (template.body),
22872302
b.stmt(b.call('$.pop'))
@@ -2376,6 +2391,22 @@ export function server_component(analysis, options) {
23762391
body.push(b.export_default(component_function));
23772392
}
23782393

2394+
if (options.dev && options.filename) {
2395+
let filename = options.filename;
2396+
if (/(\/|\w:)/.test(options.filename)) {
2397+
// filename is absolute — truncate it
2398+
const parts = filename.split(/[/\\]/);
2399+
filename = parts.length > 3 ? ['...', ...parts.slice(-3)].join('/') : filename;
2400+
}
2401+
2402+
// add `App.filename = 'App.svelte'` so that we can print useful messages later
2403+
body.unshift(
2404+
b.stmt(
2405+
b.assignment('=', b.member(b.id(analysis.name), b.id('filename')), b.literal(filename))
2406+
)
2407+
);
2408+
}
2409+
23792410
return {
23802411
type: 'Program',
23812412
sourceType: 'module',

packages/svelte/src/internal/server/context.js

+8-1
Original file line numberDiff line numberDiff line change
@@ -53,8 +53,15 @@ function get_or_init_context_map(name) {
5353
return (current_component.c ??= new Map(get_parent_context(current_component) || undefined));
5454
}
5555

56-
export function push() {
56+
/**
57+
* @param {Function} [fn]
58+
*/
59+
export function push(fn) {
5760
current_component = { p: current_component, c: null, d: null };
61+
if (DEV) {
62+
// component function
63+
current_component.function = fn;
64+
}
5865
}
5966

6067
export function pop() {
+93
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,93 @@
1+
import {
2+
disallowed_paragraph_contents,
3+
interactive_elements,
4+
is_tag_valid_with_parent
5+
} from '../../constants.js';
6+
import { current_component } from './context.js';
7+
8+
/**
9+
* @typedef {{
10+
* tag: string;
11+
* parent: null | Element;
12+
* filename: null | string;
13+
* line: number;
14+
* column: number;
15+
* }} Element
16+
*/
17+
18+
/**
19+
* @type {Element | null}
20+
*/
21+
let parent = null;
22+
23+
/** @type {Set<string>} */
24+
let seen;
25+
26+
/**
27+
* @param {Element} element
28+
*/
29+
function stringify(element) {
30+
if (element.filename === null) return `\`<${element.tag}>\``;
31+
return `\`<${element.tag}>\` (${element.filename}:${element.line}:${element.column})`;
32+
}
33+
34+
/**
35+
* @param {import('#server').Payload} payload
36+
* @param {Element} parent
37+
* @param {Element} child
38+
*/
39+
function print_error(payload, parent, child) {
40+
var message =
41+
`${stringify(child)} cannot contain ${stringify(parent)}\n\n` +
42+
'This can cause content to shift around as the browser repairs the HTML, and will likely result in a `hydration_mismatch` warning.';
43+
44+
if ((seen ??= new Set()).has(message)) return;
45+
seen.add(message);
46+
47+
// eslint-disable-next-line no-console
48+
console.error(message);
49+
payload.head.out += `<script>console.error(${JSON.stringify(message)})</script>`;
50+
}
51+
52+
/**
53+
* @param {import('#server').Payload} payload
54+
* @param {string} tag
55+
* @param {number} line
56+
* @param {number} column
57+
*/
58+
export function push_element(payload, tag, line, column) {
59+
var filename = /** @type {import('#server').Component} */ (current_component).function.filename;
60+
var child = { tag, parent, filename, line, column };
61+
62+
if (parent !== null && !is_tag_valid_with_parent(tag, parent.tag)) {
63+
print_error(payload, parent, child);
64+
}
65+
66+
if (interactive_elements.has(tag)) {
67+
let element = parent;
68+
while (element !== null) {
69+
if (interactive_elements.has(element.tag)) {
70+
print_error(payload, element, child);
71+
break;
72+
}
73+
element = element.parent;
74+
}
75+
}
76+
77+
if (disallowed_paragraph_contents.includes(tag)) {
78+
let element = parent;
79+
while (element !== null) {
80+
if (element.tag === 'p') {
81+
print_error(payload, element, child);
82+
break;
83+
}
84+
element = element.parent;
85+
}
86+
}
87+
88+
parent = child;
89+
}
90+
91+
export function pop_element() {
92+
parent = /** @type {Element} */ (parent).parent;
93+
}

packages/svelte/src/internal/server/index.js

+14-95
Original file line numberDiff line numberDiff line change
@@ -1,45 +1,19 @@
11
import { is_promise, noop } from '../shared/utils.js';
22
import { subscribe_to_store } from '../../store/utils.js';
3-
import {
4-
UNINITIALIZED,
5-
DOMBooleanAttributes,
6-
RawTextElements,
7-
disallowed_paragraph_contents,
8-
interactive_elements,
9-
is_tag_valid_with_parent
10-
} from '../../constants.js';
3+
import { UNINITIALIZED, DOMBooleanAttributes, RawTextElements } from '../../constants.js';
114
import { escape_html } from '../../escaping.js';
125
import { DEV } from 'esm-env';
136
import { current_component, pop, push } from './context.js';
147
import { BLOCK_CLOSE, BLOCK_OPEN } from './hydration.js';
158
import { validate_store } from '../shared/validate.js';
169

17-
/**
18-
* @typedef {{
19-
* tag: string;
20-
* parent: null | Element;
21-
* }} Element
22-
*/
23-
2410
/**
2511
* @typedef {{
2612
* head: string;
2713
* html: string;
2814
* }} RenderOutput
2915
*/
3016

31-
/**
32-
* @typedef {{
33-
* out: string;
34-
* anchor: number;
35-
* head: {
36-
* title: string;
37-
* out: string;
38-
* anchor: number;
39-
* };
40-
* }} Payload
41-
*/
42-
4317
// https://html.spec.whatwg.org/multipage/syntax.html#attributes-2
4418
// https://infra.spec.whatwg.org/#noncharacter
4519
const INVALID_ATTR_NAME_CHAR_REGEX =
@@ -64,19 +38,14 @@ export const VoidElements = new Set([
6438
'wbr'
6539
]);
6640

67-
/**
68-
* @type {Element | null}
69-
*/
70-
let current_element = null;
71-
72-
/** @returns {Payload} */
41+
/** @returns {import('#server').Payload} */
7342
function create_payload() {
7443
return { out: '', head: { title: '', out: '', anchor: 0 }, anchor: 0 };
7544
}
7645

7746
/**
78-
* @param {Payload} to_copy
79-
* @returns {Payload}
47+
* @param {import('#server').Payload} to_copy
48+
* @returns {import('#server').Payload}
8049
*/
8150
export function copy_payload(to_copy) {
8251
return {
@@ -87,8 +56,8 @@ export function copy_payload(to_copy) {
8756

8857
/**
8958
* Assigns second payload to first
90-
* @param {Payload} p1
91-
* @param {Payload} p2
59+
* @param {import('#server').Payload} p1
60+
* @param {import('#server').Payload} p2
9261
* @returns {void}
9362
*/
9463
export function assign_payload(p1, p2) {
@@ -98,59 +67,7 @@ export function assign_payload(p1, p2) {
9867
}
9968

10069
/**
101-
* @param {Payload} payload
102-
* @param {string} message
103-
*/
104-
function error_on_client(payload, message) {
105-
message =
106-
`Svelte SSR validation error:\n\n${message}\n\n` +
107-
'Ensure your components render valid HTML as the browser will try to repair invalid HTML, ' +
108-
'which may result in content being shifted around and will likely result in a hydration mismatch.';
109-
// eslint-disable-next-line no-console
110-
console.error(message);
111-
payload.head.out += `<script>console.error(\`${message}\`)</script>`;
112-
}
113-
114-
/**
115-
* @param {string} tag
116-
* @param {Payload} payload
117-
*/
118-
export function push_element(tag, payload) {
119-
if (current_element !== null && !is_tag_valid_with_parent(tag, current_element.tag)) {
120-
error_on_client(payload, `<${tag}> is invalid inside <${current_element.tag}>`);
121-
}
122-
if (interactive_elements.has(tag)) {
123-
let element = current_element;
124-
while (element !== null) {
125-
if (interactive_elements.has(element.tag)) {
126-
error_on_client(payload, `<${tag}> is invalid inside <${element.tag}>`);
127-
}
128-
element = element.parent;
129-
}
130-
}
131-
if (disallowed_paragraph_contents.includes(tag)) {
132-
let element = current_element;
133-
while (element !== null) {
134-
if (element.tag === 'p') {
135-
error_on_client(payload, `<${tag}> is invalid inside <p>`);
136-
}
137-
element = element.parent;
138-
}
139-
}
140-
current_element = {
141-
tag,
142-
parent: current_element
143-
};
144-
}
145-
146-
export function pop_element() {
147-
if (current_element !== null) {
148-
current_element = current_element.parent;
149-
}
150-
}
151-
152-
/**
153-
* @param {Payload} payload
70+
* @param {import('#server').Payload} payload
15471
* @param {string} tag
15572
* @param {() => void} attributes_fn
15673
* @param {() => void} children_fn
@@ -214,8 +131,8 @@ export function render(component, options) {
214131
}
215132

216133
/**
217-
* @param {Payload} payload
218-
* @param {(head_payload: Payload['head']) => void} fn
134+
* @param {import('#server').Payload} payload
135+
* @param {(head_payload: import('#server').Payload['head']) => void} fn
219136
* @returns {void}
220137
*/
221138
export function head(payload, fn) {
@@ -239,7 +156,7 @@ export function attr(name, value, boolean) {
239156
}
240157

241158
/**
242-
* @param {Payload} payload
159+
* @param {import('#server').Payload} payload
243160
* @param {boolean} is_html
244161
* @param {Record<string, string>} props
245162
* @param {() => void} component
@@ -497,8 +414,8 @@ export async function value_or_fallback_async(value, fallback) {
497414
}
498415

499416
/**
500-
* @param {Payload} payload
501-
* @param {void | ((payload: Payload, props: Record<string, unknown>) => void)} slot_fn
417+
* @param {import('#server').Payload} payload
418+
* @param {void | ((payload: import('#server').Payload, props: Record<string, unknown>) => void)} slot_fn
502419
* @param {Record<string, unknown>} slot_props
503420
* @param {null | (() => void)} fallback_fn
504421
* @returns {void}
@@ -621,6 +538,8 @@ export function once(get_value) {
621538

622539
export { push, pop } from './context.js';
623540

541+
export { push_element, pop_element } from './dev.js';
542+
624543
export {
625544
add_snippet_symbol,
626545
validate_component,

packages/svelte/src/internal/server/types.d.ts

+14
Original file line numberDiff line numberDiff line change
@@ -5,4 +5,18 @@ export interface Component {
55
c: null | Map<unknown, unknown>;
66
/** ondestroy */
77
d: null | Array<() => void>;
8+
/**
9+
* dev mode only: the component function
10+
*/
11+
function?: any;
12+
}
13+
14+
export interface Payload {
15+
out: string;
16+
anchor: number;
17+
head: {
18+
title: string;
19+
out: string;
20+
anchor: number;
21+
};
822
}

packages/svelte/tests/runtime-runes/samples/invalid-html-ssr/_config.js

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

0 commit comments

Comments
 (0)