Merge pull request #3438 from sveltejs/gh-2355

more conservative if block updates
pull/3451/head
Rich Harris 5 years ago committed by GitHub
commit 38001cec33
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23

@ -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) {

@ -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) {

@ -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) {

@ -0,0 +1,16 @@
import { sleep } from './sleep.js';
export default {
html: `
<p>loading...</p>
`,
test({ assert, component, target }) {
return sleep(50).then(() => {
assert.htmlEqual(target.innerHTML, `
<p>the answer is 42</p>
<p>count: 1</p>
`);
});
}
};

@ -0,0 +1,19 @@
<script>
import { sleep } from './sleep.js';
let count = 0;
const get_promise = () => {
return sleep(10).then(() => {
count += 1;
return 42;
});
};
</script>
{#await get_promise()}
<p>loading...</p>
{:then value}
<p>the answer is {value}</p>
<p>count: {count}</p>
{/await}

@ -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);
});

@ -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>`);
}
}

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

@ -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, 2);
assert.equal(count_b, 1);
assert.htmlEqual(target.innerHTML, `<p>SOUP</p>`);
component.foo = 'salad';
assert.equal(count_a, 3);
assert.equal(count_b, 1);
assert.htmlEqual(target.innerHTML, `<p>SALAD</p>`);
}
}

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