only reevaluate if block conditions if dependencies changed

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

@@ -7,7 +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';
+import { walk } from 'estree-walker';
 
 function is_else_if(node: ElseBlock) {
 	return (
@@ -18,8 +18,9 @@ function is_else_if(node: ElseBlock) {
 class IfBlockBranch extends Wrapper {
 	block: Block;
 	fragment: FragmentWrapper;
-	updater?: string;
+	dependencies?: string[];
-	condition: string;
+	condition?: string;
+	snippet?: string;
 	is_dynamic: boolean;
 
 	var = null;
@@ -40,17 +41,21 @@ class IfBlockBranch extends Wrapper {
 		if (expression) {
 			const dependencies = expression.dynamic_dependencies();
 
-			// TODO is this the right rule? or should only functions
+			// TODO is this the right rule? or should any non-reference count?
-			// be considered 'expensive'?
+			// const should_cache = !is_reference(expression.node, null) && dependencies.length > 0;
-			const should_cache = !is_reference(expression.node, null) && dependencies.length > 0;
+			let should_cache = false;
+			walk(expression.node, {
+				enter(node) {
+					if (node.type === 'CallExpression' || node.type === 'NewExpression') {
+						should_cache = true;
+					}
+				}
+			});
 
 			if (should_cache) {
-				const name = block.get_unique_name(`show_if`);
+				this.condition = block.get_unique_name(`show_if`);
-				const snippet = expression.render(block);
+				this.snippet = expression.render(block);
-
+				this.dependencies = dependencies;
-				block.add_variable(name, snippet);
-				this.updater = `if (${dependencies.map(d => `changed.${d}`)}) ${name} = ${snippet};`;
-				this.condition = name;
 			} else {
 				this.condition = expression.render(block);
 			}
@@ -177,6 +182,10 @@ export default class IfBlockWrapper extends Wrapper {
 		const detaching = (parent_node && parent_node !== '@_document.head') ? '' : 'detaching';
 
 		if (this.node.else) {
+			this.branches.forEach(branch => {
+				if (branch.snippet) block.add_variable(branch.condition);
+			});
+
 			if (has_outros) {
 				this.render_compound_with_outros(block, parent_node, parent_nodes, dynamic, vars, detaching);
 
@@ -233,15 +242,17 @@ export default class IfBlockWrapper extends Wrapper {
 		/* eslint-disable @typescript-eslint/indent,indent */
 		block.builders.init.add_block(deindent`
 			function ${select_block_type}(changed, ctx) {
-				${this.branches.map(({ updater, condition, block }) => deindent`
+				${this.branches.map(({ dependencies, condition, snippet, block }) => condition
-				${updater}
+				? deindent`
-				${condition ? `if (${condition}) ` : ''}return ${block.name};`)}
+				${dependencies && `if ((${condition} == null) || ${dependencies.map(n => `changed.${n}`).join(' || ')}) ${condition} = !!(${snippet})`}
+				if (${condition}) return ${block.name};`
+				: `return ${block.name};`)}
 			}
 		`);
 		/* eslint-enable @typescript-eslint/indent,indent */
 
 		block.builders.init.add_block(deindent`
-			var ${current_block_type} = ${select_block_type}(changed, ctx);
+			var ${current_block_type} = ${select_block_type}(null, ctx);
 			var ${name} = ${current_block_type_and}${current_block_type}(ctx);
 		`);
 
@@ -314,9 +325,11 @@ export default class IfBlockWrapper extends Wrapper {
 			var ${if_blocks} = [];
 
 			function ${select_block_type}(changed, ctx) {
-				${this.branches.map(({ updater, condition }, i) => deindent`
+				${this.branches.map(({ dependencies, condition, snippet }, i) => condition
-				${updater}
+				? deindent`
-				${condition ? `if (${condition}) ` : ''}return ${i};`)}
+				${dependencies && `if ((${condition} == null) || ${dependencies.map(n => `changed.${n}`).join(' || ')}) ${condition} = !!(${snippet})`}
+				if (${condition}) return ${String(i)};`
+				: `return ${i};`)}
 				${!has_else && `return -1;`}
 			}
 		`);
@@ -324,12 +337,12 @@ export default class IfBlockWrapper extends Wrapper {
 
 		if (has_else) {
 			block.builders.init.add_block(deindent`
-				${current_block_type_index} = ${select_block_type}(changed, ctx);
+				${current_block_type_index} = ${select_block_type}(null, 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}(changed, ctx))) {
+				if (~(${current_block_type_index} = ${select_block_type}(null, ctx))) {
 					${name} = ${if_blocks}[${current_block_type_index}] = ${if_block_creators}[${current_block_type_index}](ctx);
 				}
 			`);
@@ -415,6 +428,8 @@ export default class IfBlockWrapper extends Wrapper {
 	) {
 		const branch = this.branches[0];
 
+		if (branch.snippet) block.add_variable(branch.condition, branch.snippet);
+
 		block.builders.init.add_block(deindent`
 			var ${name} = (${branch.condition}) && ${branch.block.name}(ctx);
 		`);
@@ -451,8 +466,8 @@ export default class IfBlockWrapper extends Wrapper {
 				}
 			`;
 
-		if (branch.updater) {
+		if (branch.snippet) {
-			block.builders.update.add_block(branch.updater);
+			block.builders.update.add_block(`if (${branch.dependencies.map(n => `changed.${n}`).join(' || ')}) ${branch.condition} = ${branch.snippet}`);
 		}
 
 		// no `p()` here — we don't want to update outroing nodes,

test/js/samples/if-block-no-update/expected.js

@@ -57,12 +57,12 @@ function create_if_block(ctx) {
 function create_fragment(ctx) {
 	var if_block_anchor;
 
-	function select_block_type(ctx) {
+	function select_block_type(changed, ctx) {
 		if (ctx.foo) return create_if_block;
 		return create_else_block;
 	}
 
-	var current_block_type = select_block_type(ctx);
+	var current_block_type = select_block_type(null, ctx);
 	var if_block = current_block_type(ctx);
 
 	return {
@@ -77,7 +77,7 @@ function create_fragment(ctx) {
 		},
 
 		p(changed, ctx) {
-			if (current_block_type !== (current_block_type = select_block_type(ctx))) {
+			if (current_block_type !== (current_block_type = select_block_type(changed, ctx))) {
 				if_block.d(1);
 				if_block = current_block_type(ctx);
 				if (if_block) {

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

@@ -23,15 +23,15 @@ export default {
 
 		a = false;
 		component.foo = 'soup';
-		assert.equal(count_a, 1);
+		assert.equal(count_a, 2);
 		assert.equal(count_b, 1);
 
-		assert.htmlEqual(target.innerHTML, `<p>soup</p>`);
+		assert.htmlEqual(target.innerHTML, `<p>SOUP</p>`);
 
 		component.foo = 'salad';
-		assert.equal(count_a, 1);
+		assert.equal(count_a, 3);
 		assert.equal(count_b, 1);
 
-		assert.htmlEqual(target.innerHTML, `<p>salad</p>`);
+		assert.htmlEqual(target.innerHTML, `<p>SALAD</p>`);
 	}
 }

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

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