From e72d02d8490379642c639393ccc58877472cbc3a Mon Sep 17 00:00:00 2001
From: Theodore Brown <theodorejb@outlook.com>
Date: Sat, 25 Feb 2023 21:27:00 -0600
Subject: [PATCH] fix: bind null option and input values consistently

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 #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 ca35ea84db49..e219f86aa1c0 100644
--- a/src/compiler/compile/render_dom/wrappers/Element/Attribute.ts
+++ b/src/compiler/compile/render_dom/wrappers/Element/Attribute.ts
@@ -171,7 +171,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 cd1ab1d81242..1d0896d6e822 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 000000000000..b453e6869a95
--- /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 000000000000..65cab994957c
--- /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>