Skip to content

fix: bind null option and input values consistently #8328

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

Merged
merged 1 commit into from
Apr 14, 2023

Conversation

theodorejb
Copy link
Contributor

Null and undefined value bindings should always be set to an empty string. This allows native browser validation of required fields to work as expected with placeholder options.

Placeholder options bound to null are necessary in components where the field is conditionally required, and the bound value will be posted to an API endpoint which requires it to be a nullable number or object rather than a string.

Resolves #8312

@vercel
Copy link

vercel bot commented Feb 26, 2023

@theodorejb is attempting to deploy a commit to the Svelte Team on Vercel.

A member of the Team first needs to authorize it.

@dummdidumm
Copy link
Member

Before we merge this we need to see if this could be seen as a breaking change or not, and if so, if we want the changed behavior (probably) in 4.x.

@theodorejb
Copy link
Contributor Author

@dummdidumm I don't think this should be seen as a breaking change. It only has an effect on option elements that are directly bound to null or undefined, i.e.:

<option value={null}></option>
<option value={undefined}></option>

It does not change the value bound to the parent select element. In the following example, the component's foo variable will be bound to null both before and after this patch when the empty option is selected:

<select bind:value={foo} required>
    <option value={null}></option>
    <option>A</option>
</select>

The only difference is that now the value attribute in the DOM for the empty option will be a blank string, which is consistent with how value bindings work on other input elements. This allows native browser validation of the required dropdown to work as expected.

The one case I can think of that could potentially break is if someone has a select dropdown that isn't bound to a component value, e.g.:

<select name="item">
    <option value={null}></option>
    <option value="a">A</option>
</select>

Currently if the blank option is selected, the string "null" would be submitted for the item form value, but after this patch it would be an empty string. I would still see this as a bugfix for unexpected behavior, though. If someone actually wants the string "null" to be submitted for an option, they would enter it as a string:

<select name="item">
    <option value="null">Null</option>
    <option value="a">A</option>
</select>

@vercel
Copy link

vercel bot commented Feb 27, 2023

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Comments Updated
svelte-dev-2 ❌ Failed (Inspect) Feb 27, 2023 at 4:18PM (UTC)

Null and undefined `value` bindings should always be set to an empty string. This allows native browser validation of `required` fields to work as expected with placeholder options.

Placeholder options bound to null are necessary in forms where the field is conditionally required, and the bound value is posted to an API endpoint which requires it to be a nullable number or object rather than a string.

Resolves sveltejs#8312
@theodorejb theodorejb force-pushed the consistent-value-binding branch from 7a00169 to e72d02d Compare March 1, 2023 04:35
@theodorejb
Copy link
Contributor Author

A more likely scenario this change could break is if a select element is incorrectly required. For example:

<select bind:value={foo} required>
    <option value={null}></option>
    <option value="a">A</option>
    <option value="b">B</option>
</select>

If the form was intended to allow being submitted with the blank option selected, it currently works by happenstance because the empty option is given a non-empty value in the DOM. I can see this occurring if a developer blindly added required to all inputs, or copy/pasted markup from another element and forgot to remove the required attribute. In this case the fix would be to remove the incorrect required attribute.

But I would consider it more likely that the above code was intended not to submit when the blank option is selected, and merging this pull request will fix the bug.

@dummdidumm dummdidumm added this to the 4.x milestone Mar 3, 2023
@benmccann benmccann changed the base branch from master to version-4 April 11, 2023 21:06
Copy link
Member

@dummdidumm dummdidumm left a comment

Choose a reason for hiding this comment

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

Thank you!

@dummdidumm dummdidumm merged commit 88728e3 into sveltejs:version-4 Apr 14, 2023
@theodorejb theodorejb deleted the consistent-value-binding branch April 14, 2023 15:58
dummdidumm pushed a commit that referenced this pull request Apr 18, 2023
Null and undefined `value` bindings should always be set to an empty string. This allows native browser validation of `required` fields to work as expected with placeholder options.

Placeholder options bound to null are necessary in forms where the field is conditionally required, and the bound value is posted to an API endpoint which requires it to be a nullable number or object rather than a string.

fixes #8312
@gtm-nayan gtm-nayan mentioned this pull request Jun 17, 2023
5 tasks
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 this pull request may close these issues.

Select option with null value prevents native required field validation
2 participants