Merge pull request #1529 from sveltejs/gh-1527

avoid unnecessary remounts

Author: Rich-Harris

src/compile/dom/Block.ts

@@ -142,6 +142,10 @@ export default class Block {
 	}
 
 	addVariable(name: string, init?: string) {
+		if (name[0] === '#') {
+			name = this.alias(name.slice(1));
+		}
+
 		if (this.variables.has(name) && this.variables.get(name) !== init) {
 			throw new Error(
 				`Variable '${name}' already initialised with a different value`
@@ -166,18 +170,16 @@ export default class Block {
 	toString() {
 		const { dev } = this.compiler.options;
 
-		let introing;
-		const hasIntros = !this.builders.intro.isEmpty();
-		if (hasIntros) {
-			introing = this.getUniqueName('introing');
-			this.addVariable(introing);
-		}
+		if (this.hasIntroMethod || this.hasOutroMethod) {
+			this.addVariable('#current');
 
-		let outroing;
-		const hasOutros = !this.builders.outro.isEmpty();
-		if (hasOutros) {
-			outroing = this.alias('outroing');
-			this.addVariable(outroing);
+			if (!this.builders.mount.isEmpty()) {
+				this.builders.mount.addLine(`#current = true;`);
+			}
+
+			if (!this.builders.outro.isEmpty()) {
+				this.builders.outro.addLine(`#current = false;`);
+			}
 		}
 
 		if (this.autofocus) {
@@ -275,46 +277,30 @@ export default class Block {
 		}
 
 		if (this.hasIntroMethod || this.hasOutroMethod) {
-			if (hasIntros) {
+			if (this.builders.mount.isEmpty()) {
+				properties.addBlock(`i: @noop,`);
+			} else {
 				properties.addBlock(deindent`
 					${dev ? 'i: function intro' : 'i'}(#target, anchor) {
-						if (${introing}) return;
-						${introing} = true;
-						${hasOutros && `${outroing} = false;`}
-
+						if (#current) return;
 						${this.builders.intro}
-
 						this.m(#target, anchor);
 					},
 				`);
-			} else {
-				if (this.builders.mount.isEmpty()) {
-					properties.addBlock(`i: @noop,`);
-				} else {
-					properties.addBlock(deindent`
-						${dev ? 'i: function intro' : 'i'}(#target, anchor) {
-							this.m(#target, anchor);
-						},
-					`);
-				}
 			}
 
-			if (hasOutros) {
+			if (this.builders.outro.isEmpty()) {
+				properties.addBlock(`o: @run,`);
+			} else {
 				properties.addBlock(deindent`
 					${dev ? 'o: function outro' : 'o'}(#outrocallback) {
-						if (${outroing}) return;
-						${outroing} = true;
-						${hasIntros && `${introing} = false;`}
+						if (!#current) return;
 
 						${this.outros > 1 && `#outrocallback = @callAfter(#outrocallback, ${this.outros});`}
 
 						${this.builders.outro}
 					},
 				`);
-			} else {
-				properties.addBlock(deindent`
-					o: @run,
-				`);
 			}
 		}
 

src/compile/nodes/Attribute.ts

@@ -232,7 +232,7 @@ export default class Attribute extends Node {
 			if (this.dependencies.size || isSelectValueAttribute) {
 				const dependencies = Array.from(this.dependencies);
 				const changedCheck = (
-					( block.hasOutros ? `#outroing || ` : '' ) +
+					(block.hasOutros ? `!#current || ` : '') +
 					dependencies.map(dependency => `changed.${dependency}`).join(' || ')
 				);
 
@@ -308,7 +308,7 @@ export default class Attribute extends Node {
 				if (propDependencies.size) {
 					const dependencies = Array.from(propDependencies);
 					const condition = (
-						(block.hasOutros ? `#outroing || ` : '') +
+						(block.hasOutros ? `!#current || ` : '') +
 						dependencies.map(dependency => `changed.${dependency}`).join(' || ')
 					);
 

src/compile/nodes/Title.ts

@@ -81,7 +81,7 @@ export default class Title extends Node {
 			if (allDependencies.size) {
 				const dependencies = Array.from(allDependencies);
 				const changedCheck = (
-					( block.hasOutros ? `#outroing || ` : '' ) +
+					( block.hasOutros ? `!#current || ` : '' ) +
 					dependencies.map(dependency => `changed.${dependency}`).join(' || ')
 				);
 

src/compile/nodes/shared/Tag.ts

@@ -35,7 +35,7 @@ export default class Tag extends Node {
 
 		if (dependencies.size) {
 			const changedCheck = (
-				(block.hasOutros ? `#outroing || ` : '') +
+				(block.hasOutros ? `!#current || ` : '') +
 				[...dependencies].map((dependency: string) => `changed.${dependency}`).join(' || ')
 			);
 

test/runtime/samples/nested-transition-if-block-not-remounted/Span.html

@@ -0,0 +1 @@
+<span><slot></slot></span>

test/runtime/samples/nested-transition-if-block-not-remounted/_config.js

@@ -0,0 +1,26 @@
+export default {
+	data: {
+		x: true,
+		value: 'one'
+	},
+
+	html: `
+		<div>
+			<input>
+			<span>x</span>
+		</div>
+	`,
+
+	nestedTransitions: true,
+
+	test(assert, component, target, window, raf) {
+		const div = target.querySelector('div');
+		const { appendChild, insertBefore } = div;
+
+		div.appendChild = div.insertBefore = () => {
+			throw new Error('DOM was mutated');
+		};
+
+		component.set({ value: 'two' });
+	},
+};

test/runtime/samples/nested-transition-if-block-not-remounted/main.html

@@ -0,0 +1,14 @@
+<div>
+	{#if x}
+		<input on:input="set({ value: this.value })">
+		<Span>x</Span>
+	{/if}
+</div>
+
+<script>
+	export default {
+		components: {
+			Span: './Span.html'
+		}
+	};
+</script>