Skip to content
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

chore: improve SSR invalid element error message #11585

Merged
merged 13 commits into from
May 14, 2024
Merged
5 changes: 5 additions & 0 deletions .changeset/bright-falcons-float.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
"svelte": patch
---

chore: improve SSR invalid element error message
Original file line number Diff line number Diff line change
Expand Up @@ -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}-->`);
Expand Down Expand Up @@ -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)
)
)
)
);
}

Expand Down Expand Up @@ -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'))
Expand Down Expand Up @@ -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',
Expand Down
9 changes: 8 additions & 1 deletion packages/svelte/src/internal/server/context.js
Original file line number Diff line number Diff line change
Expand Up @@ -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() {
Expand Down
93 changes: 93 additions & 0 deletions packages/svelte/src/internal/server/dev.js
Original file line number Diff line number Diff line change
@@ -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;
}
109 changes: 14 additions & 95 deletions packages/svelte/src/internal/server/index.js
Original file line number Diff line number Diff line change
@@ -1,45 +1,19 @@
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;
* html: string;
* }} 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 =
Expand All @@ -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 {
Expand All @@ -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) {
Expand All @@ -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
Expand Down Expand Up @@ -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) {
Expand All @@ -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
Expand Down Expand Up @@ -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}
Expand Down Expand Up @@ -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,
Expand Down
14 changes: 14 additions & 0 deletions packages/svelte/src/internal/server/types.d.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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;
};
}
Original file line number Diff line number Diff line change
Expand Up @@ -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.'
);
}
}
Expand Down
Loading
Loading