Merge pull request #1498 from sveltejs/gh-1497

detach outroing blocks correctly

src/compile/nodes/AwaitBlock.ts

@@ -172,12 +172,13 @@ export default class AwaitBlock extends Node {
 		}
 
 		if (this.pending.block.hasOutroMethod && this.compiler.options.nestedTransitions) {
+			const countdown = block.getUniqueName('countdown');
 			block.builders.outro.addBlock(deindent`
-				#outrocallback = @callAfter(#outrocallback, 3);
+				const ${countdown} = @callAfter(#outrocallback, 3);
 				for (let #i = 0; #i < 3; #i += 1) {
 					const block = ${info}.blocks[#i];
-					if (block) block.o(#outrocallback);
-					else #outrocallback();
+					if (block) block.o(${countdown});
+					else ${countdown}();
 				}
 			`);
 		}

src/compile/nodes/EachBlock.ts

@@ -331,9 +331,10 @@ export default class EachBlock extends Node {
 		`);
 
 		if (this.compiler.options.nestedTransitions) {
+			const countdown = block.getUniqueName('countdown');
 			block.builders.outro.addBlock(deindent`
-				#outrocallback = @callAfter(#outrocallback, ${blocks}.length);
-				for (#i = 0; #i < ${blocks}.length; #i += 1) ${blocks}[#i].o(#outrocallback);
+				const ${countdown} = @callAfter(#outrocallback, ${blocks}.length);
+				for (#i = 0; #i < ${blocks}.length; #i += 1) ${blocks}[#i].o(${countdown});
 			`);
 		}
 
@@ -393,14 +394,16 @@ export default class EachBlock extends Node {
 			allDependencies.add(dependency);
 		});
 
-		const outro = this.block.hasOutros && block.getUniqueName('outro')
-		if (outro) {
+		const outroBlock = this.block.hasOutros && block.getUniqueName('outroBlock')
+		if (outroBlock) {
 			block.builders.init.addBlock(deindent`
-				function ${outro}(i, detach, fn) {
+				function ${outroBlock}(i, detach, fn) {
 					if (${iterations}[i]) {
 						${iterations}[i].o(() => {
-							${iterations}[i].d(detach);
-							if (detach) ${iterations}[i] = null;
+							if (detach) {
+								${iterations}[i].d(detach);
+								${iterations}[i] = null;
+							}
 							if (fn) fn();
 						});
 					}
@@ -447,7 +450,7 @@ export default class EachBlock extends Node {
 			if (this.block.hasOutros) {
 				destroy = deindent`
 					@transitionManager.groupOutros();
-					for (; #i < ${iterations}.length; #i += 1) ${outro}(#i, 1);
+					for (; #i < ${iterations}.length; #i += 1) ${outroBlock}(#i, 1);
 				`;
 			} else {
 				destroy = deindent`
@@ -473,10 +476,11 @@ export default class EachBlock extends Node {
 			`);
 		}
 
-		if (outro && this.compiler.options.nestedTransitions) {
+		if (outroBlock && this.compiler.options.nestedTransitions) {
+			const countdown = block.getUniqueName('countdown');
 			block.builders.outro.addBlock(deindent`
-				#outrocallback = @callAfter(#outrocallback, #i);
-				for (let #i = 0; #i < ${iterations}.length; #i += 1) ${outro}(#i, 0, #outrocallback);`
+				const ${countdown} = @callAfter(#outrocallback, ${iterations}.length);
+				for (let #i = 0; #i < ${iterations}.length; #i += 1) ${outroBlock}(#i, 0, ${countdown});`
 			);
 		}
 

src/compile/nodes/IfBlock.ts

@@ -147,9 +147,10 @@ export default class IfBlock extends Node {
 			this.buildSimple(block, parentNode, parentNodes, branches[0], dynamic, vars);
 
 			if (hasOutros && this.compiler.options.nestedTransitions) {
-				block.builders.outro.addLine(
-					`if (${name}) ${name}.o(#outrocallback);`
-				);
+				block.builders.outro.addBlock(deindent`
+					if (${name}) ${name}.o(#outrocallback);
+					else #outrocallback();
+				`);
 			}
 		}
 

test/runtime/samples/nested-transition-detach-each/_config.js

@@ -0,0 +1,41 @@
+export default {
+	data: {
+		visible: false,
+		rows: [1, 2, 3],
+		cols: ['a', 'b', 'c']
+	},
+
+	html: ``,
+
+	compileOptions: {
+		dev: true
+	},
+	nestedTransitions: true,
+	skipIntroByDefault: true,
+
+	test(assert, component, target, window, raf) {
+		component.set({ visible: true });
+		assert.htmlEqual(target.innerHTML, `
+			<div class="row">
+				<div class="cell">1, a</div>
+				<div class="cell">1, b</div>
+				<div class="cell">1, c</div>
+			</div>
+			<div class="row">
+				<div class="cell">2, a</div>
+				<div class="cell">2, b</div>
+				<div class="cell">2, c</div>
+			</div>
+			<div class="row">
+				<div class="cell">3, a</div>
+				<div class="cell">3, b</div>
+				<div class="cell">3, c</div>
+			</div>
+		`);
+
+		component.set({ visible: false });
+		raf.tick(0);
+		raf.tick(100);
+		assert.htmlEqual(target.innerHTML, ``);
+	},
+};

test/runtime/samples/nested-transition-detach-each/main.html

@@ -0,0 +1,22 @@
+{#if visible}
+	{#each rows as row}
+		<div out:foo class="row">
+			{#each cols as col}
+				<div out:foo class="cell">{row}, {col}</div>
+			{/each}
+		</div>
+	{/each}
+{/if}
+
+<script>
+	export default {
+		transitions: {
+			foo(node) {
+				return {
+					duration: 100,
+					tick: t => node.foo = t
+				};
+			}
+		}
+	};
+</script>

test/runtime/samples/nested-transition-detach-if-false/Folder.html

@@ -0,0 +1,44 @@
+<li>
+	<span>{dir}</span>
+
+	{#if open}
+		<ul>
+			{#each items as item (item.filename)}
+				{#if item.isDir}
+					<svelte:self dir={item.filename}/>
+				{:else}
+					<li>{item.filename}</li>
+				{/if}
+			{/each}
+		</ul>
+	{/if}
+</li>
+
+<script>
+	export default {
+		data() {
+			return {
+				items: [],
+				open: true
+			};
+		},
+
+		computed: {
+			items: ({ dir }) => {
+				return dir === 'a'
+					? [
+						{
+							filename: 'a/b',
+							isDir: true
+						}
+					]
+					: [
+						{
+							filename: 'a/b/c',
+							isDir: false
+						}
+					];
+			}
+		}
+	};
+</script>

test/runtime/samples/nested-transition-detach-if-false/_config.js

@@ -0,0 +1,26 @@
+export default {
+	html: `
+		<li>
+			<span>a</span>
+			<ul>
+				<li>
+					<span>a/b</span>
+					<ul>
+						<li>a/b/c</li>
+					</ul>
+				</li>
+			</ul>
+		</li>
+	`,
+
+	nestedTransitions: true,
+
+	test(assert, component, target, window, raf) {
+		component.refs.folder.set({ open: false });
+		assert.htmlEqual(target.innerHTML, `
+			<li>
+				<span>a</span>
+			</li>
+		`);
+	},
+};

test/runtime/samples/nested-transition-detach-if-false/main.html

@@ -0,0 +1,9 @@
+<Folder ref:folder dir="a"/>
+
+<script>
+	export default {
+		components: {
+			Folder: './Folder.html'
+		}
+	};
+</script>