Skip to content

Commit 08b9599

Browse files
committed
fix: make <select> <option value> behavior consistent
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 b2448dc commit 08b9599

File tree

5 files changed

+134
-2
lines changed

5 files changed

+134
-2
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: 25 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -155,10 +155,20 @@ 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)) {
161-
next[key] = null;
162+
if (is_option_element && key === 'value') {
163+
// Because of the "set falsy option values to the empty string" logic below, which can't
164+
// differentiate between a missing value and an explicitly set value of null or undefined,
165+
// we need to remove the attribute here and delete the key from the object.
166+
element.removeAttribute(key);
167+
delete current[key];
168+
delete next[key];
169+
} else {
170+
next[key] = null;
171+
}
162172
}
163173
}
164174

@@ -178,6 +188,20 @@ export function set_attributes(element, prev, next, lowercase_attributes, css_ha
178188
for (const key in next) {
179189
// let instead of var because referenced in a closure
180190
let value = next[key];
191+
192+
// Up here because we want to do this for the initial value, too, even if it's undefined,
193+
// and this wouldn't be reached in case of undefined because of the equality check below
194+
if (is_option_element && key === 'value' && value == null) {
195+
// The <option> element is a special case because removing the value attribute means
196+
// the value is set to the text content of the option element, and setting the value
197+
// to null or undefined means the value is set to the string "null" or "undefined".
198+
// To align with how we handle this case in non-spread-scenarios, this logic is needed.
199+
// @ts-ignore
200+
element.value = element.__value = '';
201+
current[key] = value;
202+
continue;
203+
}
204+
181205
var prev_value = current[key];
182206
if (value === prev_value) continue;
183207

Lines changed: 66 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,66 @@
1+
import { flushSync } from 'svelte';
2+
import { test } from '../../test';
3+
4+
// <option value> is special because falsy values should result in an empty string value attribute
5+
export default test({
6+
mode: ['client'],
7+
test({ assert, target }) {
8+
assert.htmlEqual(
9+
target.innerHTML,
10+
`
11+
<select>
12+
<option value="">Default</option>
13+
</select>
14+
15+
<select>
16+
<option value="">Default</option>
17+
</select>
18+
19+
<select>
20+
<option value="">Default</option>
21+
</select>
22+
23+
<select>
24+
<option value="">Default</option>
25+
</select>
26+
27+
<select>
28+
<option value="">Default</option>
29+
</select>
30+
31+
<button>update reactive spread</button>
32+
`
33+
);
34+
35+
const btn = target.querySelector('button');
36+
btn?.click();
37+
flushSync();
38+
39+
assert.htmlEqual(
40+
target.innerHTML,
41+
`
42+
<select>
43+
<option value="">Default</option>
44+
</select>
45+
46+
<select>
47+
<option value="">Default</option>
48+
</select>
49+
50+
<select>
51+
<option value="">Default</option>
52+
</select>
53+
54+
<select>
55+
<option value="">Default</option>
56+
</select>
57+
58+
<select>
59+
<option>Default</option>
60+
</select>
61+
62+
<button>update reactive spread</button>
63+
`
64+
);
65+
}
66+
});
Lines changed: 28 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,28 @@
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>
27+
28+
<button onclick={() => (reactive_spread = {})}>update reactive spread</button>

0 commit comments

Comments
 (0)