From 6250046c052eb360e51b272c55870cff71f41a70 Mon Sep 17 00:00:00 2001 From: Tan Li Hau Date: Sun, 16 Feb 2020 00:57:34 +0800 Subject: [PATCH] perform dirty check before updating keyed each blocks (#4413) --- CHANGELOG.md | 1 + .../compile/render_dom/wrappers/EachBlock.ts | 27 ++++++++++++------- .../each-block-keyed-animated/expected.js | 10 ++++--- test/js/samples/each-block-keyed/expected.js | 6 +++-- .../component-binding-blowback-c/main.svelte | 4 +-- .../each-block-keyed-dynamic-2/_config.js | 23 ++++++++++++++++ .../each-block-keyed-dynamic-2/main.svelte | 20 ++++++++++++++ 7 files changed, 74 insertions(+), 17 deletions(-) create mode 100644 test/runtime/samples/each-block-keyed-dynamic-2/_config.js create mode 100644 test/runtime/samples/each-block-keyed-dynamic-2/main.svelte diff --git a/CHANGELOG.md b/CHANGELOG.md index 74b16cc16f..406f2a591a 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -3,6 +3,7 @@ ## Unreleased * Fix indirect bindings involving elements with spreads ([#3680](https://github.com/sveltejs/svelte/issues/3680)) +* Fix unneeded updating of keyed each blocks ([#4373](https://github.com/sveltejs/svelte/issues/4373)) ## 3.18.2 diff --git a/src/compiler/compile/render_dom/wrappers/EachBlock.ts b/src/compiler/compile/render_dom/wrappers/EachBlock.ts index 639a8831bd..8f8fe29564 100644 --- a/src/compiler/compile/render_dom/wrappers/EachBlock.ts +++ b/src/compiler/compile/render_dom/wrappers/EachBlock.ts @@ -412,16 +412,25 @@ export default class EachBlockWrapper extends Wrapper { ? `@outro_and_destroy_block` : `@destroy_block`; - block.chunks.update.push(b` - const ${this.vars.each_block_value} = ${snippet}; + const all_dependencies = new Set(this.block.dependencies); // TODO should be dynamic deps only + this.node.expression.dynamic_dependencies().forEach((dependency: string) => { + all_dependencies.add(dependency); + }); - ${this.block.has_outros && b`@group_outros();`} - ${this.node.has_animation && b`for (let #i = 0; #i < ${view_length}; #i += 1) ${iterations}[#i].r();`} - ${this.renderer.options.dev && b`@validate_each_keys(#ctx, ${this.vars.each_block_value}, ${this.vars.get_each_context}, ${get_key});`} - ${iterations} = @update_keyed_each(${iterations}, #dirty, ${get_key}, ${dynamic ? 1 : 0}, #ctx, ${this.vars.each_block_value}, ${lookup}, ${update_mount_node}, ${destroy}, ${create_each_block}, ${update_anchor_node}, ${this.vars.get_each_context}); - ${this.node.has_animation && b`for (let #i = 0; #i < ${view_length}; #i += 1) ${iterations}[#i].a();`} - ${this.block.has_outros && b`@check_outros();`} - `); + if (all_dependencies.size) { + block.chunks.update.push(b` + if (${block.renderer.dirty(Array.from(all_dependencies))}) { + const ${this.vars.each_block_value} = ${snippet}; + + ${this.block.has_outros && b`@group_outros();`} + ${this.node.has_animation && b`for (let #i = 0; #i < ${view_length}; #i += 1) ${iterations}[#i].r();`} + ${this.renderer.options.dev && b`@validate_each_keys(#ctx, ${this.vars.each_block_value}, ${this.vars.get_each_context}, ${get_key});`} + ${iterations} = @update_keyed_each(${iterations}, #dirty, ${get_key}, ${dynamic ? 1 : 0}, #ctx, ${this.vars.each_block_value}, ${lookup}, ${update_mount_node}, ${destroy}, ${create_each_block}, ${update_anchor_node}, ${this.vars.get_each_context}); + ${this.node.has_animation && b`for (let #i = 0; #i < ${view_length}; #i += 1) ${iterations}[#i].a();`} + ${this.block.has_outros && b`@check_outros();`} + } + `); + } if (this.block.has_outros) { block.chunks.outro.push(b` diff --git a/test/js/samples/each-block-keyed-animated/expected.js b/test/js/samples/each-block-keyed-animated/expected.js index 3b697d6860..7fb81c27a2 100644 --- a/test/js/samples/each-block-keyed-animated/expected.js +++ b/test/js/samples/each-block-keyed-animated/expected.js @@ -92,10 +92,12 @@ function create_fragment(ctx) { insert(target, each_1_anchor, anchor); }, p(ctx, [dirty]) { - const each_value = /*things*/ ctx[0]; - for (let i = 0; i < each_blocks.length; i += 1) each_blocks[i].r(); - each_blocks = update_keyed_each(each_blocks, dirty, get_key, 1, ctx, each_value, each_1_lookup, each_1_anchor.parentNode, fix_and_destroy_block, create_each_block, each_1_anchor, get_each_context); - for (let i = 0; i < each_blocks.length; i += 1) each_blocks[i].a(); + if (dirty & /*things*/ 1) { + const each_value = /*things*/ ctx[0]; + for (let i = 0; i < each_blocks.length; i += 1) each_blocks[i].r(); + each_blocks = update_keyed_each(each_blocks, dirty, get_key, 1, ctx, each_value, each_1_lookup, each_1_anchor.parentNode, fix_and_destroy_block, create_each_block, each_1_anchor, get_each_context); + for (let i = 0; i < each_blocks.length; i += 1) each_blocks[i].a(); + } }, i: noop, o: noop, diff --git a/test/js/samples/each-block-keyed/expected.js b/test/js/samples/each-block-keyed/expected.js index 1cd9a31ece..ad8c074e99 100644 --- a/test/js/samples/each-block-keyed/expected.js +++ b/test/js/samples/each-block-keyed/expected.js @@ -77,8 +77,10 @@ function create_fragment(ctx) { insert(target, each_1_anchor, anchor); }, p(ctx, [dirty]) { - const each_value = /*things*/ ctx[0]; - each_blocks = update_keyed_each(each_blocks, dirty, get_key, 1, ctx, each_value, each_1_lookup, each_1_anchor.parentNode, destroy_block, create_each_block, each_1_anchor, get_each_context); + if (dirty & /*things*/ 1) { + const each_value = /*things*/ ctx[0]; + each_blocks = update_keyed_each(each_blocks, dirty, get_key, 1, ctx, each_value, each_1_lookup, each_1_anchor.parentNode, destroy_block, create_each_block, each_1_anchor, get_each_context); + } }, i: noop, o: noop, diff --git a/test/runtime/samples/component-binding-blowback-c/main.svelte b/test/runtime/samples/component-binding-blowback-c/main.svelte index 4876af13a8..75552bf358 100644 --- a/test/runtime/samples/component-binding-blowback-c/main.svelte +++ b/test/runtime/samples/component-binding-blowback-c/main.svelte @@ -4,7 +4,7 @@ export let count; export let idToValue = Object.create(null); - function ids() { + function ids(count) { return new Array(count) .fill(null) .map((_, i) => ({ id: 'id-' + i})) @@ -15,7 +15,7 @@
    - {#each ids() as object (object.id)} + {#each ids(count) as object (object.id)} {object.id}: value is {idToValue[object.id]} diff --git a/test/runtime/samples/each-block-keyed-dynamic-2/_config.js b/test/runtime/samples/each-block-keyed-dynamic-2/_config.js new file mode 100644 index 0000000000..7eba75a5ff --- /dev/null +++ b/test/runtime/samples/each-block-keyed-dynamic-2/_config.js @@ -0,0 +1,23 @@ +export default { + html: ` + + 0 + + `, + + async test({ assert, component, target, window }) { + const button = target.querySelector("button"); + + const event = new window.MouseEvent("click"); + await button.dispatchEvent(event); + + assert.htmlEqual( + target.innerHTML, + ` + + 1 + + ` + ); + } +}; diff --git a/test/runtime/samples/each-block-keyed-dynamic-2/main.svelte b/test/runtime/samples/each-block-keyed-dynamic-2/main.svelte new file mode 100644 index 0000000000..ab455a585e --- /dev/null +++ b/test/runtime/samples/each-block-keyed-dynamic-2/main.svelte @@ -0,0 +1,20 @@ + + + +{num} +