Skip to content

Commit c1f6ee0

Browse files
authored
fix: handle dynamic selects with falsy select values (#9471)
when options are added later, we need to ensure the select value still stays in sync fixes #9412
1 parent 19f84ca commit c1f6ee0

File tree

6 files changed

+184
-5
lines changed

6 files changed

+184
-5
lines changed

Diff for: .changeset/lazy-spiders-think.md

+5
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
---
2+
'svelte': patch
3+
---
4+
5+
fix: handle dynamic selects with falsy select values

Diff for: packages/svelte/src/internal/client/render.js

+14-5
Original file line numberDiff line numberDiff line change
@@ -881,9 +881,10 @@ export function bind_window_size(type, update) {
881881
const callback = () => update(window[type]);
882882
listen_to_events(window, ['resize'], callback);
883883
}
884+
884885
/**
885886
* Finds the containing `<select>` element and potentially updates its `selected` state.
886-
* @param {Element} dom
887+
* @param {HTMLOptionElement} dom
887888
* @returns {void}
888889
*/
889890
export function selected(dom) {
@@ -902,10 +903,15 @@ export function selected(dom) {
902903
const select_value = select.__value;
903904
// @ts-ignore
904905
const option_value = dom.__value;
905-
// @ts-ignore
906-
dom.selected = select_value === option_value;
907-
// @ts-ignore
906+
const selected = select_value === option_value;
907+
dom.selected = selected;
908908
dom.value = option_value;
909+
// Handle the edge case of new options being added to a select when its state is "nothing selected"
910+
// and keeping the selection state in sync (the DOM auto-selects the first option on insert)
911+
// @ts-ignore
912+
if (select.__value === null) {
913+
/** @type {HTMLSelectElement} */ (select).value = '';
914+
}
909915
}
910916
});
911917
}
@@ -957,13 +963,16 @@ export function bind_select_value(dom, get_value, update) {
957963
}
958964
update(value);
959965
});
960-
render_effect(() => {
966+
// 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
967+
effect(() => {
961968
const value = get_value();
962969
if (value == null && !mounted) {
963970
/** @type {HTMLOptionElement | null} */
964971
let selected_option = value === undefined ? dom.querySelector(':checked') : null;
965972
if (selected_option === null) {
966973
dom.value = '';
974+
// @ts-ignore
975+
dom.__value = null;
967976
}
968977
const options = dom.querySelectorAll('option');
969978
for (const option of options) {
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,27 @@
1+
import { ok, test } from '../../test';
2+
3+
export default test({
4+
skip_if_ssr: 'permanent',
5+
6+
html: `
7+
<p>selected: a</p>
8+
9+
<select>
10+
<option disabled=''>x</option>
11+
<option value="a">a</option>
12+
<option value="b">b</option>
13+
<option value="c">c</option>
14+
</select>
15+
`,
16+
17+
test({ assert, component, target }) {
18+
assert.equal(component.selected, 'a');
19+
const select = target.querySelector('select');
20+
ok(select);
21+
const options = [...target.querySelectorAll('option')];
22+
23+
// first enabled option should be selected
24+
assert.equal(select.value, 'a');
25+
assert.ok(options[1].selected);
26+
}
27+
});
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,12 @@
1+
<script>
2+
export let selected;
3+
</script>
4+
5+
<p>selected: {selected}</p>
6+
7+
<select bind:value={selected}>
8+
<option disabled>x</option>
9+
{#each ["a", "b", "c"] as val}
10+
<option>{val}</option>
11+
{/each}
12+
</select>
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,112 @@
1+
import { ok, test } from '../../test';
2+
3+
// // same test as the previous one, but with a dynamic each block
4+
export default test({
5+
html: `
6+
<p>selected: </p>
7+
8+
<select>
9+
<option value="a">a</option>
10+
<option value="b">b</option>
11+
<option value="c">c</option>
12+
</select>
13+
14+
<p>selected: </p>
15+
`,
16+
17+
async test({ assert, component, target }) {
18+
const select = target.querySelector('select');
19+
ok(select);
20+
21+
const options = [...target.querySelectorAll('option')];
22+
23+
assert.equal(component.selected, null);
24+
25+
// no option should be selected since none of the options matches the bound value
26+
assert.equal(select.value, '');
27+
assert.equal(select.selectedIndex, -1);
28+
assert.ok(!options[0].selected);
29+
30+
component.selected = 'a'; // first option should now be selected
31+
assert.equal(select.value, 'a');
32+
assert.ok(options[0].selected);
33+
34+
assert.htmlEqual(
35+
target.innerHTML,
36+
`
37+
<p>selected: a</p>
38+
39+
<select>
40+
<option value="a">a</option>
41+
<option value="b">b</option>
42+
<option value="c">c</option>
43+
</select>
44+
45+
<p>selected: a</p>
46+
`
47+
);
48+
49+
component.selected = 'd'; // doesn't match an option
50+
51+
// now no option should be selected again
52+
assert.equal(select.value, '');
53+
assert.equal(select.selectedIndex, -1);
54+
assert.ok(!options[0].selected);
55+
56+
assert.htmlEqual(
57+
target.innerHTML,
58+
`
59+
<p>selected: d</p>
60+
61+
<select>
62+
<option value="a">a</option>
63+
<option value="b">b</option>
64+
<option value="c">c</option>
65+
</select>
66+
67+
<p>selected: d</p>
68+
`
69+
);
70+
71+
component.selected = 'b'; // second option should now be selected
72+
assert.equal(select.value, 'b');
73+
assert.ok(options[1].selected);
74+
75+
assert.htmlEqual(
76+
target.innerHTML,
77+
`
78+
<p>selected: b</p>
79+
80+
<select>
81+
<option value="a">a</option>
82+
<option value="b">b</option>
83+
<option value="c">c</option>
84+
</select>
85+
86+
<p>selected: b</p>
87+
`
88+
);
89+
90+
component.selected = undefined; // also doesn't match an option
91+
92+
// now no option should be selected again
93+
assert.equal(select.value, '');
94+
assert.equal(select.selectedIndex, -1);
95+
assert.ok(!options[0].selected);
96+
97+
assert.htmlEqual(
98+
target.innerHTML,
99+
`
100+
<p>selected: </p>
101+
102+
<select>
103+
<option value="a">a</option>
104+
<option value="b">b</option>
105+
<option value="c">c</option>
106+
</select>
107+
108+
<p>selected: </p>
109+
`
110+
);
111+
}
112+
});
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,14 @@
1+
<script>
2+
// set as null so no option will be selected by default
3+
export let selected = null;
4+
</script>
5+
6+
<p>selected: {selected}</p>
7+
8+
<select bind:value={selected}>
9+
{#each ['a', 'b', 'c'] as letter}
10+
<option>{letter}</option>
11+
{/each}
12+
</select>
13+
14+
<p>selected: {selected}</p>

0 commit comments

Comments
 (0)