Fix select binding when matching enabled option has null value

Resolves #9545

Author: theodorejb

packages/svelte/src/compiler/phases/3-transform/client/visitors/template.js

@@ -567,7 +567,11 @@ function serialize_element_special_value_attribute(element, node_id, attribute,
 	const inner_assignment = b.assignment(
 		'=',
 		b.member(node_id, b.id('value')),
-		b.assignment('=', b.member(node_id, b.id('__value')), value)
+		b.conditional(
+			b.binary('==', b.literal(null), b.assignment('=', b.member(node_id, b.id('__value')), value)),
+			b.literal(''), // render null/undefined values as empty string to support placeholder options
+			value
+		)
 	);
 	const is_reactive = attribute.metadata.dynamic;
 	const needs_selected_call =
@@ -582,9 +586,12 @@ function serialize_element_special_value_attribute(element, node_id, attribute,
 					b.call('$.selected', node_id)
 			  ])
 			: needs_option_call
-			? // This ensures a one-way street to the DOM in case it's <select {value}>
-			  // and not <select bind:value>
-			  b.call('$.select_option', node_id, inner_assignment)
+			? b.sequence([
+					inner_assignment,
+					// This ensures a one-way street to the DOM in case it's <select {value}>
+					// and not <select bind:value>
+					b.call('$.select_option', node_id, value)
+			  ])
 			: inner_assignment
 	);
 

packages/svelte/src/internal/client/render.js

@@ -444,20 +444,22 @@ export function class_toggle(dom, class_name, value) {
  * @template V
  * @param {HTMLSelectElement} select
  * @param {V} value
+ * @param {boolean} [mounting]
  */
-export function select_option(select, value) {
+export function select_option(select, value, mounting) {
 	if (select.multiple) {
 		return select_options(select, value);
 	}
-	for (let i = 0; i < select.options.length; i += 1) {
-		const option = select.options[i];
+	for (const option of select.options) {
 		const option_value = get_option_value(option);
 		if (option_value === value) {
 			option.selected = true;
 			return;
 		}
 	}
-	select.value = '';
+	if (!mounting || value !== undefined) {
+		select.value = '';
+	}
 }
 
 /**
@@ -466,8 +468,7 @@ export function select_option(select, value) {
  * @param {V} value
  */
 function select_options(select, value) {
-	for (let i = 0; i < select.options.length; i += 1) {
-		const option = select.options[i];
+	for (const option of select.options) {
 		// @ts-ignore
 		option.selected = ~value.indexOf(get_option_value(option));
 	}
@@ -897,20 +898,10 @@ export function selected(dom) {
 			}
 			select = select.parentNode;
 		}
-		if (select != null) {
-			// @ts-ignore
-			const select_value = select.__value;
-			// @ts-ignore
-			const option_value = dom.__value;
-			const selected = select_value === option_value;
-			dom.selected = selected;
-			dom.value = option_value;
-			// Handle the edge case of new options being added to a select when its state is "nothing selected"
-			// and keeping the selection state in sync (the DOM auto-selects the first option on insert)
-			// @ts-ignore
-			if (select.__value === null) {
-				/** @type {HTMLSelectElement} */ (select).value = '';
-			}
+		// @ts-ignore
+		if (select != null && dom.__value === select.__value) {
+			// never set to false, since this causes browser to select default option
+			dom.selected = true;
 		}
 	});
 }
@@ -949,7 +940,7 @@ export function bind_value(dom, get_value, update) {
  * @returns {void}
  */
 export function bind_select_value(dom, get_value, update) {
-	let mounted = false;
+	let mounting = true;
 	dom.addEventListener('change', () => {
 		/** @type {unknown} */
 		let value;
@@ -964,40 +955,19 @@ export function bind_select_value(dom, get_value, update) {
 	});
 	// Needs to be an effect, not a render_effect, so that in case of each loops the logic runs after the each block has updated
 	effect(() => {
-		const value = get_value();
-		if (value == null && !mounted) {
+		let value = get_value();
+		select_option(dom, value, mounting);
+		if (mounting && value === undefined) {
 			/** @type {HTMLOptionElement | null} */
-			let selected_option = value === undefined ? dom.querySelector(':checked') : null;
-			if (selected_option === null) {
-				dom.value = '';
-				// @ts-ignore
-				dom.__value = null;
+			let selected_option = dom.querySelector(':checked');
+			if (selected_option !== null) {
+				value = get_option_value(selected_option);
+				update(value);
 			}
-			const options = dom.querySelectorAll('option');
-			for (const option of options) {
-				if (get_option_value(option) === value || option.hasAttribute('selected')) {
-					if (option.disabled) {
-						option.value = '';
-					}
-					option.selected = true;
-					selected_option = option;
-					break;
-				}
-			}
-			if (selected_option != null) {
-				const non_null_value = get_option_value(selected_option);
-				update(non_null_value);
-				if (selected_option.hasAttribute('selected')) {
-					selected_option.removeAttribute('selected');
-					selected_option.selected = true;
-				}
-			}
-		} else {
-			select_option(dom, value);
-			// @ts-ignore
-			dom.__value = value;
 		}
-		mounted = true;
+		// @ts-ignore
+		dom.__value = value;
+		mounting = false;
 	});
 }
 

packages/svelte/tests/runtime-legacy/samples/binding-select-initial-value-undefined-2/_config.js

@@ -8,7 +8,7 @@ export default test({
 
 		<select>
 			<option>a</option>
-			<option>b</option>
+			<option selected="">b</option>
 			<option>c</option>
 		</select>
 

packages/svelte/tests/runtime-legacy/samples/binding-select-null-placeholder-2/_config.js

@@ -0,0 +1,28 @@
+import { ok, test } from '../../test';
+
+const items = [{ id: 'a' }, { id: 'b' }];
+
+export default test({
+	get props() {
+		return {
+			/** @type {{ id: string } | null} */
+			foo: null,
+			items
+		};
+	},
+
+	test({ assert, component, target }) {
+		const select = target.querySelector('select');
+		ok(select);
+
+		const options = target.querySelectorAll('option');
+
+		assert.equal(options[0].selected, true);
+		assert.equal(options[1].selected, false);
+		assert.equal(options[0].value, '');
+
+		component.foo = items[0];
+		assert.equal(options[0].selected, false);
+		assert.equal(options[1].selected, true);
+	}
+});

packages/svelte/tests/runtime-legacy/samples/binding-select-null-placeholder-2/main.svelte

@@ -0,0 +1,11 @@
+<script>
+	export let foo;
+	export let items;
+</script>
+
+<select bind:value={foo}>
+	<option value={null}></option>
+	{#each items as item}
+		<option value={item}>{item.id}</option>
+	{/each}
+</select>