Skip to content

Commit

Permalink
fix select bind:value when option value change
Browse files Browse the repository at this point in the history
  • Loading branch information
tanhauhau committed Jun 23, 2020
1 parent 1644f20 commit 0174c1a
Show file tree
Hide file tree
Showing 6 changed files with 94 additions and 17 deletions.
32 changes: 26 additions & 6 deletions src/compiler/compile/render_dom/wrappers/Element/Attribute.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ import { string_literal } from '../../../utils/stringify';
import { b, x } from 'code-red';
import Expression from '../../../nodes/shared/Expression';
import Text from '../../../nodes/Text';
import handle_select_value_binding from './handle_select_value_binding';

export default class AttributeWrapper {
node: Attribute;
Expand Down Expand Up @@ -36,6 +37,10 @@ export default class AttributeWrapper {
});
}
}

if (node.name === 'value') {
handle_select_value_binding(this, node.dependencies);
}
}
}

Expand Down Expand Up @@ -74,7 +79,7 @@ export default class AttributeWrapper {

const is_legacy_input_type = element.renderer.component.compile_options.legacy && name === 'type' && this.parent.node.name === 'input';

const dependencies = this.node.get_dependencies();
const dependencies = this.get_dependencies();
const value = this.get_value(block);

const is_src = this.node.name === 'src'; // TODO retire this exception in favour of https://github.com/sveltejs/svelte/issues/3750
Expand All @@ -83,7 +88,7 @@ export default class AttributeWrapper {

const is_input_value = name === 'value' && element.node.name === 'input';

const should_cache = is_src || this.node.should_cache() || is_select_value_attribute; // TODO is this necessary?
const should_cache = is_src || this.node.should_cache();

const last = should_cache && block.get_unique_name(
`${element.var.name}_${name.replace(/[^a-zA-Z_$]/g, '_')}_value`
Expand All @@ -104,13 +109,12 @@ export default class AttributeWrapper {
const is_multiple_select = element.node.get_static_attribute_value('multiple');

if (is_multiple_select) {
updater = b`@select_options(${element.var}, ${last});`;
updater = b`@select_options(${element.var}, ${value});`;
} else {
updater = b`@select_option(${element.var}, ${last});`;
updater = b`@select_option(${element.var}, ${value});`;
}

block.chunks.mount.push(b`
${last} = ${value};
${updater}
`);
} else if (is_src) {
Expand Down Expand Up @@ -168,10 +172,26 @@ export default class AttributeWrapper {
const update_value = b`${element.var}.value = ${element.var}.__value;`;

block.chunks.hydrate.push(update_value);
if (this.node.get_dependencies().length > 0) block.chunks.update.push(update_value);
if (dependencies.length > 0) block.chunks.update.push(update_value);
}
}

get_dependencies() {
const node_dependencies = this.node.get_dependencies();
const dependencies = new Set(node_dependencies);

node_dependencies.forEach((prop: string) => {
const indirect_dependencies = this.parent.renderer.component.indirect_dependencies.get(prop);
if (indirect_dependencies) {
indirect_dependencies.forEach(indirect_dependency => {
dependencies.add(indirect_dependency);
});
}
});

return Array.from(dependencies);
}

get_metadata() {
if (this.parent.node.namespace) return null;
const metadata = attribute_lookup[fix_attribute_casing(this.node.name)];
Expand Down
9 changes: 3 additions & 6 deletions src/compiler/compile/render_dom/wrappers/Element/Binding.ts
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ import flatten_reference from '../../../utils/flatten_reference';
import { Node, Identifier } from 'estree';
import add_to_set from '../../../utils/add_to_set';
import mark_each_block_bindings from '../shared/mark_each_block_bindings';
import handle_select_value_binding from './handle_select_value_binding';

export default class BindingWrapper {
node: Binding;
Expand All @@ -35,12 +36,8 @@ export default class BindingWrapper {
block.add_dependencies(dependencies);

// TODO does this also apply to e.g. `<input type='checkbox' bind:group='foo'>`?
if (parent.node.name === 'select') {
(parent as ElementWrapper).select_binding_dependencies = dependencies;
dependencies.forEach((prop: string) => {
parent.renderer.component.indirect_dependencies.set(prop, new Set());
});
}

handle_select_value_binding(this, dependencies);

if (node.is_contextual) {
mark_each_block_bindings(this.parent, this.node);
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,16 @@
import AttributeWrapper from "./Attribute";
import BindingWrapper from "./Binding";
import ElementWrapper from "./index";

export default function handle_select_value_binding(
attr: AttributeWrapper | BindingWrapper,
dependencies: Set<string>
) {
const { parent } = attr;
if (parent.node.name === "select") {
(parent as ElementWrapper).select_binding_dependencies = dependencies;
dependencies.forEach((prop: string) => {
parent.renderer.component.indirect_dependencies.set(prop, new Set());
});
}
}
8 changes: 3 additions & 5 deletions test/js/samples/select-dynamic-value/expected.js
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,6 @@ function create_fragment(ctx) {
let select;
let option0;
let option1;
let select_value_value;

return {
c() {
Expand All @@ -33,12 +32,11 @@ function create_fragment(ctx) {
insert(target, select, anchor);
append(select, option0);
append(select, option1);
select_value_value = /*current*/ ctx[0];
select_option(select, select_value_value);
select_option(select, /*current*/ ctx[0]);
},
p(ctx, [dirty]) {
if (dirty & /*current*/ 1 && select_value_value !== (select_value_value = /*current*/ ctx[0])) {
select_option(select, select_value_value);
if (dirty & /*current*/ 1) {
select_option(select, /*current*/ ctx[0]);
}
},
i: noop,
Expand Down
34 changes: 34 additions & 0 deletions test/runtime/samples/binding-select-late-3/_config.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,34 @@
export default {
props: {
items: [],
selected: 'two'
},

html: `
<select></select>
<p>selected: two</p>
`,

ssrHtml: `
<select value="two"></select>
<p>selected: two</p>
`,

test({ assert, component, target }) {
component.items = [ 'one', 'two', 'three' ];

const options = target.querySelectorAll('option');
assert.ok(!options[0].selected);
assert.ok(options[1].selected);
assert.ok(!options[2].selected);

assert.htmlEqual(target.innerHTML, `
<select>
<option value='one'>one</option>
<option value='two'>two</option>
<option value='three'>three</option>
</select>
<p>selected: two</p>
`);
}
};
12 changes: 12 additions & 0 deletions test/runtime/samples/binding-select-late-3/main.svelte
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
<script>
export let selected;
export let items;
</script>

<select value={selected} on:blur={e => selected = e.target.value}>
{#each items as item}
<option>{item}</option>
{/each}
</select>

<p>selected: {selected || 'nothing'}</p>

0 comments on commit 0174c1a

Please sign in to comment.