more conservative if block updates

src/compiler/compile/render_dom/wrappers/IfBlock.ts

@@ -7,6 +7,7 @@ import create_debugging_comment from './shared/create_debugging_comment';
 import ElseBlock from '../../nodes/ElseBlock';
 import FragmentWrapper from './Fragment';
 import deindent from '../../utils/deindent';
+import is_reference from 'is-reference';
 
 function is_else_if(node: ElseBlock) {
 	return (
@@ -17,6 +18,7 @@ function is_else_if(node: ElseBlock) {
 class IfBlockBranch extends Wrapper {
 	block: Block;
 	fragment: FragmentWrapper;
+	updater?: string;
 	condition: string;
 	is_dynamic: boolean;
 
@@ -32,13 +34,31 @@ class IfBlockBranch extends Wrapper {
 	) {
 		super(renderer, block, parent, node);
 
-		this.condition = (node as IfBlock).expression && (node as IfBlock).expression.render(block);
+		const { expression } = (node as IfBlock);
+		const is_else = !expression;
+
+		if (expression) {
+			const dependencies = expression.dynamic_dependencies();
+
+			// TODO is this the right rule? or should only functions
+			// be considered 'expensive'?
+			const should_cache = !is_reference(expression.node, null) && dependencies.length > 0;
+
+			if (should_cache) {
+				const name = block.get_unique_name(`show_if`);
+				const snippet = expression.render(block);
+
+				block.add_variable(name, snippet);
+				this.updater = `if (${dependencies.map(d => `changed.${d}`)}) ${name} = ${snippet};`;
+				this.condition = name;
+			} else {
+				this.condition = expression.render(block);
+			}
+		}
 
 		this.block = block.child({
 			comment: create_debugging_comment(node, parent.renderer.component),
-			name: parent.renderer.component.get_unique_name(
+			name: parent.renderer.component.get_unique_name(is_else ? `create_else_block` : `create_if_block`)
-				(node as IfBlock).expression ? `create_if_block` : `create_else_block`
-			)
 		});
 
 		this.fragment = new FragmentWrapper(renderer, this.block, node.children, parent, strip_whitespace, next_sibling);
@@ -212,16 +232,16 @@ export default class IfBlockWrapper extends Wrapper {
 
 		/* eslint-disable @typescript-eslint/indent,indent */
 		block.builders.init.add_block(deindent`
-			function ${select_block_type}(ctx) {
+			function ${select_block_type}(changed, ctx) {
-				${this.branches
+				${this.branches.map(({ updater, condition, block }) => deindent`
-					.map(({ condition, block }) => `${condition ? `if (${condition}) ` : ''}return ${block.name};`)
+				${updater}
-					.join('\n')}
+				${condition ? `if (${condition}) ` : ''}return ${block.name};`)}
 			}
 		`);
 		/* eslint-enable @typescript-eslint/indent,indent */
 
 		block.builders.init.add_block(deindent`
-			var ${current_block_type} = ${select_block_type}(ctx);
+			var ${current_block_type} = ${select_block_type}(changed, ctx);
 			var ${name} = ${current_block_type_and}${current_block_type}(ctx);
 		`);
 
@@ -245,7 +265,7 @@ export default class IfBlockWrapper extends Wrapper {
 
 		if (dynamic) {
 			block.builders.update.add_block(deindent`
-				if (${current_block_type} === (${current_block_type} = ${select_block_type}(ctx)) && ${name}) {
+				if (${current_block_type} === (${current_block_type} = ${select_block_type}(changed, ctx)) && ${name}) {
 					${name}.p(changed, ctx);
 				} else {
 					${change_block}
@@ -253,7 +273,7 @@ export default class IfBlockWrapper extends Wrapper {
 			`);
 		} else {
 			block.builders.update.add_block(deindent`
-				if (${current_block_type} !== (${current_block_type} = ${select_block_type}(ctx))) {
+				if (${current_block_type} !== (${current_block_type} = ${select_block_type}(changed, ctx))) {
 					${change_block}
 				}
 			`);
@@ -293,10 +313,10 @@ export default class IfBlockWrapper extends Wrapper {
 
 			var ${if_blocks} = [];
 
-			function ${select_block_type}(ctx) {
+			function ${select_block_type}(changed, ctx) {
-				${this.branches
+				${this.branches.map(({ updater, condition }, i) => deindent`
-					.map(({ condition }, i) => `${condition ? `if (${condition}) ` : ''}return ${i};`)
+				${updater}
-					.join('\n')}
+				${condition ? `if (${condition}) ` : ''}return ${i};`)}
 				${!has_else && `return -1;`}
 			}
 		`);
@@ -304,12 +324,12 @@ export default class IfBlockWrapper extends Wrapper {
 
 		if (has_else) {
 			block.builders.init.add_block(deindent`
-				${current_block_type_index} = ${select_block_type}(ctx);
+				${current_block_type_index} = ${select_block_type}(changed, ctx);
 				${name} = ${if_blocks}[${current_block_type_index}] = ${if_block_creators}[${current_block_type_index}](ctx);
 			`);
 		} else {
 			block.builders.init.add_block(deindent`
-				if (~(${current_block_type_index} = ${select_block_type}(ctx))) {
+				if (~(${current_block_type_index} = ${select_block_type}(changed, ctx))) {
 					${name} = ${if_blocks}[${current_block_type_index}] = ${if_block_creators}[${current_block_type_index}](ctx);
 				}
 			`);
@@ -363,7 +383,7 @@ export default class IfBlockWrapper extends Wrapper {
 		if (dynamic) {
 			block.builders.update.add_block(deindent`
 				var ${previous_block_index} = ${current_block_type_index};
-				${current_block_type_index} = ${select_block_type}(ctx);
+				${current_block_type_index} = ${select_block_type}(changed, ctx);
 				if (${current_block_type_index} === ${previous_block_index}) {
 					${if_current_block_type_index}${if_blocks}[${current_block_type_index}].p(changed, ctx);
 				} else {
@@ -373,7 +393,7 @@ export default class IfBlockWrapper extends Wrapper {
 		} else {
 			block.builders.update.add_block(deindent`
 				var ${previous_block_index} = ${current_block_type_index};
-				${current_block_type_index} = ${select_block_type}(ctx);
+				${current_block_type_index} = ${select_block_type}(changed, ctx);
 				if (${current_block_type_index} !== ${previous_block_index}) {
 					${change_block}
 				}
@@ -431,6 +451,10 @@ export default class IfBlockWrapper extends Wrapper {
 				}
 			`;
 
+		if (branch.updater) {
+			block.builders.update.add_block(branch.updater);
+		}
+
 		// no `p()` here — we don't want to update outroing nodes,
 		// as that will typically result in glitching
 		if (branch.block.has_outro_method) {

test/runtime/samples/if-block-conservative-update/_config.js

@@ -0,0 +1,22 @@
+let count = 0;
+
+export default {
+	props: {
+		foo: 'potato',
+		fn: () => {
+			count += 1;
+			return true;
+		}
+	},
+
+	html: `<p>potato</p>`,
+
+	test({ assert, component, target }) {
+		assert.equal(count, 1);
+
+		component.foo = 'soup';
+		assert.equal(count, 1);
+
+		assert.htmlEqual(target.innerHTML, `<p>soup</p>`);
+	}
+}

test/runtime/samples/if-block-conservative-update/main.svelte

@@ -0,0 +1,8 @@
+<script>
+	export let fn;
+	export let foo;
+</script>
+
+{#if fn()}
+	<p>{foo}</p>
+{/if}

test/runtime/samples/if-block-else-conservative-update/_config.js

@@ -0,0 +1,37 @@
+let a = true;
+let count_a = 0;
+let count_b = 0;
+
+export default {
+	props: {
+		foo: 'potato',
+		fn: () => {
+			count_a += 1;
+			return a;
+		},
+		other_fn: () => {
+			count_b += 1;
+			return true;
+		}
+	},
+
+	html: `<p>potato</p>`,
+
+	test({ assert, component, target }) {
+		assert.equal(count_a, 1);
+		assert.equal(count_b, 0);
+
+		a = false;
+		component.foo = 'soup';
+		assert.equal(count_a, 1);
+		assert.equal(count_b, 1);
+
+		assert.htmlEqual(target.innerHTML, `<p>soup</p>`);
+
+		component.foo = 'salad';
+		assert.equal(count_a, 1);
+		assert.equal(count_b, 1);
+
+		assert.htmlEqual(target.innerHTML, `<p>salad</p>`);
+	}
+}

test/runtime/samples/if-block-else-conservative-update/main.svelte

@@ -0,0 +1,11 @@
+<script>
+	export let fn;
+	export let other_fn;
+	export let foo;
+</script>
+
+{#if fn()}
+	<p>{foo}</p>
+{:else if other_fn()}
+	<p>{foo.toUpperCase()}</p>
+{/if}