From 88728e35918738ea28e50f0bdf588b3777aac502 Mon Sep 17 00:00:00 2001
From: Theodore Brown <theodorejb@outlook.com>
Date: Fri, 14 Apr 2023 10:13:30 -0500
Subject: [PATCH] fix: bind null option and input values consistently (#8328)

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
---
 .../render_dom/wrappers/Element/Attribute.ts  |  2 +-
 .../samples/select-dynamic-value/expected.js  |  7 +++--
 .../_config.js                                | 28 +++++++++++++++++++
 .../main.svelte                               | 11 ++++++++
 4 files changed, 44 insertions(+), 4 deletions(-)
 create mode 100644 test/runtime/samples/binding-select-null-placeholder/_config.js
 create mode 100644 test/runtime/samples/binding-select-null-placeholder/main.svelte

diff --git a/src/compiler/compile/render_dom/wrappers/Element/Attribute.ts b/src/compiler/compile/render_dom/wrappers/Element/Attribute.ts
index 64178030f6..61311fd83c 100644
--- a/src/compiler/compile/render_dom/wrappers/Element/Attribute.ts
+++ b/src/compiler/compile/render_dom/wrappers/Element/Attribute.ts
@@ -172,7 +172,7 @@ export default class AttributeWrapper extends BaseAttributeWrapper {
 		}
 
 		if (is_indirectly_bound_value) {
-			const update_value = b`${element.var}.value = ${element.var}.__value;`;
+			const update_value = b`@set_input_value(${element.var}, ${element.var}.__value);`;
 			block.chunks.hydrate.push(update_value);
 
 			updater = b`
diff --git a/test/js/samples/select-dynamic-value/expected.js b/test/js/samples/select-dynamic-value/expected.js
index cd1ab1d812..1d0896d6e8 100644
--- a/test/js/samples/select-dynamic-value/expected.js
+++ b/test/js/samples/select-dynamic-value/expected.js
@@ -8,7 +8,8 @@ import {
 	insert,
 	noop,
 	safe_not_equal,
-	select_option
+	select_option,
+	set_input_value
 } from "svelte/internal";
 
 function create_fragment(ctx) {
@@ -24,9 +25,9 @@ function create_fragment(ctx) {
 			option1 = element("option");
 			option1.textContent = "2";
 			option0.__value = "1";
-			option0.value = option0.__value;
+			set_input_value(option0, option0.__value);
 			option1.__value = "2";
-			option1.value = option1.__value;
+			set_input_value(option1, option1.__value);
 		},
 		m(target, anchor) {
 			insert(target, select, anchor);
diff --git a/test/runtime/samples/binding-select-null-placeholder/_config.js b/test/runtime/samples/binding-select-null-placeholder/_config.js
new file mode 100644
index 0000000000..b453e6869a
--- /dev/null
+++ b/test/runtime/samples/binding-select-null-placeholder/_config.js
@@ -0,0 +1,28 @@
+const items = [ { id: 'a' }, { id: 'b' } ];
+
+export default {
+	props: {
+		foo: null,
+		items
+	},
+
+	test({ assert, component, target }) {
+		const select = target.querySelector( 'select' );
+		const options = target.querySelectorAll( 'option' );
+
+		assert.equal( options[0].selected, true );
+		assert.equal( options[0].disabled, true );
+		assert.equal( options[1].selected, false );
+		assert.equal( options[1].disabled, false );
+
+		// placeholder option value must be blank string for native required field validation
+		assert.equal( options[0].value, '' );
+		assert.equal( select.checkValidity(), false );
+
+		component.foo = items[0];
+
+		assert.equal( options[0].selected, false );
+		assert.equal( options[1].selected, true );
+		assert.equal( select.checkValidity(), true );
+	}
+};
diff --git a/test/runtime/samples/binding-select-null-placeholder/main.svelte b/test/runtime/samples/binding-select-null-placeholder/main.svelte
new file mode 100644
index 0000000000..65cab99495
--- /dev/null
+++ b/test/runtime/samples/binding-select-null-placeholder/main.svelte
@@ -0,0 +1,11 @@
+<script>
+	export let foo;
+	export let items;
+</script>
+
+<select bind:value={foo} required>
+	<option value={null} disabled>Select an option</option>
+	{#each items as item}
+		<option value={item}>{item.id}</option>
+	{/each}
+</select>