From c7dcfac883a34655421932258d3d5a3b0ee96362 Mon Sep 17 00:00:00 2001
From: Theodore Brown <theodorejb@outlook.com>
Date: Mon, 13 Mar 2023 12:21:51 -0500
Subject: [PATCH] fix: select option with selected attribute when initial state
 is undefined (#8371)

Resolves a second unintended regression introduced in #6170.

Follow-up to #8331, this time addressing the root issue so the correct select option won't be deselected in the first place when the initial bound value is undefined.

Fixes #8361
---
 .../render_dom/wrappers/Element/Binding.ts    | 10 +++++---
 src/runtime/internal/dom.ts                   | 16 ++++--------
 .../_config.js                                | 25 +++++++++++++++++++
 .../main.svelte                               | 13 ++++++++++
 4 files changed, 49 insertions(+), 15 deletions(-)
 create mode 100644 test/runtime/samples/binding-select-initial-value-undefined-2/_config.js
 create mode 100644 test/runtime/samples/binding-select-initial-value-undefined-2/main.svelte

diff --git a/src/compiler/compile/render_dom/wrappers/Element/Binding.ts b/src/compiler/compile/render_dom/wrappers/Element/Binding.ts
index e48f3e0030..aa3d097ce3 100644
--- a/src/compiler/compile/render_dom/wrappers/Element/Binding.ts
+++ b/src/compiler/compile/render_dom/wrappers/Element/Binding.ts
@@ -145,8 +145,8 @@ export default class BindingWrapper {
 		}
 
 		// model to view
-		let update_dom = get_dom_updater(parent, this);
-		let mount_dom = update_dom;
+		let update_dom = get_dom_updater(parent, this, false);
+		let mount_dom = get_dom_updater(parent, this, true);
 
 		// special cases
 		switch (this.node.name) {
@@ -234,7 +234,8 @@ export default class BindingWrapper {
 
 function get_dom_updater(
 	element: ElementWrapper | InlineComponentWrapper,
-	binding: BindingWrapper
+	binding: BindingWrapper,
+	mounting: boolean
 ) {
 	const { node } = element;
 
@@ -249,6 +250,7 @@ function get_dom_updater(
 	if (node.name === 'select') {
 		return node.get_static_attribute_value('multiple') === true ?
 			b`@select_options(${element.var}, ${binding.snippet})` :
+			mounting ? b`@select_option(${element.var}, ${binding.snippet}, true)` :
 			b`@select_option(${element.var}, ${binding.snippet})`;
 	}
 
@@ -439,7 +441,7 @@ function get_value_from_dom(
 		return x`$$value`;
 	}
 
-	// <select bind:value='selected>
+	// <select bind:value='selected'>
 	if (node.name === 'select') {
 		return node.get_static_attribute_value('multiple') === true ?
 			x`@select_multiple_value(this)` :
diff --git a/src/runtime/internal/dom.ts b/src/runtime/internal/dom.ts
index 70e7dcd7f8..99894d3d88 100644
--- a/src/runtime/internal/dom.ts
+++ b/src/runtime/internal/dom.ts
@@ -606,7 +606,7 @@ export function set_style(node, key, value, important) {
 	}
 }
 
-export function select_option(select, value) {
+export function select_option(select, value, mounting) {
 	for (let i = 0; i < select.options.length; i += 1) {
 		const option = select.options[i];
 
@@ -616,7 +616,9 @@ export function select_option(select, value) {
 		}
 	}
 
-	select.selectedIndex = -1; // no option should be selected
+	if (!mounting || value !== undefined) {
+		select.selectedIndex = -1; // no option should be selected
+	}
 }
 
 export function select_options(select, value) {
@@ -626,16 +628,8 @@ export function select_options(select, value) {
 	}
 }
 
-function first_enabled_option(select) {
-	for (const option of select.options) {
-		if (!option.disabled) {
-			return option;
-		}
-	}
-}
-
 export function select_value(select) {
-	const selected_option = select.querySelector(':checked') || first_enabled_option(select);
+	const selected_option = select.querySelector(':checked');
 	return selected_option && selected_option.__value;
 }
 
diff --git a/test/runtime/samples/binding-select-initial-value-undefined-2/_config.js b/test/runtime/samples/binding-select-initial-value-undefined-2/_config.js
new file mode 100644
index 0000000000..f7c91809bd
--- /dev/null
+++ b/test/runtime/samples/binding-select-initial-value-undefined-2/_config.js
@@ -0,0 +1,25 @@
+export default {
+	skip_if_ssr: true, // TODO would be nice to fix this in SSR as well
+
+	html: `
+		<p>selected: b</p>
+
+		<select>
+			<option value='a'>a</option>
+			<option value='b'>b</option>
+			<option value='c'>c</option>
+		</select>
+
+		<p>selected: b</p>
+	`,
+
+	test({ assert, component, target }) {
+		assert.equal(component.selected, 'b');
+		const select = target.querySelector('select');
+		const options = [...target.querySelectorAll('option')];
+
+		// option with selected attribute should be selected
+		assert.equal(select.value, 'b');
+		assert.ok(options[1].selected);
+	}
+};
diff --git a/test/runtime/samples/binding-select-initial-value-undefined-2/main.svelte b/test/runtime/samples/binding-select-initial-value-undefined-2/main.svelte
new file mode 100644
index 0000000000..0640a9ce27
--- /dev/null
+++ b/test/runtime/samples/binding-select-initial-value-undefined-2/main.svelte
@@ -0,0 +1,13 @@
+<script>
+	export let selected;
+</script>
+
+<p>selected: {selected}</p>
+
+<select bind:value={selected}>
+	<option>a</option>
+	<option selected>b</option>
+	<option>c</option>
+</select>
+
+<p>selected: {selected}</p>
\ No newline at end of file