Skip to content

fix: throw runtime error when template returns different html #15524

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

Draft
wants to merge 2 commits into
base: main
Choose a base branch
from
Draft
Show file tree
Hide file tree
Changes from 1 commit
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/fluffy-eggs-do.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
'svelte': patch
---

fix: throw runtime error when template returns different html
6 changes: 6 additions & 0 deletions documentation/docs/98-reference/.generated/client-errors.md
Original file line number Diff line number Diff line change
Expand Up @@ -80,6 +80,12 @@ Maximum update depth exceeded. This can happen when a reactive block or effect r
Failed to hydrate the application
```

### invalid_html_structure

```
This html structure `%html_input%` would be corrected like this `%html_output%` by the browser making this component impossible to hydrate properly
```

### invalid_snippet

```
Expand Down
4 changes: 4 additions & 0 deletions packages/svelte/messages/client-errors/errors.md
Original file line number Diff line number Diff line change
Expand Up @@ -52,6 +52,10 @@ See the [migration guide](/docs/svelte/v5-migration-guide#Components-are-no-long

> Failed to hydrate the application

## invalid_html_structure

> This html structure `%html_input%` would be corrected like this `%html_output%` by the browser making this component impossible to hydrate properly

## invalid_snippet

> Could not `{@render}` snippet due to the expression being `null` or `undefined`. Consider using optional chaining `{@render snippet?.()}`
Expand Down
2 changes: 1 addition & 1 deletion packages/svelte/src/internal/client/dom/blocks/html.js
Original file line number Diff line number Diff line change
Expand Up @@ -99,7 +99,7 @@ export function html(node, get_value, svg, mathml, skip_warning) {
// Don't use create_fragment_with_script_from_html here because that would mean script tags are executed.
// @html is basically `.innerHTML = ...` and that doesn't execute scripts either due to security reasons.
/** @type {DocumentFragment | Element} */
var node = create_fragment_from_html(html);
var node = create_fragment_from_html(html, false);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

can you elaborate on why we're not doing this for {@html ...}? It may survive hydration but it's still delivering an unexpected outcome. Feels like we should just do it in all cases

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It mainly goes down to self-closing-but-not-really tags.

{@html "<div />"}

This will yield an error (I suppose if we switch to a warning it could be fine tho)


if (svg || mathml) {
node = /** @type {Element} */ (get_first_child(node));
Expand Down
27 changes: 25 additions & 2 deletions packages/svelte/src/internal/client/dom/reconciler.js
Original file line number Diff line number Diff line change
@@ -1,6 +1,29 @@
/** @param {string} html */
export function create_fragment_from_html(html) {
import { DEV } from 'esm-env';
import * as e from '../errors.js';

/**
* @param {string} html
* @param {boolean} [check_structure]
*/
export function create_fragment_from_html(html, check_structure = true) {
var elem = document.createElement('template');
elem.innerHTML = html;
if (DEV && check_structure) {
let replace_comments = html.replaceAll('<!>', '<!---->');
let remove_attributes_and_text_input = replace_comments
// we remove every attribute since the template automatically adds ="" after boolean attributes
.replace(/<([a-z0-9]+)(\s+[^>]+?)?>/g, '<$1>')
// we remove the text within the elements because the template change & to &amp; (and similar)
.replace(/>([^<>]*)/g, '>');
let remove_attributes_and_text_output = elem.innerHTML
// we remove every attribute since the template automatically adds ="" after boolean attributes
.replace(/<([a-z0-9]+)(\s+[^>]+?)?>/g, '<$1>')
// we remove the text within the elements because the template change & to &amp; (and similar)
.replace(/>([^<>]*)/g, '>');
if (remove_attributes_and_text_input !== remove_attributes_and_text_output) {
e.invalid_html_structure(remove_attributes_and_text_input, remove_attributes_and_text_output);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The HTML snippets could become arbitrarily large (especially if we enabled this for {@html ...} as well), we may need to adding truncation here, or remove common prefixes/suffixes etc

The alternative approach of checking the structure manually could help here since it means we can point to the specific element that's being mishandled. (Perhaps add_locations becomes a validation function as well — it already knows the intended structure.)

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah I though about that but also thought a diffing algorithm would be too much and not showing complete information would be not really helpful.

I can try to put up a PR for the alternative method tomorrow to check if it could yield better results

}
}

return elem.content;
}
17 changes: 17 additions & 0 deletions packages/svelte/src/internal/client/errors.js
Original file line number Diff line number Diff line change
Expand Up @@ -198,6 +198,23 @@ export function hydration_failed() {
}
}

/**
* This html structure `%html_input%` would be corrected like this `%html_output%` by the browser making this component impossible to hydrate properly
* @param {string} html_input
* @param {string} html_output
* @returns {never}
*/
export function invalid_html_structure(html_input, html_output) {
if (DEV) {
const error = new Error(`invalid_html_structure\nThis html structure \`${html_input}\` would be corrected like this \`${html_output}\` by the browser making this component impossible to hydrate properly\nhttps://svelte.dev/e/invalid_html_structure`);

error.name = 'Svelte error';
throw error;
} else {
throw new Error(`https://svelte.dev/e/invalid_html_structure`);
}
}

/**
* Could not `{@render}` snippet due to the expression being `null` or `undefined`. Consider using optional chaining `{@render snippet?.()}`
* @returns {never}
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
import { test } from '../../test';

export default test({
mode: ['client', 'hydrate'],
recover: true,
runtime_error: 'invalid_html_structure'
});
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
<svelte:options runes />
<p></p>
<tr></tr>