From 04d762d2f9503ac575eb9efaead91be8ab0d44ab Mon Sep 17 00:00:00 2001 From: Almaz Date: Fri, 23 Aug 2019 00:47:19 +0200 Subject: [PATCH 1/5] Fixed complex {#if} behaviour --- .../compile/render_dom/wrappers/IfBlock.ts | 2 +- test/js/samples/if-block-complex/expected.js | 93 +++++++++++++++++++ test/js/samples/if-block-complex/input.svelte | 9 ++ 3 files changed, 103 insertions(+), 1 deletion(-) create mode 100644 test/js/samples/if-block-complex/expected.js create mode 100644 test/js/samples/if-block-complex/input.svelte diff --git a/src/compiler/compile/render_dom/wrappers/IfBlock.ts b/src/compiler/compile/render_dom/wrappers/IfBlock.ts index 58b651331100..fb364a534301 100644 --- a/src/compiler/compile/render_dom/wrappers/IfBlock.ts +++ b/src/compiler/compile/render_dom/wrappers/IfBlock.ts @@ -466,7 +466,7 @@ export default class IfBlockWrapper extends Wrapper { } `; - if (branch.snippet) { + if (branch.snippet && branch.dependencies.length) { block.builders.update.add_block(`if (${branch.dependencies.map(n => `changed.${n}`).join(' || ')}) ${branch.condition} = ${branch.snippet}`); } diff --git a/test/js/samples/if-block-complex/expected.js b/test/js/samples/if-block-complex/expected.js new file mode 100644 index 000000000000..8dffddef33e3 --- /dev/null +++ b/test/js/samples/if-block-complex/expected.js @@ -0,0 +1,93 @@ +/* generated by Svelte vX.Y.Z */ +import { + SvelteComponent, + attr, + detach, + element, + empty, + init, + insert, + noop, + safe_not_equal +} from "svelte/internal"; + +// (7:0) {#if (item.divider && item.divider.includes(1))} +function create_if_block(ctx) { + var div; + + return { + c() { + div = element("div"); + attr(div, "class", "divider"); + }, + + m(target, anchor) { + insert(target, div, anchor); + }, + + d(detaching) { + if (detaching) { + detach(div); + } + } + }; +} + +function create_fragment(ctx) { + var show_if = (ctx.item.divider && ctx.item.divider.includes(1)), if_block_anchor; + + var if_block = (show_if) && create_if_block(ctx); + + return { + c() { + if (if_block) if_block.c(); + if_block_anchor = empty(); + }, + + m(target, anchor) { + if (if_block) if_block.m(target, anchor); + insert(target, if_block_anchor, anchor); + }, + + p(changed, ctx) { + if (show_if) { + if (!if_block) { + if_block = create_if_block(ctx); + if_block.c(); + if_block.m(if_block_anchor.parentNode, if_block_anchor); + } + } else if (if_block) { + if_block.d(1); + if_block = null; + } + }, + + i: noop, + o: noop, + + d(detaching) { + if (if_block) if_block.d(detaching); + + if (detaching) { + detach(if_block_anchor); + } + } + }; +} + +function instance($$self) { + let item = { + divider: [1] + } + + return { item }; +} + +class Component extends SvelteComponent { + constructor(options) { + super(); + init(this, options, instance, create_fragment, safe_not_equal, []); + } +} + +export default Component; diff --git a/test/js/samples/if-block-complex/input.svelte b/test/js/samples/if-block-complex/input.svelte new file mode 100644 index 000000000000..8c1143596b8d --- /dev/null +++ b/test/js/samples/if-block-complex/input.svelte @@ -0,0 +1,9 @@ + + +{#if (item.divider && item.divider.includes(1))} +
+{/if} From 334323fb8b1ef94b5a26804a3dde13906fcde554 Mon Sep 17 00:00:00 2001 From: Rich Harris Date: Fri, 30 Aug 2019 12:02:20 -0400 Subject: [PATCH 2/5] dont generate code for non-updating if block - fixes #3447 --- .../compile/render_dom/wrappers/IfBlock.ts | 98 +++++++++---------- test/js/samples/if-block-complex/expected.js | 14 +-- 2 files changed, 50 insertions(+), 62 deletions(-) diff --git a/src/compiler/compile/render_dom/wrappers/IfBlock.ts b/src/compiler/compile/render_dom/wrappers/IfBlock.ts index fb364a534301..f12929ffb142 100644 --- a/src/compiler/compile/render_dom/wrappers/IfBlock.ts +++ b/src/compiler/compile/render_dom/wrappers/IfBlock.ts @@ -441,58 +441,58 @@ export default class IfBlockWrapper extends Wrapper { `if (${name}) ${name}.m(${initial_mount_node}, ${anchor_node});` ); - const update_mount_node = this.get_update_mount_node(anchor); - - const enter = dynamic - ? deindent` - if (${name}) { - ${name}.p(changed, ctx); - ${has_transitions && `@transition_in(${name}, 1);`} - } else { - ${name} = ${branch.block.name}(ctx); - ${name}.c(); - ${has_transitions && `@transition_in(${name}, 1);`} - ${name}.m(${update_mount_node}, ${anchor}); - } - ` - : deindent` - if (!${name}) { - ${name} = ${branch.block.name}(ctx); - ${name}.c(); - ${has_transitions && `@transition_in(${name}, 1);`} - ${name}.m(${update_mount_node}, ${anchor}); - ${has_transitions && `} else { - @transition_in(${name}, 1);`} - } - `; + if (!branch.dependencies || branch.dependencies.length > 0) { + const update_mount_node = this.get_update_mount_node(anchor); - if (branch.snippet && branch.dependencies.length) { - block.builders.update.add_block(`if (${branch.dependencies.map(n => `changed.${n}`).join(' || ')}) ${branch.condition} = ${branch.snippet}`); - } + const enter = dynamic + ? deindent` + if (${name}) { + ${name}.p(changed, ctx); + ${has_transitions && `@transition_in(${name}, 1);`} + } else { + ${name} = ${branch.block.name}(ctx); + ${name}.c(); + ${has_transitions && `@transition_in(${name}, 1);`} + ${name}.m(${update_mount_node}, ${anchor}); + } + ` + : deindent` + if (!${name}) { + ${name} = ${branch.block.name}(ctx); + ${name}.c(); + ${has_transitions && `@transition_in(${name}, 1);`} + ${name}.m(${update_mount_node}, ${anchor}); + } ${has_transitions && `else @transition_in(${name}, 1);`} + `; + + 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) { - block.builders.update.add_block(deindent` - if (${branch.condition}) { - ${enter} - } else if (${name}) { - @group_outros(); - @transition_out(${name}, 1, 1, () => { + // no `p()` here — we don't want to update outroing nodes, + // as that will typically result in glitching + if (branch.block.has_outro_method) { + block.builders.update.add_block(deindent` + if (${branch.condition}) { + ${enter} + } else if (${name}) { + @group_outros(); + @transition_out(${name}, 1, 1, () => { + ${name} = null; + }); + @check_outros(); + } + `); + } else { + block.builders.update.add_block(deindent` + if (${branch.condition}) { + ${enter} + } else if (${name}) { + ${name}.d(1); ${name} = null; - }); - @check_outros(); - } - `); - } else { - block.builders.update.add_block(deindent` - if (${branch.condition}) { - ${enter} - } else if (${name}) { - ${name}.d(1); - ${name} = null; - } - `); + } + `); + } } block.builders.destroy.add_line(`${if_name}${name}.d(${detaching});`); diff --git a/test/js/samples/if-block-complex/expected.js b/test/js/samples/if-block-complex/expected.js index 8dffddef33e3..f0da492e3caa 100644 --- a/test/js/samples/if-block-complex/expected.js +++ b/test/js/samples/if-block-complex/expected.js @@ -49,19 +49,7 @@ function create_fragment(ctx) { insert(target, if_block_anchor, anchor); }, - p(changed, ctx) { - if (show_if) { - if (!if_block) { - if_block = create_if_block(ctx); - if_block.c(); - if_block.m(if_block_anchor.parentNode, if_block_anchor); - } - } else if (if_block) { - if_block.d(1); - if_block = null; - } - }, - + p: noop, i: noop, o: noop, From 6665a52bad7072f2b758875a36173aaee8a52e47 Mon Sep 17 00:00:00 2001 From: Rich Harris Date: Fri, 30 Aug 2019 13:35:05 -0400 Subject: [PATCH 3/5] update simple if-blocks with complex but static conditions --- src/compiler/compile/render_dom/wrappers/IfBlock.ts | 4 ++++ .../if-block-static-with-dynamic-contents/_config.js | 12 ++++++++++++ .../main.svelte | 9 +++++++++ 3 files changed, 25 insertions(+) create mode 100644 test/runtime/samples/if-block-static-with-dynamic-contents/_config.js create mode 100644 test/runtime/samples/if-block-static-with-dynamic-contents/main.svelte diff --git a/src/compiler/compile/render_dom/wrappers/IfBlock.ts b/src/compiler/compile/render_dom/wrappers/IfBlock.ts index f12929ffb142..64b90ecaf3ce 100644 --- a/src/compiler/compile/render_dom/wrappers/IfBlock.ts +++ b/src/compiler/compile/render_dom/wrappers/IfBlock.ts @@ -493,6 +493,10 @@ export default class IfBlockWrapper extends Wrapper { } `); } + } else if (dynamic) { + block.builders.update.add_block( + `if (${branch.condition}) ${name}.p(changed, ctx);` + ); } block.builders.destroy.add_line(`${if_name}${name}.d(${detaching});`); diff --git a/test/runtime/samples/if-block-static-with-dynamic-contents/_config.js b/test/runtime/samples/if-block-static-with-dynamic-contents/_config.js new file mode 100644 index 000000000000..c5c99c5305b8 --- /dev/null +++ b/test/runtime/samples/if-block-static-with-dynamic-contents/_config.js @@ -0,0 +1,12 @@ +export default { + props: { + foo: 42 + }, + + html: '

42

', + + test({ assert, component, target }) { + component.foo = 43; + assert.htmlEqual(target.innerHTML, '

43

'); + } +}; diff --git a/test/runtime/samples/if-block-static-with-dynamic-contents/main.svelte b/test/runtime/samples/if-block-static-with-dynamic-contents/main.svelte new file mode 100644 index 000000000000..57484f252f51 --- /dev/null +++ b/test/runtime/samples/if-block-static-with-dynamic-contents/main.svelte @@ -0,0 +1,9 @@ + + +{#if show()} +

{foo}

+{/if} From bb5dc8b25f0d84a6c005b1cad7d0169b4026fffa Mon Sep 17 00:00:00 2001 From: Rich Harris Date: Fri, 30 Aug 2019 13:48:59 -0400 Subject: [PATCH 4/5] update tests --- test/js/samples/if-block-complex/expected.js | 2 +- test/js/samples/transition-local/expected.js | 4 +--- test/js/samples/transition-repeated-outro/expected.js | 4 +--- 3 files changed, 3 insertions(+), 7 deletions(-) diff --git a/test/js/samples/if-block-complex/expected.js b/test/js/samples/if-block-complex/expected.js index f0da492e3caa..67d537f343da 100644 --- a/test/js/samples/if-block-complex/expected.js +++ b/test/js/samples/if-block-complex/expected.js @@ -78,4 +78,4 @@ class Component extends SvelteComponent { } } -export default Component; +export default Component; \ No newline at end of file diff --git a/test/js/samples/transition-local/expected.js b/test/js/samples/transition-local/expected.js index 53bce5f5d07b..f1397e63089f 100644 --- a/test/js/samples/transition-local/expected.js +++ b/test/js/samples/transition-local/expected.js @@ -37,9 +37,7 @@ function create_if_block(ctx) { if_block.c(); transition_in(if_block, 1); if_block.m(if_block_anchor.parentNode, if_block_anchor); - } else { - transition_in(if_block, 1); - } + } else transition_in(if_block, 1); } else if (if_block) { if_block.d(1); if_block = null; diff --git a/test/js/samples/transition-repeated-outro/expected.js b/test/js/samples/transition-repeated-outro/expected.js index 8543d0029b00..666f13a6aafe 100644 --- a/test/js/samples/transition-repeated-outro/expected.js +++ b/test/js/samples/transition-repeated-outro/expected.js @@ -76,9 +76,7 @@ function create_fragment(ctx) { if_block.c(); transition_in(if_block, 1); if_block.m(if_block_anchor.parentNode, if_block_anchor); - } else { - transition_in(if_block, 1); - } + } else transition_in(if_block, 1); } else if (if_block) { group_outros(); transition_out(if_block, 1, 1, () => { From c5e117a640585651ada63cf2f238ef4089590f20 Mon Sep 17 00:00:00 2001 From: Richard Harris Date: Tue, 3 Sep 2019 21:05:01 -0400 Subject: [PATCH 5/5] make things marginally more self-explanatory --- src/compiler/compile/render_dom/wrappers/IfBlock.ts | 9 ++++----- 1 file changed, 4 insertions(+), 5 deletions(-) diff --git a/src/compiler/compile/render_dom/wrappers/IfBlock.ts b/src/compiler/compile/render_dom/wrappers/IfBlock.ts index 64b90ecaf3ce..3c2899f355ee 100644 --- a/src/compiler/compile/render_dom/wrappers/IfBlock.ts +++ b/src/compiler/compile/render_dom/wrappers/IfBlock.ts @@ -39,7 +39,7 @@ class IfBlockBranch extends Wrapper { const is_else = !expression; if (expression) { - const dependencies = expression.dynamic_dependencies(); + this.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; @@ -55,7 +55,6 @@ class IfBlockBranch extends Wrapper { 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); } @@ -244,7 +243,7 @@ export default class IfBlockWrapper extends Wrapper { 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})`} + ${snippet && `if ((${condition} == null) || ${dependencies.map(n => `changed.${n}`).join(' || ')}) ${condition} = !!(${snippet})`} if (${condition}) return ${block.name};` : `return ${block.name};`)} } @@ -327,7 +326,7 @@ export default class IfBlockWrapper extends Wrapper { 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})`} + ${snippet && `if ((${condition} == null) || ${dependencies.map(n => `changed.${n}`).join(' || ')}) ${condition} = !!(${snippet})`} if (${condition}) return ${String(i)};` : `return ${i};`)} ${!has_else && `return -1;`} @@ -441,7 +440,7 @@ export default class IfBlockWrapper extends Wrapper { `if (${name}) ${name}.m(${initial_mount_node}, ${anchor_node});` ); - if (!branch.dependencies || branch.dependencies.length > 0) { + if (branch.dependencies.length > 0) { const update_mount_node = this.get_update_mount_node(anchor); const enter = dynamic