fix else-block update in keyed each-block (#4558)

Co-authored-by: Benjamin W. Broersma <bw@broersma.com>

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

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
+		if (this.dependencies.size) {
-		this.node.expression.dynamic_dependencies().forEach((dependency: string) => {
+			this.updates.push(b`
-			all_dependencies.add(dependency);
+				const ${this.vars.each_block_value} = ${snippet};
-		});
+				${this.renderer.options.dev && b`@validate_each_argument(${this.vars.each_block_value});`}
 
-		if (all_dependencies.size) {
+				${this.block.has_outros && b`@group_outros();`}
-			block.chunks.update.push(b`
+				${this.node.has_animation && b`for (let #i = 0; #i < ${view_length}; #i += 1) ${iterations}[#i].r();`}
-				if (${block.renderer.dirty(Array.from(all_dependencies))}) {
+				${this.renderer.options.dev && b`@validate_each_keys(#ctx, ${this.vars.each_block_value}, ${this.vars.get_each_context}, ${get_key});`}
-					const ${this.vars.each_block_value} = ${snippet};
+				${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.renderer.options.dev && b`@validate_each_argument(${this.vars.each_block_value});`}
+				${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
+		if (this.dependencies.size) {
-		this.node.expression.dynamic_dependencies().forEach((dependency: string) => {
-			all_dependencies.add(dependency);
-		});
-
-		if (all_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`
+			this.updates.push(update);
-				if (${block.renderer.dirty(Array.from(all_dependencies))}) {
-					${update}
-				}
-			`);
 		}
 
 		if (this.block.has_outros) {

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
+		`);
+	}
+};

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

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
+		`);
+	}
+};

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