diff --git a/src/compiler/compile/render_dom/wrappers/IfBlock.ts b/src/compiler/compile/render_dom/wrappers/IfBlock.ts index fe870df862..bc11e62028 100644 --- a/src/compiler/compile/render_dom/wrappers/IfBlock.ts +++ b/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( - (node as IfBlock).expression ? `create_if_block` : `create_else_block` - ) + name: parent.renderer.component.get_unique_name(is_else ? `create_else_block` : `create_if_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) { - ${this.branches - .map(({ condition, block }) => `${condition ? `if (${condition}) ` : ''}return ${block.name};`) - .join('\n')} + function ${select_block_type}(changed, ctx) { + ${this.branches.map(({ updater, condition, block }) => deindent` + ${updater} + ${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) { - ${this.branches - .map(({ condition }, i) => `${condition ? `if (${condition}) ` : ''}return ${i};`) - .join('\n')} + function ${select_block_type}(changed, ctx) { + ${this.branches.map(({ updater, condition }, i) => deindent` + ${updater} + ${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) { diff --git a/test/runtime/samples/if-block-conservative-update/_config.js b/test/runtime/samples/if-block-conservative-update/_config.js new file mode 100644 index 0000000000..1586ec6d0f --- /dev/null +++ b/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: `

potato

`, + + test({ assert, component, target }) { + assert.equal(count, 1); + + component.foo = 'soup'; + assert.equal(count, 1); + + assert.htmlEqual(target.innerHTML, `

soup

`); + } +} \ No newline at end of file diff --git a/test/runtime/samples/if-block-conservative-update/main.svelte b/test/runtime/samples/if-block-conservative-update/main.svelte new file mode 100644 index 0000000000..93a40b0e9e --- /dev/null +++ b/test/runtime/samples/if-block-conservative-update/main.svelte @@ -0,0 +1,8 @@ + + +{#if fn()} +

{foo}

+{/if} \ No newline at end of file diff --git a/test/runtime/samples/if-block-else-conservative-update/_config.js b/test/runtime/samples/if-block-else-conservative-update/_config.js new file mode 100644 index 0000000000..848a6080cf --- /dev/null +++ b/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: `

potato

`, + + 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, `

soup

`); + + component.foo = 'salad'; + assert.equal(count_a, 1); + assert.equal(count_b, 1); + + assert.htmlEqual(target.innerHTML, `

salad

`); + } +} \ No newline at end of file diff --git a/test/runtime/samples/if-block-else-conservative-update/main.svelte b/test/runtime/samples/if-block-else-conservative-update/main.svelte new file mode 100644 index 0000000000..abaab61cfd --- /dev/null +++ b/test/runtime/samples/if-block-else-conservative-update/main.svelte @@ -0,0 +1,11 @@ + + +{#if fn()} +

{foo}

+{:else if other_fn()} +

{foo.toUpperCase()}

+{/if} \ No newline at end of file