From 404ed3dbfe84e2d16c2a3658257536e194c33463 Mon Sep 17 00:00:00 2001
From: Tan Li Hau <tanhauhau@users.noreply.github.com>
Date: Sun, 15 Mar 2020 02:27:39 +0800
Subject: [PATCH] fix else-block update in keyed each-block (#4558)

Co-authored-by: Benjamin W. Broersma <bw@broersma.com>
---
 CHANGELOG.md                                  |  1 +
 .../compile/render_dom/wrappers/EachBlock.ts  | 60 +++++++++----------
 .../samples/each-block-keyed-else/_config.js  | 37 ++++++++++++
 .../samples/each-block-keyed-else/main.svelte | 12 ++++
 .../each-block-unkeyed-else-2/_config.js      | 37 ++++++++++++
 .../each-block-unkeyed-else-2/main.svelte     | 12 ++++
 6 files changed, 129 insertions(+), 30 deletions(-)
 create mode 100644 test/runtime/samples/each-block-keyed-else/_config.js
 create mode 100644 test/runtime/samples/each-block-keyed-else/main.svelte
 create mode 100644 test/runtime/samples/each-block-unkeyed-else-2/_config.js
 create mode 100644 test/runtime/samples/each-block-unkeyed-else-2/main.svelte

diff --git a/CHANGELOG.md b/CHANGELOG.md
index 084a900295..1e9024c318 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -4,6 +4,7 @@
 
 * Allow `<svelte:self>` to be used in a slot ([#2798](https://github.com/sveltejs/svelte/issues/2798))
 * Expose object of unknown props in `$$restProps` ([#2930](https://github.com/sveltejs/svelte/issues/2930))
+* Fix updating keyed `{#each}` blocks with `{:else}` ([#4536](https://github.com/sveltejs/svelte/issues/4536), [#4549](https://github.com/sveltejs/svelte/issues/4549))
 * Fix hydration of top-level content ([#4542](https://github.com/sveltejs/svelte/issues/4542))
 
 ## 3.19.2
diff --git a/src/compiler/compile/render_dom/wrappers/EachBlock.ts b/src/compiler/compile/render_dom/wrappers/EachBlock.ts
index f0dfa5fbcc..43a0f754f9 100644
--- a/src/compiler/compile/render_dom/wrappers/EachBlock.ts
+++ b/src/compiler/compile/render_dom/wrappers/EachBlock.ts
@@ -62,6 +62,8 @@ export default class EachBlockWrapper extends Wrapper {
 
 	context_props: Array<Node | Node[]>;
 	index_name: Identifier;
+	updates: Array<Node | Node[]> = [];
+	dependencies: Set<string>;
 
 	var: Identifier = { type: 'Identifier', name: 'each' };
 
@@ -235,6 +237,12 @@ export default class EachBlockWrapper extends Wrapper {
 			update_mount_node
 		};
 
+		const all_dependencies = new Set(this.block.dependencies); // TODO should be dynamic deps only
+		this.node.expression.dynamic_dependencies().forEach((dependency: string) => {
+			all_dependencies.add(dependency);
+		});
+		this.dependencies = all_dependencies;
+
 		if (this.node.key) {
 			this.render_keyed(args);
 		} else {
@@ -291,7 +299,7 @@ export default class EachBlockWrapper extends Wrapper {
 			`);
 
 			if (this.else.block.has_update_method) {
-				block.chunks.update.push(b`
+				this.updates.push(b`
 					if (!${this.vars.data_length} && ${each_block_else}) {
 						${each_block_else}.p(#ctx, #dirty);
 					} else if (!${this.vars.data_length}) {
@@ -304,7 +312,7 @@ export default class EachBlockWrapper extends Wrapper {
 					}
 				`);
 			} else {
-				block.chunks.update.push(b`
+				this.updates.push(b`
 					if (${this.vars.data_length}) {
 						if (${each_block_else}) {
 							${each_block_else}.d(1);
@@ -323,6 +331,14 @@ export default class EachBlockWrapper extends Wrapper {
 			`);
 		}
 
+		if (this.updates.length) {
+			block.chunks.update.push(b`
+				if (${block.renderer.dirty(Array.from(all_dependencies))}) {
+					${this.updates}
+				}
+			`);
+		}
+
 		this.fragment.render(this.block, null, x`#nodes` as Identifier);
 
 		if (this.else) {
@@ -415,24 +431,17 @@ export default class EachBlockWrapper extends Wrapper {
 				? `@outro_and_destroy_block`
 				: `@destroy_block`;
 
-		const all_dependencies = new Set(this.block.dependencies); // TODO should be dynamic deps only
-		this.node.expression.dynamic_dependencies().forEach((dependency: string) => {
-			all_dependencies.add(dependency);
-		});
+		if (this.dependencies.size) {
+			this.updates.push(b`
+				const ${this.vars.each_block_value} = ${snippet};
+				${this.renderer.options.dev && b`@validate_each_argument(${this.vars.each_block_value});`}
 
-		if (all_dependencies.size) {
-			block.chunks.update.push(b`
-				if (${block.renderer.dirty(Array.from(all_dependencies))}) {
-					const ${this.vars.each_block_value} = ${snippet};
-					${this.renderer.options.dev && b`@validate_each_argument(${this.vars.each_block_value});`}
-
-					${this.block.has_outros && b`@group_outros();`}
-					${this.node.has_animation && b`for (let #i = 0; #i < ${view_length}; #i += 1) ${iterations}[#i].r();`}
-					${this.renderer.options.dev && b`@validate_each_keys(#ctx, ${this.vars.each_block_value}, ${this.vars.get_each_context}, ${get_key});`}
-					${iterations} = @update_keyed_each(${iterations}, #dirty, ${get_key}, ${dynamic ? 1 : 0}, #ctx, ${this.vars.each_block_value}, ${lookup}, ${update_mount_node}, ${destroy}, ${create_each_block}, ${update_anchor_node}, ${this.vars.get_each_context});
-					${this.node.has_animation && b`for (let #i = 0; #i < ${view_length}; #i += 1) ${iterations}[#i].a();`}
-					${this.block.has_outros && b`@check_outros();`}
-				}
+				${this.block.has_outros && b`@group_outros();`}
+				${this.node.has_animation && b`for (let #i = 0; #i < ${view_length}; #i += 1) ${iterations}[#i].r();`}
+				${this.renderer.options.dev && b`@validate_each_keys(#ctx, ${this.vars.each_block_value}, ${this.vars.get_each_context}, ${get_key});`}
+				${iterations} = @update_keyed_each(${iterations}, #dirty, ${get_key}, ${dynamic ? 1 : 0}, #ctx, ${this.vars.each_block_value}, ${lookup}, ${update_mount_node}, ${destroy}, ${create_each_block}, ${update_anchor_node}, ${this.vars.get_each_context});
+				${this.node.has_animation && b`for (let #i = 0; #i < ${view_length}; #i += 1) ${iterations}[#i].a();`}
+				${this.block.has_outros && b`@check_outros();`}
 			`);
 		}
 
@@ -504,12 +513,7 @@ export default class EachBlockWrapper extends Wrapper {
 			}
 		`);
 
-		const all_dependencies = new Set(this.block.dependencies); // TODO should be dynamic deps only
-		this.node.expression.dynamic_dependencies().forEach((dependency: string) => {
-			all_dependencies.add(dependency);
-		});
-
-		if (all_dependencies.size) {
+		if (this.dependencies.size) {
 			const has_transitions = !!(this.block.has_intro_method || this.block.has_outro_method);
 
 			const for_loop_body = this.block.has_update_method
@@ -588,11 +592,7 @@ export default class EachBlockWrapper extends Wrapper {
 				${remove_old_blocks}
 			`;
 
-			block.chunks.update.push(b`
-				if (${block.renderer.dirty(Array.from(all_dependencies))}) {
-					${update}
-				}
-			`);
+			this.updates.push(update);
 		}
 
 		if (this.block.has_outros) {
diff --git a/test/runtime/samples/each-block-keyed-else/_config.js b/test/runtime/samples/each-block-keyed-else/_config.js
new file mode 100644
index 0000000000..a5bf722a80
--- /dev/null
+++ b/test/runtime/samples/each-block-keyed-else/_config.js
@@ -0,0 +1,37 @@
+export default {
+	props: {
+		animals: ['alpaca', 'baboon', 'capybara'],
+		foo: 'something else'
+	},
+
+	html: `
+		before
+		<p>alpaca</p>
+		<p>baboon</p>
+		<p>capybara</p>
+		after
+	`,
+
+	test({ assert, component, target }) {
+		component.animals = [];
+		assert.htmlEqual(target.innerHTML, `
+			before
+			<p>no animals, but rather something else</p>
+			after
+		`);
+
+		component.foo = 'something other';
+		assert.htmlEqual(target.innerHTML, `
+			before
+			<p>no animals, but rather something other</p>
+			after
+		`);
+
+		component.animals = ['wombat'];
+		assert.htmlEqual(target.innerHTML, `
+			before
+			<p>wombat</p>
+			after
+		`);
+	}
+};
diff --git a/test/runtime/samples/each-block-keyed-else/main.svelte b/test/runtime/samples/each-block-keyed-else/main.svelte
new file mode 100644
index 0000000000..2a82653ff1
--- /dev/null
+++ b/test/runtime/samples/each-block-keyed-else/main.svelte
@@ -0,0 +1,12 @@
+<script>
+	export let animals;
+	export let foo;
+</script>
+
+before
+{#each animals as animal (animal)}
+	<p>{animal}</p>
+{:else}
+	<p>no animals, but rather {foo}</p>
+{/each}
+after
diff --git a/test/runtime/samples/each-block-unkeyed-else-2/_config.js b/test/runtime/samples/each-block-unkeyed-else-2/_config.js
new file mode 100644
index 0000000000..a5bf722a80
--- /dev/null
+++ b/test/runtime/samples/each-block-unkeyed-else-2/_config.js
@@ -0,0 +1,37 @@
+export default {
+	props: {
+		animals: ['alpaca', 'baboon', 'capybara'],
+		foo: 'something else'
+	},
+
+	html: `
+		before
+		<p>alpaca</p>
+		<p>baboon</p>
+		<p>capybara</p>
+		after
+	`,
+
+	test({ assert, component, target }) {
+		component.animals = [];
+		assert.htmlEqual(target.innerHTML, `
+			before
+			<p>no animals, but rather something else</p>
+			after
+		`);
+
+		component.foo = 'something other';
+		assert.htmlEqual(target.innerHTML, `
+			before
+			<p>no animals, but rather something other</p>
+			after
+		`);
+
+		component.animals = ['wombat'];
+		assert.htmlEqual(target.innerHTML, `
+			before
+			<p>wombat</p>
+			after
+		`);
+	}
+};
diff --git a/test/runtime/samples/each-block-unkeyed-else-2/main.svelte b/test/runtime/samples/each-block-unkeyed-else-2/main.svelte
new file mode 100644
index 0000000000..3275cb1f83
--- /dev/null
+++ b/test/runtime/samples/each-block-unkeyed-else-2/main.svelte
@@ -0,0 +1,12 @@
+<script>
+	export let animals;
+	export let foo;
+</script>
+
+before
+{#each animals as animal}
+	<p>{animal}</p>
+{:else}
+	<p>no animals, but rather {foo}</p>
+{/each}
+after