Skip to content

Commit

Permalink
fix reactivity with assigning item in each block (sveltejs#4945)
Browse files Browse the repository at this point in the history
  • Loading branch information
tanhauhau authored and taylorzane committed Dec 17, 2020
1 parent 799c9b3 commit 44806f6
Show file tree
Hide file tree
Showing 6 changed files with 168 additions and 0 deletions.
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
## Unreleased

* Fix `bind:this` to the value of an `{#each}` block ([#4517](https://github.com/sveltejs/svelte/issues/4517))
* Fix reactivity when assigning to contextual `{#each}` variable ([#4574](https://github.com/sveltejs/svelte/issues/4574), [#4744](https://github.com/sveltejs/svelte/issues/4744))
* Fix binding to contextual `{#each}` values that shadow outer names ([#4757](https://github.com/sveltejs/svelte/issues/4757))

## 3.23.0
Expand Down
25 changes: 25 additions & 0 deletions src/compiler/compile/nodes/shared/Expression.ts
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,8 @@ import { invalidate } from '../../render_dom/invalidate';
import { Node, FunctionExpression, Identifier } from 'estree';
import { TemplateNode } from '../../../interfaces';
import { is_reserved_keyword } from '../../utils/reserved_keywords';
import replace_object from '../../utils/replace_object';
import EachBlock from '../EachBlock';

type Owner = Wrapper | TemplateNode;

Expand Down Expand Up @@ -134,6 +136,8 @@ export default class Expression {
const variable = component.var_lookup.get(name);
if (variable) variable[deep ? 'mutated' : 'reassigned'] = true;
});
const each_block = template_scope.get_owner(name);
(each_block as EachBlock).has_binding = true;
} else {
component.add_reference(name);

Expand Down Expand Up @@ -311,6 +315,10 @@ export default class Expression {
if (node.type === 'AssignmentExpression' || node.type === 'UpdateExpression') {
const assignee = node.type === 'AssignmentExpression' ? node.left : node.argument;

const object_name = get_object(assignee).name;

if (scope.has(object_name)) return;

// normally (`a = 1`, `b.c = 2`), there'll be a single name
// (a or b). In destructuring cases (`[d, e] = [e, d]`) there
// may be more, in which case we need to tack the extra ones
Expand All @@ -327,6 +335,23 @@ export default class Expression {
}
});

const context = block.bindings.get(object_name);

if (context) {
// for `{#each array as item}`
// replace `item = 1` to `each_array[each_index] = 1`, this allow us to mutate the array
// rather than mutating the local `item` variable
const { snippet, object, property } = context;
const replaced: any = replace_object(assignee, snippet);
if (node.type === 'AssignmentExpression') {
node.left = replaced;
} else {
node.argument = replaced;
}
contextual_dependencies.add(object.name);
contextual_dependencies.add(property.name);
}

this.replace(invalidate(block.renderer, scope, node, traced));
}
}
Expand Down
20 changes: 20 additions & 0 deletions test/runtime/samples/each-blocks-assignment-2/_config.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,20 @@
export default {
html: `
<span class="content">foo</span>
<button>Test</button>
`,
async test({ assert, component, target, window }) {
const button = target.querySelector("button");

const clickEvent = new window.MouseEvent("click");
await button.dispatchEvent(clickEvent);

assert.htmlEqual(
target.innerHTML,
`
<span class="content">bar</span>
<button>Test</button>
`
);
},
};
12 changes: 12 additions & 0 deletions test/runtime/samples/each-blocks-assignment-2/main.svelte
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
<script>
let obj = {
prop: "foo"
};
let arr = [obj]
</script>

{#each arr as o}
<span class="content">{o.prop}</span>
<button on:click={ () => o = { ...o, prop: "bar" } }>Test</button>
{/each}
97 changes: 97 additions & 0 deletions test/runtime/samples/each-blocks-assignment/_config.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,97 @@
export default {
html: `
<button>Add</button>
<span class="content">1</span>
<button>Test</button>
<span class="content">2</span>
<button>Test</button>
<span class="content">3</span>
<button>Test</button>
`,
async test({ assert, component, target, window }) {
let [incrementBtn, ...buttons] = target.querySelectorAll("button");

const clickEvent = new window.MouseEvent("click");
await buttons[0].dispatchEvent(clickEvent);

assert.htmlEqual(
target.innerHTML,
`
<button>Add</button>
<span class="content">2</span>
<button>Test</button>
<span class="content">2</span>
<button>Test</button>
<span class="content">3</span>
<button>Test</button>
`
);

await buttons[0].dispatchEvent(clickEvent);

assert.htmlEqual(
target.innerHTML,
`
<button>Add</button>
<span class="content">4</span>
<button>Test</button>
<span class="content">2</span>
<button>Test</button>
<span class="content">3</span>
<button>Test</button>
`
);

await buttons[2].dispatchEvent(clickEvent);
await buttons[2].dispatchEvent(clickEvent);

assert.htmlEqual(
target.innerHTML,
`
<button>Add</button>
<span class="content">4</span>
<button>Test</button>
<span class="content">2</span>
<button>Test</button>
<span class="content">12</span>
<button>Test</button>
`
);

await incrementBtn.dispatchEvent(clickEvent);

assert.htmlEqual(
target.innerHTML,
`
<button>Add</button>
<span class="content">4</span>
<button>Test</button>
<span class="content">2</span>
<button>Test</button>
<span class="content">12</span>
<button>Test</button>
<span class="content">4</span>
<button>Test</button>
`
);

[incrementBtn, ...buttons] = target.querySelectorAll("button");

await buttons[3].dispatchEvent(clickEvent);

assert.htmlEqual(
target.innerHTML,
`
<button>Add</button>
<span class="content">4</span>
<button>Test</button>
<span class="content">2</span>
<button>Test</button>
<span class="content">12</span>
<button>Test</button>
<span class="content">8</span>
<button>Test</button>
`
);
},
};
13 changes: 13 additions & 0 deletions test/runtime/samples/each-blocks-assignment/main.svelte
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
<script>
let obj = {
prop: "foo"
};
let arr = [1, 2, 3]
</script>

<button on:click={() => arr = [...arr, arr.length + 1]}>Add</button>
{#each arr as o}
<span class="content">{o}</span>
<button on:click={() => { o *= 2; }}>Test</button>
{/each}

0 comments on commit 44806f6

Please sign in to comment.