svelte

svelte copied to clipboard

Describe the bug

When binding to inputs in nested forms, the binding fails and a runtime error is thrown:

<form>
	<form>
			<input bind:value />
	</form>
</form>

Although nested forms are not valid according to the HTML spec, the error should be either friendlier or not throw at all since it worked just fine in Svelte 4.

Reproduction

https://svelte-5-preview.vercel.app/#H4sIAAAAAAAACkWM2wrDIBBEf0WWQloQfLe20O-ofbDJCoI3dA2EkH8vNoEwL2eGw6xgnccK8r1CNAFBwitn4EBL7qXO6AmBQ02tjH1RdSwu01NHTR6JzcY3ZA92qWQIr8Nwu-uoxGlFZVMJf_-kHuVibsS-Lk5yfxG7JQ7tAOAQ0uSswwkklYbbZ_sBkmW8mbgAAAA=

Logs

Cannot read properties of null (reading 'addEventListener')

in App.svelte

System Info

running Svelte compiler version 5.0.0-next.151

Severity

blocking an upgrade

This is listed as one of the breaking changes in Svelte 5. What's your use case for creating invalid HTML?

The error here isn't actually a compiler error - it's a runtime error - but, in cases where we can see both <form>s in one component, maybe we could add an actual compiler error for this, like we do for other types of invalid element nesting.

This is listed as one of the breaking changes in Svelte 5. What's your use case for creating invalid HTML?

No use case, just porting Svelte 4 code, this made us realize we were actually shipping invalid HTML, so it's kind of good this errors then. It was a bit hard to spot since one form lived deeply inside a component. If this is as expected you may close this, could you link where this is listed as a breaking change?

I just think it's a bit hard to know what you're doing wrong since it only throws when binding inside a nested form but not without. Could be confusing idk.

Is it: https://svelte-5-preview.vercel.app/docs/breaking-changes#other-breaking-changes-bindings-now-react-to-form-resets?

Huh I tell a lie, this isn't listed yet in the breaking changes. Therefore giving it the documentation label.

The error here isn't actually a compiler error - it's a runtime error - but, in cases where we can see both <form>s in one component, maybe we could add an actual compiler error for this, like we do for other types of invalid element nesting.

I agree with this...if it's statically analizable throwing a compiler error with a nice message is preferrable.

I can work on this but i would love some kind of approval if that's something we actually want to do.

Yes, we already have a validation for this in place, seems like it's missing the form/form case. Though as others have said this only works when things are within the same component. In the mid term it would be cool to investigate if there's something we can do at runtime in dev mode

Yes, we already have a validation for this in place, seems like it's missing the form/form case. Though as others have said this only works when things are within the same component. In the mid term it would be cool to investigate if there's something we can do at runtime in dev mode

will check on that :)

Yes, we already have a validation for this in place, seems like it's missing the form/form case. Though as others have said this only works when things are within the same component. In the mid term it would be cool to investigate if there's something we can do at runtime in dev mode

So i've started investingating this a bit:

1. the general problem is that by creating the html using <template /> any invalid HTML will be attempted to be corrected, in this case is eliminating the outer form causing the code of the component to be off by one

import * as $ from "svelte/internal/client";

var root = $.template(`<form><form><input></form></form>`); // this ends like this <form><input></form>

export default function App($$anchor) {
	let value = $.source('');
	var form = root();
	var form_1 = $.child(form); // the child of the form is the actual input
	var input = $.child(form_1); // the input doesn't have a child so this is null

	$.remove_input_attr_defaults(input);
        // this fails because is setting a listener on null is not possible.
	$.bind_value(input, () => $.get(value), ($$value) => $.set(value, $$value)); 
	$.append($$anchor, form);
}

2. the whole bug will be obviously fixed by the validator but cases like this might happen anyway for other wrong HTML which we are not yet aware of. The situation can get also much worst if you don't add listeners on the input but on the inner form because in that case you are actually adding everything to the input. The good news is that technically this is "fixable" statically because is you stick the form in a component it get's it's own template root and just ships the double form to the browser (not ideal but at least doesn't fail cryptically).
3. one possible idea for this could be to always validate the output of a function to retrieve an element checking against the tagName

import * as $ from "svelte/internal/client";

var on_input = () => {};
var root = $.template(`<form><form><input></form></form>`);

export default function App($$anchor) {
	var form = root();
        $.validate_element(form, "form");
	var form_1 = $.child(form);
        $.validate_element(form_1, "form");
	var input = $.child(form_1);
        $.validate_element(input, "input");

	input.__input = [on_input];
	$.append($$anchor, form);
}

$.delegate(["input"]);

this in theory is guaranteed to error for at least 1 element (either is null or the wrong expected element) but to be fair i don't know how well it would scale and the performance hit.

In the meantime i'm gonna open the PR to fix the validator we can iterate there or in another PR.

Reopening this since we still haven't fixed <form><Form></Form></form>, which makes hydration errors very likely. Opened #11969 to investigate further