From 0f43ad40abea15295eac26f389d6de24ec888fe4 Mon Sep 17 00:00:00 2001 From: Tan Li Hau Date: Mon, 8 Jun 2020 22:43:59 +0800 Subject: [PATCH] fix reactivity with assigning item in each block (#4945) --- CHANGELOG.md | 1 + .../compile/nodes/shared/Expression.ts | 25 +++++ .../each-blocks-assignment-2/_config.js | 20 ++++ .../each-blocks-assignment-2/main.svelte | 12 +++ .../samples/each-blocks-assignment/_config.js | 97 +++++++++++++++++++ .../each-blocks-assignment/main.svelte | 13 +++ 6 files changed, 168 insertions(+) create mode 100644 test/runtime/samples/each-blocks-assignment-2/_config.js create mode 100644 test/runtime/samples/each-blocks-assignment-2/main.svelte create mode 100644 test/runtime/samples/each-blocks-assignment/_config.js create mode 100644 test/runtime/samples/each-blocks-assignment/main.svelte diff --git a/CHANGELOG.md b/CHANGELOG.md index aa2e0339c0..0f7bec3c54 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -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 diff --git a/src/compiler/compile/nodes/shared/Expression.ts b/src/compiler/compile/nodes/shared/Expression.ts index 2f2541c720..f7c9897d58 100644 --- a/src/compiler/compile/nodes/shared/Expression.ts +++ b/src/compiler/compile/nodes/shared/Expression.ts @@ -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; @@ -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); @@ -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 @@ -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)); } } diff --git a/test/runtime/samples/each-blocks-assignment-2/_config.js b/test/runtime/samples/each-blocks-assignment-2/_config.js new file mode 100644 index 0000000000..e5fe84ea7c --- /dev/null +++ b/test/runtime/samples/each-blocks-assignment-2/_config.js @@ -0,0 +1,20 @@ +export default { + html: ` + foo + + `, + 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, + ` + bar + + ` + ); + }, +}; diff --git a/test/runtime/samples/each-blocks-assignment-2/main.svelte b/test/runtime/samples/each-blocks-assignment-2/main.svelte new file mode 100644 index 0000000000..5ef3ae83ac --- /dev/null +++ b/test/runtime/samples/each-blocks-assignment-2/main.svelte @@ -0,0 +1,12 @@ + + +{#each arr as o} + {o.prop} + +{/each} \ No newline at end of file diff --git a/test/runtime/samples/each-blocks-assignment/_config.js b/test/runtime/samples/each-blocks-assignment/_config.js new file mode 100644 index 0000000000..ac07ad1656 --- /dev/null +++ b/test/runtime/samples/each-blocks-assignment/_config.js @@ -0,0 +1,97 @@ +export default { + html: ` + + 1 + + 2 + + 3 + + `, + 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, + ` + + 2 + + 2 + + 3 + + ` + ); + + await buttons[0].dispatchEvent(clickEvent); + + assert.htmlEqual( + target.innerHTML, + ` + + 4 + + 2 + + 3 + + ` + ); + + await buttons[2].dispatchEvent(clickEvent); + await buttons[2].dispatchEvent(clickEvent); + + assert.htmlEqual( + target.innerHTML, + ` + + 4 + + 2 + + 12 + + ` + ); + + await incrementBtn.dispatchEvent(clickEvent); + + assert.htmlEqual( + target.innerHTML, + ` + + 4 + + 2 + + 12 + + 4 + + ` + ); + + [incrementBtn, ...buttons] = target.querySelectorAll("button"); + + await buttons[3].dispatchEvent(clickEvent); + + assert.htmlEqual( + target.innerHTML, + ` + + 4 + + 2 + + 12 + + 8 + + ` + ); + }, +}; diff --git a/test/runtime/samples/each-blocks-assignment/main.svelte b/test/runtime/samples/each-blocks-assignment/main.svelte new file mode 100644 index 0000000000..f74bffbe04 --- /dev/null +++ b/test/runtime/samples/each-blocks-assignment/main.svelte @@ -0,0 +1,13 @@ + + + +{#each arr as o} + {o} + +{/each} \ No newline at end of file