Skip to content

Commit

Permalink
fix: select option with selected attribute when initial state is unde…
Browse files Browse the repository at this point in the history
…fined (sveltejs#8371)

Resolves a second unintended regression introduced in sveltejs#6170.

Follow-up to sveltejs#8331, this time addressing the root issue so the correct select option won't be deselected in the first place when the initial bound value is undefined.

Fixes sveltejs#8361
  • Loading branch information
theodorejb committed Mar 13, 2023
1 parent 5c14bc5 commit c7dcfac
Show file tree
Hide file tree
Showing 4 changed files with 49 additions and 15 deletions.
10 changes: 6 additions & 4 deletions src/compiler/compile/render_dom/wrappers/Element/Binding.ts
Expand Up @@ -145,8 +145,8 @@ export default class BindingWrapper {
}

// model to view
let update_dom = get_dom_updater(parent, this);
let mount_dom = update_dom;
let update_dom = get_dom_updater(parent, this, false);
let mount_dom = get_dom_updater(parent, this, true);

// special cases
switch (this.node.name) {
Expand Down Expand Up @@ -234,7 +234,8 @@ export default class BindingWrapper {

function get_dom_updater(
element: ElementWrapper | InlineComponentWrapper,
binding: BindingWrapper
binding: BindingWrapper,
mounting: boolean
) {
const { node } = element;

Expand All @@ -249,6 +250,7 @@ function get_dom_updater(
if (node.name === 'select') {
return node.get_static_attribute_value('multiple') === true ?
b`@select_options(${element.var}, ${binding.snippet})` :
mounting ? b`@select_option(${element.var}, ${binding.snippet}, true)` :
b`@select_option(${element.var}, ${binding.snippet})`;
}

Expand Down Expand Up @@ -439,7 +441,7 @@ function get_value_from_dom(
return x`$$value`;
}

// <select bind:value='selected>
// <select bind:value='selected'>
if (node.name === 'select') {
return node.get_static_attribute_value('multiple') === true ?
x`@select_multiple_value(this)` :
Expand Down
16 changes: 5 additions & 11 deletions src/runtime/internal/dom.ts
Expand Up @@ -606,7 +606,7 @@ export function set_style(node, key, value, important) {
}
}

export function select_option(select, value) {
export function select_option(select, value, mounting) {
for (let i = 0; i < select.options.length; i += 1) {
const option = select.options[i];

Expand All @@ -616,7 +616,9 @@ export function select_option(select, value) {
}
}

select.selectedIndex = -1; // no option should be selected
if (!mounting || value !== undefined) {
select.selectedIndex = -1; // no option should be selected
}
}

export function select_options(select, value) {
Expand All @@ -626,16 +628,8 @@ export function select_options(select, value) {
}
}

function first_enabled_option(select) {
for (const option of select.options) {
if (!option.disabled) {
return option;
}
}
}

export function select_value(select) {
const selected_option = select.querySelector(':checked') || first_enabled_option(select);
const selected_option = select.querySelector(':checked');
return selected_option && selected_option.__value;
}

Expand Down
@@ -0,0 +1,25 @@
export default {
skip_if_ssr: true, // TODO would be nice to fix this in SSR as well

html: `
<p>selected: b</p>
<select>
<option value='a'>a</option>
<option value='b'>b</option>
<option value='c'>c</option>
</select>
<p>selected: b</p>
`,

test({ assert, component, target }) {
assert.equal(component.selected, 'b');
const select = target.querySelector('select');
const options = [...target.querySelectorAll('option')];

// option with selected attribute should be selected
assert.equal(select.value, 'b');
assert.ok(options[1].selected);
}
};
@@ -0,0 +1,13 @@
<script>
export let selected;
</script>

<p>selected: {selected}</p>

<select bind:value={selected}>
<option>a</option>
<option selected>b</option>
<option>c</option>
</select>

<p>selected: {selected}</p>

0 comments on commit c7dcfac

Please sign in to comment.