Skip to content

Commit 6000e9b

Browse files
authored
Merge pull request #902 from sveltejs/gh-893
possible fix for #893
2 parents ad12854 + 5646df7 commit 6000e9b

File tree

3 files changed

+70
-1
lines changed

3 files changed

+70
-1
lines changed

src/generators/dom/visitors/Element/Binding.ts

Lines changed: 11 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -61,7 +61,7 @@ export default function visitBinding(
6161
type
6262
);
6363

64-
let setter = getSetter(block, name, snippet, state.parentNode, attribute, dependencies, value);
64+
let setter = getSetter(generator, block, name, snippet, state.parentNode, attribute, dependencies, value);
6565
let updateElement = `${state.parentNode}.${attribute.name} = ${snippet};`;
6666

6767
const needsLock = !isReadOnly && node.name !== 'input' || !/radio|checkbox|range|color/.test(type); // TODO others?
@@ -290,6 +290,7 @@ function getBindingGroup(generator: DomGenerator, value: Node) {
290290
}
291291

292292
function getSetter(
293+
generator: DomGenerator,
293294
block: Block,
294295
name: string,
295296
snippet: string,
@@ -319,6 +320,15 @@ function getSetter(
319320
}
320321

321322
if (attribute.value.type === 'MemberExpression') {
323+
// This is a little confusing, and should probably be tidied up
324+
// at some point. It addresses a tricky bug (#893), wherein
325+
// Svelte tries to `set()` a computed property, which throws an
326+
// error in dev mode. a) it's possible that we should be
327+
// replacing computations with *their* dependencies, and b)
328+
// we should probably populate `generator.readonly` sooner so
329+
// that we don't have to do the `.some()` here
330+
dependencies = dependencies.filter(prop => !generator.computations.some(computation => computation.key === prop));
331+
322332
return deindent`
323333
var state = #component.get();
324334
${snippet} = ${value};
Lines changed: 32 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,32 @@
1+
export default {
2+
dev: true,
3+
4+
html: `
5+
<select>
6+
<option value='A'>A</option>
7+
<option value='B'>B</option>
8+
<option value='C'>C</option>
9+
</select>
10+
`,
11+
12+
test(assert, component, target, window) {
13+
const select = target.querySelector('select');
14+
const options = target.querySelectorAll('option');
15+
16+
const change = new window.Event('change');
17+
18+
options[1].selected = true;
19+
select.dispatchEvent(change);
20+
21+
assert.equal(component.get('selected').letter, 'B');
22+
assert.htmlEqual(target.innerHTML, `
23+
<select>
24+
<option value='A'>A</option>
25+
<option value='B'>B</option>
26+
<option value='C'>C</option>
27+
</select>
28+
29+
B
30+
`);
31+
}
32+
};
Lines changed: 27 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,27 @@
1+
<select bind:value="selected.letter">
2+
{{#each uppercase as letter}}
3+
<option value="{{letter}}">{{letter}}</option>
4+
{{/each}}
5+
</select>
6+
7+
{{selected.letter}}
8+
9+
<script>
10+
export default {
11+
data() {
12+
return {
13+
letters: [
14+
'a',
15+
'b',
16+
'c'
17+
],
18+
selected: {
19+
letter: ''
20+
}
21+
}
22+
},
23+
computed:{
24+
uppercase: letters => letters.map(x => x.toUpperCase())
25+
}
26+
};
27+
</script>

0 commit comments

Comments
 (0)