From 63f3ee4ffd3131030ee82979f78aba24153c90fe Mon Sep 17 00:00:00 2001
From: Simon H <5968653+dummdidumm@users.noreply.github.com>
Date: Mon, 8 Jul 2024 21:06:57 +0200
Subject: [PATCH] fix: make `<select>` `<option value>` behavior consistent
 (#12316)

Setting the `value` attribute of an `<option>` element to a falsy value should result in the empty string. This wasn't happening in all situations previously.
Fixes #11616
---
 .changeset/light-hounds-carry.md              |  5 +++
 .../3-transform/client/visitors/template.js   | 11 ++++++-
 .../client/dom/elements/attributes.js         | 21 ++++++++++++
 .../samples/select-falsy-value/_config.js     | 32 +++++++++++++++++++
 .../samples/select-falsy-value/main.svelte    | 26 +++++++++++++++
 5 files changed, 94 insertions(+), 1 deletion(-)
 create mode 100644 .changeset/light-hounds-carry.md
 create mode 100644 packages/svelte/tests/runtime-runes/samples/select-falsy-value/_config.js
 create mode 100644 packages/svelte/tests/runtime-runes/samples/select-falsy-value/main.svelte

diff --git a/.changeset/light-hounds-carry.md b/.changeset/light-hounds-carry.md
new file mode 100644
index 0000000000..9e124f900f
--- /dev/null
+++ b/.changeset/light-hounds-carry.md
@@ -0,0 +1,5 @@
+---
+'svelte': patch
+---
+
+fix: make `<select>` `<option value>` behavior consistent
diff --git a/packages/svelte/src/compiler/phases/3-transform/client/visitors/template.js b/packages/svelte/src/compiler/phases/3-transform/client/visitors/template.js
index 594295c467..deaf7dd208 100644
--- a/packages/svelte/src/compiler/phases/3-transform/client/visitors/template.js
+++ b/packages/svelte/src/compiler/phases/3-transform/client/visitors/template.js
@@ -617,7 +617,16 @@ function serialize_element_special_value_attribute(element, node_id, attribute,
 
 	if (is_reactive) {
 		const id = state.scope.generate(`${node_id.name}_value`);
-		serialize_update_assignment(state, id, undefined, value, update);
+		serialize_update_assignment(
+			state,
+			id,
+			// `<option>` is a special case: The value property reflects to the DOM. If the value is set to undefined,
+			// that means the value should be set to the empty string. To be able to do that when the value is
+			// initially undefined, we need to set a value that is guaranteed to be different.
+			element === 'option' ? b.object([]) : undefined,
+			value,
+			update
+		);
 		return true;
 	} else {
 		state.init.push(update);
diff --git a/packages/svelte/src/internal/client/dom/elements/attributes.js b/packages/svelte/src/internal/client/dom/elements/attributes.js
index c65acc9a24..5856e5e65e 100644
--- a/packages/svelte/src/internal/client/dom/elements/attributes.js
+++ b/packages/svelte/src/internal/client/dom/elements/attributes.js
@@ -155,6 +155,7 @@ export function set_custom_element_data(node, prop, value) {
 export function set_attributes(element, prev, next, lowercase_attributes, css_hash) {
 	var has_hash = css_hash.length !== 0;
 	var current = prev || {};
+	var is_option_element = element.tagName === 'OPTION';
 
 	for (var key in prev) {
 		if (!(key in next)) {
@@ -178,6 +179,26 @@ export function set_attributes(element, prev, next, lowercase_attributes, css_ha
 	for (const key in next) {
 		// let instead of var because referenced in a closure
 		let value = next[key];
+
+		// Up here because we want to do this for the initial value, too, even if it's undefined,
+		// and this wouldn't be reached in case of undefined because of the equality check below
+		if (is_option_element && key === 'value' && value == null) {
+			// The <option> element is a special case because removing the value attribute means
+			// the value is set to the text content of the option element, and setting the value
+			// to null or undefined means the value is set to the string "null" or "undefined".
+			// To align with how we handle this case in non-spread-scenarios, this logic is needed.
+			// There's a super-edge-case bug here that is left in in favor of smaller code size:
+			// Because of the "set missing props to null" logic above, we can't differentiate
+			// between a missing value and an explicitly set value of null or undefined. That means
+			// that once set, the value attribute of an <option> element can't be removed. This is
+			// a very rare edge case, and removing the attribute altogether isn't possible either
+			// for the <option value={undefined}> case, so we're not losing any functionality here.
+			// @ts-ignore
+			element.value = element.__value = '';
+			current[key] = value;
+			continue;
+		}
+
 		var prev_value = current[key];
 		if (value === prev_value) continue;
 
diff --git a/packages/svelte/tests/runtime-runes/samples/select-falsy-value/_config.js b/packages/svelte/tests/runtime-runes/samples/select-falsy-value/_config.js
new file mode 100644
index 0000000000..5d535db521
--- /dev/null
+++ b/packages/svelte/tests/runtime-runes/samples/select-falsy-value/_config.js
@@ -0,0 +1,32 @@
+import { test } from '../../test';
+
+// <option value> is special because falsy values should result in an empty string value attribute
+export default test({
+	mode: ['client'],
+	test({ assert, target }) {
+		assert.htmlEqual(
+			target.innerHTML,
+			`
+			<select>
+				<option value="">Default</option>
+			</select>
+
+			<select>
+				<option value="">Default</option>
+			</select>
+
+			<select>
+				<option value="">Default</option>
+			</select>
+
+			<select>
+				<option value="">Default</option>
+			</select>
+
+			<select>
+				<option value="">Default</option>
+			</select>
+		`
+		);
+	}
+});
diff --git a/packages/svelte/tests/runtime-runes/samples/select-falsy-value/main.svelte b/packages/svelte/tests/runtime-runes/samples/select-falsy-value/main.svelte
new file mode 100644
index 0000000000..95e96c6d19
--- /dev/null
+++ b/packages/svelte/tests/runtime-runes/samples/select-falsy-value/main.svelte
@@ -0,0 +1,26 @@
+<script>
+	let nonreactive = undefined;
+	let reactive = $state();
+	let nonreactive_spread = { value: undefined };
+	let reactive_spread = $state({ value: undefined });
+</script>
+
+<select>
+	<option value={undefined}>Default</option>
+</select>
+
+<select>
+	<option value={nonreactive}>Default</option>
+</select>
+
+<select>
+	<option value={reactive}>Default</option>
+</select>
+
+<select>
+	<option {...nonreactive_spread}>Default</option>
+</select>
+
+<select>
+	<option {...reactive_spread}>Default</option>
+</select>