Skip to content

Commit 63f3ee4

Browse files
authored
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
1 parent ba93e5f commit 63f3ee4

File tree

5 files changed

+94
-1
lines changed

5 files changed

+94
-1
lines changed

.changeset/light-hounds-carry.md

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
---
2+
'svelte': patch
3+
---
4+
5+
fix: make `<select>` `<option value>` behavior consistent

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

Lines changed: 10 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -617,7 +617,16 @@ function serialize_element_special_value_attribute(element, node_id, attribute,
617617

618618
if (is_reactive) {
619619
const id = state.scope.generate(`${node_id.name}_value`);
620-
serialize_update_assignment(state, id, undefined, value, update);
620+
serialize_update_assignment(
621+
state,
622+
id,
623+
// `<option>` is a special case: The value property reflects to the DOM. If the value is set to undefined,
624+
// that means the value should be set to the empty string. To be able to do that when the value is
625+
// initially undefined, we need to set a value that is guaranteed to be different.
626+
element === 'option' ? b.object([]) : undefined,
627+
value,
628+
update
629+
);
621630
return true;
622631
} else {
623632
state.init.push(update);

packages/svelte/src/internal/client/dom/elements/attributes.js

Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -155,6 +155,7 @@ export function set_custom_element_data(node, prop, value) {
155155
export function set_attributes(element, prev, next, lowercase_attributes, css_hash) {
156156
var has_hash = css_hash.length !== 0;
157157
var current = prev || {};
158+
var is_option_element = element.tagName === 'OPTION';
158159

159160
for (var key in prev) {
160161
if (!(key in next)) {
@@ -178,6 +179,26 @@ export function set_attributes(element, prev, next, lowercase_attributes, css_ha
178179
for (const key in next) {
179180
// let instead of var because referenced in a closure
180181
let value = next[key];
182+
183+
// Up here because we want to do this for the initial value, too, even if it's undefined,
184+
// and this wouldn't be reached in case of undefined because of the equality check below
185+
if (is_option_element && key === 'value' && value == null) {
186+
// The <option> element is a special case because removing the value attribute means
187+
// the value is set to the text content of the option element, and setting the value
188+
// to null or undefined means the value is set to the string "null" or "undefined".
189+
// To align with how we handle this case in non-spread-scenarios, this logic is needed.
190+
// There's a super-edge-case bug here that is left in in favor of smaller code size:
191+
// Because of the "set missing props to null" logic above, we can't differentiate
192+
// between a missing value and an explicitly set value of null or undefined. That means
193+
// that once set, the value attribute of an <option> element can't be removed. This is
194+
// a very rare edge case, and removing the attribute altogether isn't possible either
195+
// for the <option value={undefined}> case, so we're not losing any functionality here.
196+
// @ts-ignore
197+
element.value = element.__value = '';
198+
current[key] = value;
199+
continue;
200+
}
201+
181202
var prev_value = current[key];
182203
if (value === prev_value) continue;
183204

Lines changed: 32 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,32 @@
1+
import { test } from '../../test';
2+
3+
// <option value> is special because falsy values should result in an empty string value attribute
4+
export default test({
5+
mode: ['client'],
6+
test({ assert, target }) {
7+
assert.htmlEqual(
8+
target.innerHTML,
9+
`
10+
<select>
11+
<option value="">Default</option>
12+
</select>
13+
14+
<select>
15+
<option value="">Default</option>
16+
</select>
17+
18+
<select>
19+
<option value="">Default</option>
20+
</select>
21+
22+
<select>
23+
<option value="">Default</option>
24+
</select>
25+
26+
<select>
27+
<option value="">Default</option>
28+
</select>
29+
`
30+
);
31+
}
32+
});
Lines changed: 26 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,26 @@
1+
<script>
2+
let nonreactive = undefined;
3+
let reactive = $state();
4+
let nonreactive_spread = { value: undefined };
5+
let reactive_spread = $state({ value: undefined });
6+
</script>
7+
8+
<select>
9+
<option value={undefined}>Default</option>
10+
</select>
11+
12+
<select>
13+
<option value={nonreactive}>Default</option>
14+
</select>
15+
16+
<select>
17+
<option value={reactive}>Default</option>
18+
</select>
19+
20+
<select>
21+
<option {...nonreactive_spread}>Default</option>
22+
</select>
23+
24+
<select>
25+
<option {...reactive_spread}>Default</option>
26+
</select>

0 commit comments

Comments
 (0)