fix: throw runtime error when template returns different html

[paoloricciuti]

Closes #15502

I've added a runtime error that checks if the html generated by the template is the same structure as the passed in string. This should prevent more obscure errors because if the structure is different our marching of the dom will still fail at runtime.

To properly check i had to transform both the innerHTML and the incoming structure to remove inner texts (that get's converted in html entities) and attributes (because boolean attributes get ="" appended automatically.

Also i don't do this check in {@html} because you could technically pass invalid html there but that doesn't affect the marching algorithm.

Another idea I had (might still open a PR as an alternative) is to add a validate_element call after every next or child...basically like this

import 'svelte/internal/disclose-version';
import 'svelte/internal/flags/legacy';
import * as $ from 'svelte/internal/client';

var root = $.template(`<p></p> <tr><td></td></tr>`, 1);

export default function App($$anchor) {
	let name = 'world';
	var fragment = root();
	var tr = $.sibling($.first_child(fragment), 2);
	$.validate_element(tr, 'tr');
	var td = $.child(tr);
	$.validate_element(td, 'td');
	td.textContent = name;
	$.reset(tr);
	$.append($$anchor, fragment);
}

but i think this would be more unreliable.

Before submitting the PR, please make sure you do the following

• It's really useful if your PR references an issue where it is discussed ahead of time. In many cases, features are absent for a reason. For large changes, please create an RFC: https://github.com/sveltejs/rfcs
• Prefix your PR title with feat:, fix:, chore:, or docs:.
• This message body should clearly illustrate what problems it solves.
• Ideally, include a test that fails without this PR but passes with it.
• If this PR changes code within packages/svelte/src, add a changeset (npx changeset).

Tests and linting

• Run the tests with pnpm test and lint the project with pnpm lint

[changeset-bot]

🦋 Changeset detected

Latest commit: 62a6a15

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 1 package

Name	Type
svelte	Patch

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

[svelte-docs-bot]

Preview: https://svelte-dev-git-preview-svelte-15524-svelte.vercel.app/

[github-actions]

Playground

pnpm add https://pkg.pr.new/svelte@15524

[Rich-Harris]

Nice idea! This would be a breaking change though so it would need to be a warning rather than an error. I suspect the proposed alternative approach would work better — details inline. (It wouldn't work for {@html ...} though... so maybe we need both?)

Also, we should do this checking eagerly, otherwise no warning will be issued for malformed HTML in a block that happens not to get rendered

[Rich-Harris]

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

[paoloricciuti]

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)

[Rich-Harris]

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.)

[paoloricciuti]

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

[paoloricciuti]

This would be a breaking change though so it would need to be a warning rather than an error

Can you think of some cases where having the wrong output from the template would not lead to a runtime error anyway? I thought of making it a warning but I also thought that it would be a runtime error anyway.

[Rich-Harris]

Many cases where you're not hydrating (the test case in this PR, for example). Hopefully there aren't too many of those cases in the wild but there's probably some where it's 'failing' in a way that's harmless

[paoloricciuti]

Many cases where you're not hydrating (the test case in this PR, for example). Hopefully there aren't too many of those cases in the wild but there's probably some where it's 'failing' in a way that's harmless

Mmm that's annoying...well gonna move it to warning tomorrow 😁

[7nik]

Do I miss something or the issue example still throws the same error? And I don't see how it will help when the problem caused by invalid composition of valid components, e.g. <Button><Button>btn</Button></Button>

[paoloricciuti]

Do I miss something or the issue example still throws the same error? And I don't see how it will help when the problem caused by invalid composition of valid components, e.g. <Button><Button>btn</Button></Button>

Yes because now it's just a warning that you can see in the console.

Also no, it will not solve that issue, only local marching problems

[Rich-Harris]

Come to think of it, couldn't we find these issues at compile time?

[Rich-Harris]

(I mean not for {@html ...} but for anything that gets turned into a $.template call)

[paoloricciuti]

Come to think of it, couldn't we find these issues at compile time?

The main issue i see with this is that I'm pretty sure this is browser dependent...a browser might correct in a different way. Also keeping up with the actual transformation the browser does would be very error prone.

[Rich-Harris]

I would be very surprised if browsers differed — this stuff is all spec'd, it's not open to interpretation. I'm not sure exactly what rules we'd need to follow but I bet we could figure it out