diff --git a/src/compiler/compile/render_dom/wrappers/AwaitBlock.ts b/src/compiler/compile/render_dom/wrappers/AwaitBlock.ts index ff835ddef6..354143feab 100644 --- a/src/compiler/compile/render_dom/wrappers/AwaitBlock.ts +++ b/src/compiler/compile/render_dom/wrappers/AwaitBlock.ts @@ -188,29 +188,35 @@ export default class AwaitBlockWrapper extends Wrapper { conditions.push( `(${dependencies.map(dep => `'${dep}' in changed`).join(' || ')})` ); - } - conditions.push( - `${promise} !== (${promise} = ${snippet})`, - `@handle_promise(${promise}, ${info})` - ); + conditions.push( + `${promise} !== (${promise} = ${snippet})`, + `@handle_promise(${promise}, ${info})` + ); - block.builders.update.add_line( - `${info}.ctx = ctx;` - ); + block.builders.update.add_line( + `${info}.ctx = ctx;` + ); - if (this.pending.block.has_update_method) { - block.builders.update.add_block(deindent` - if (${conditions.join(' && ')}) { - // nothing - } else { - ${info}.block.p(changed, @assign(@assign({}, ctx), ${info}.resolved)); - } - `); + if (this.pending.block.has_update_method) { + block.builders.update.add_block(deindent` + if (${conditions.join(' && ')}) { + // nothing + } else { + ${info}.block.p(changed, @assign(@assign({}, ctx), ${info}.resolved)); + } + `); + } else { + block.builders.update.add_block(deindent` + ${conditions.join(' && ')} + `); + } } else { - block.builders.update.add_block(deindent` - ${conditions.join(' && ')} - `); + if (this.pending.block.has_update_method) { + block.builders.update.add_block(deindent` + ${info}.block.p(changed, @assign(@assign({}, ctx), ${info}.resolved)); + `); + } } if (this.pending.block.has_outro_method) { diff --git a/src/compiler/compile/render_dom/wrappers/IfBlock.ts b/src/compiler/compile/render_dom/wrappers/IfBlock.ts index fe870df862..58b6513311 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 { walk } from 'estree-walker'; function is_else_if(node: ElseBlock) { return ( @@ -17,7 +18,9 @@ function is_else_if(node: ElseBlock) { class IfBlockBranch extends Wrapper { block: Block; fragment: FragmentWrapper; - condition: string; + dependencies?: string[]; + condition?: string; + snippet?: string; is_dynamic: boolean; var = null; @@ -32,13 +35,35 @@ 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 any non-reference count? + // 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) { + this.condition = block.get_unique_name(`show_if`); + this.snippet = expression.render(block); + this.dependencies = dependencies; + } 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); @@ -157,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); @@ -212,16 +241,18 @@ 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(({ dependencies, condition, snippet, block }) => condition + ? deindent` + ${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}(ctx); + var ${current_block_type} = ${select_block_type}(null, ctx); var ${name} = ${current_block_type_and}${current_block_type}(ctx); `); @@ -245,7 +276,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 +284,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 +324,12 @@ 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(({ dependencies, condition, snippet }, i) => condition + ? deindent` + ${dependencies && `if ((${condition} == null) || ${dependencies.map(n => `changed.${n}`).join(' || ')}) ${condition} = !!(${snippet})`} + if (${condition}) return ${String(i)};` + : `return ${i};`)} ${!has_else && `return -1;`} } `); @@ -304,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}(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}(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); } `); @@ -363,7 +396,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 +406,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} } @@ -395,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); `); @@ -431,6 +466,10 @@ export default class IfBlockWrapper extends Wrapper { } `; + if (branch.snippet) { + 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, // as that will typically result in glitching if (branch.block.has_outro_method) { diff --git a/test/js/samples/if-block-no-update/expected.js b/test/js/samples/if-block-no-update/expected.js index 0dca40a81e..5cb29e8593 100644 --- a/test/js/samples/if-block-no-update/expected.js +++ b/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) { diff --git a/test/runtime/samples/await-conservative-update/_config.js b/test/runtime/samples/await-conservative-update/_config.js new file mode 100644 index 0000000000..4e81ff8e37 --- /dev/null +++ b/test/runtime/samples/await-conservative-update/_config.js @@ -0,0 +1,16 @@ +import { sleep } from './sleep.js'; + +export default { + html: ` +

loading...

+ `, + + test({ assert, component, target }) { + return sleep(50).then(() => { + assert.htmlEqual(target.innerHTML, ` +

the answer is 42

+

count: 1

+ `); + }); + } +}; \ No newline at end of file diff --git a/test/runtime/samples/await-conservative-update/main.svelte b/test/runtime/samples/await-conservative-update/main.svelte new file mode 100644 index 0000000000..91c9a6c4c2 --- /dev/null +++ b/test/runtime/samples/await-conservative-update/main.svelte @@ -0,0 +1,19 @@ + + +{#await get_promise()} +

loading...

+{:then value} +

the answer is {value}

+

count: {count}

+{/await} \ No newline at end of file diff --git a/test/runtime/samples/await-conservative-update/sleep.js b/test/runtime/samples/await-conservative-update/sleep.js new file mode 100644 index 0000000000..994f85f38a --- /dev/null +++ b/test/runtime/samples/await-conservative-update/sleep.js @@ -0,0 +1,11 @@ +export let stopped = false; + +export const stop = () => stopped = true; + +export const sleep = ms => new Promise(f => { + if (stopped) return; + setTimeout(() => { + if (stopped) return; + f(); + }, ms); +}); \ No newline at end of file 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..6d002afb55 --- /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, 2); + assert.equal(count_b, 1); + + assert.htmlEqual(target.innerHTML, `

SOUP

`); + + component.foo = 'salad'; + assert.equal(count_a, 3); + 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..ef9da266b9 --- /dev/null +++ b/test/runtime/samples/if-block-else-conservative-update/main.svelte @@ -0,0 +1,11 @@ + + +{#if fn(foo)} +

{foo}

+{:else if other_fn()} +

{foo.toUpperCase()}

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