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

Weird input validation breakage when using $$props #4418

Closed
arggh opened this issue Feb 15, 2020 · 7 comments · Fixed by #5004
Closed

Weird input validation breakage when using $$props #4418

arggh opened this issue Feb 15, 2020 · 7 comments · Fixed by #5004

Comments

@arggh
Copy link
Contributor

arggh commented Feb 15, 2020

Describe the bug

When passing down unknown props using $$props, like so (Svelma does this, maybe other component libs too?)

<script>
// SvelteInput.svelte

export let value;

$: props = omit($$props, 'value');
</script>

<input type="text" bind:value {...props}/>

...the standard HTML input validation fails when using minlength, like so

<SvelteInput required minlength="5" bind:value={val}/>

The produced HTML seems fine, the input element has both required and minlength="5" set, but the validation doesn't kick in if the input has fewer than 5 characters. If the input is empty, validation works.

If I remove the reactive $: props... assignment and instead pass required and minlength manually, the validation starts working again.

To Reproduce

https://svelte.dev/repl/8da0a0fdbc3245d19e57aeab025e763f?version=3.18.2

Type something into the input. The other one correctly displays the :invalid style, the other one doesn't.

Expected behavior

I'd expect the html input validation to work even when using $: props = ....

Information about your Svelte project:

  • All latest Safari, Firefox, Chrome

  • Mac OS Mojave

  • Svelte version 3.18.2, but checked that the issue exists at least down to 3.7.0

Severity
Breaks validation, but in a very sneaky way, so I'd give it a 7/10 on severity index!

@antony
Copy link
Member

antony commented Feb 16, 2020

Do remember that $$props is an internal API and as such, it's behaviour for end users is unpredictable, and subject to change.

@arggh
Copy link
Contributor Author

arggh commented Feb 16, 2020

It is not, though: https://svelte.dev/docs#Attributes_and_props (...scroll to the end of section "Attributes and props")

Also this: #2528

@PatrickG
Copy link

I don't think this has anything to do with the spread operator.
The two inputs behave the same for me (only tested in chromium 81).

It seems this has something to do with setting the value of an input element with javascript (in this case, by the bind directive).
See https://stackoverflow.com/questions/53226031/html-input-validity-not-checked-when-value-is-changed-by-javascript

@arggh
Copy link
Contributor Author

arggh commented Feb 28, 2020

The two inputs behave the same for me (only tested in chromium 81).

Screenshot 2020-02-28 at 9 36 18

@PatrickG Do you mean, that if you type asdasd into either one of the inputs, it doesn't look like the screenshot, where one field displays invalid state, other displays valid?

Both of those inputs definitely set the input's value via JavaScript using Svelte's two-way binding, so that is not the cause. The other one works just fine, the other one doesn't, and the only difference is the use of {...props}.

Due to this:

If an element has a minimum allowed value length, its dirty value flag is true, its value was last changed by a user edit (as opposed to a change made by a script), its value is not the empty string, and the code-unit length of the element’s value is less than the element’s minimum allowed value length, then the element is suffering from being too short.

...I believe the only way around this issue is to introduce an $$attrs variable for Svelte components, since with the current "hacky $$props" -approach, props gets reassigned on every keystroke, thus (probably) breaking the input's minlength-validation.

More about that here: #3015 (comment)

@PatrickG
Copy link

PatrickG commented Feb 28, 2020

@PatrickG Do you mean, that if you type asdasd into either one of the inputs, it doesn't look like the screenshot, where one field displays invalid state, other displays valid?

Only the input I'm currently typing shows the invalid state. But it doesn't matter which one. If I type in the input with {...props} that shows the invalid state, if I type in the one with {maxlength} that shows the invalid state.

If you use two inputs with {maxlength}, it is the same problem: https://svelte.dev/repl/d3ed4891c78042df86f447d67a3f0160?version=3.18.2

With the pattern attribute instead of the maxlength, it works: https://svelte.dev/repl/6e99e8920b1c4d229e70d1918fe6b9d5?version=3.18.2

props gets reassigned on every keystroke, thus (probably) breaking the input's minlength-validation.

It is not the reassinged props that makes the problem. its the assignment of input.value. If you update value property of the input in the dev tools, the invalid state is not applied either. See the stackoverflow link in my previous comment.

@arggh
Copy link
Contributor Author

arggh commented Feb 29, 2020

@PatrickG it seems I jumped into hasty conclusions when producing the repro from our actual code.

Maybe this REPL is a better starting point: https://svelte.dev/repl/361a85968bd5458a920c62bf610e8a9a?version=3.18.2

It seems using 2-way-binding for the value keeps things working, but using on:input to set the value manually, combined with ...props seems to break things.

Using on:input to set the value without ...props seems to "work", meaning the inpu displays validation error only after first input from user.

@Conduitry
Copy link
Member

This should be fixed now in 3.24.0 - https://svelte.dev/repl/361a85968bd5458a920c62bf610e8a9a?version=3.24.0

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants