Skip to content

Commit

Permalink
fix else-block update in keyed each-block (#4558)
Browse files Browse the repository at this point in the history 
Co-authored-by: Benjamin W. Broersma <[email protected]>
  • Loading branch information
@tanhauhau @bwbroersma
tanhauhau and bwbroersma authored Mar 14, 2020
1 parent ec3589e commit 404ed3d
Show file tree
Hide file tree
Showing 6 changed files with 129 additions and 30 deletions.
1 change: 1 addition & 0 deletions CHANGELOG.md
View file
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@

* Allow `<svelte:self>` to be used in a slot ([#2798](https://github.com/sveltejs/svelte/issues/2798))
* Expose object of unknown props in `$$restProps` ([#2930](https://github.com/sveltejs/svelte/issues/2930))
* Fix updating keyed `{#each}` blocks with `{:else}` ([#4536](https://github.com/sveltejs/svelte/issues/4536), [#4549](https://github.com/sveltejs/svelte/issues/4549))
* Fix hydration of top-level content ([#4542](https://github.com/sveltejs/svelte/issues/4542))

## 3.19.2
Expand Down
60 changes: 30 additions & 30 deletions src/compiler/compile/render_dom/wrappers/EachBlock.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -62,6 +62,8 @@ export default class EachBlockWrapper extends Wrapper {

context_props: Array<Node | Node[]>;
index_name: Identifier;
updates: Array<Node | Node[]> = [];
dependencies: Set<string>;

var: Identifier = { type: 'Identifier', name: 'each' };

Expand Down Expand Up @@ -235,6 +237,12 @@ export default class EachBlockWrapper extends Wrapper {
update_mount_node
};

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.dependencies = all_dependencies;

if (this.node.key) {
this.render_keyed(args);
} else {
Expand Down Expand Up @@ -291,7 +299,7 @@ export default class EachBlockWrapper extends Wrapper {
`);

if (this.else.block.has_update_method) {
block.chunks.update.push(b`
this.updates.push(b`
if (!${this.vars.data_length} && ${each_block_else}) {
${each_block_else}.p(#ctx, #dirty);
} else if (!${this.vars.data_length}) {
Expand All @@ -304,7 +312,7 @@ export default class EachBlockWrapper extends Wrapper {
}
`);
} else {
block.chunks.update.push(b`
this.updates.push(b`
if (${this.vars.data_length}) {
if (${each_block_else}) {
${each_block_else}.d(1);
Expand All @@ -323,6 +331,14 @@ export default class EachBlockWrapper extends Wrapper {
`);
}

if (this.updates.length) {
block.chunks.update.push(b`
if (${block.renderer.dirty(Array.from(all_dependencies))}) {
${this.updates}
}
`);
}

this.fragment.render(this.block, null, x`#nodes` as Identifier);

if (this.else) {
Expand Down Expand Up @@ -415,24 +431,17 @@ export default class EachBlockWrapper extends Wrapper {
? `@outro_and_destroy_block`
: `@destroy_block`;

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);
});
if (this.dependencies.size) {
this.updates.push(b`
const ${this.vars.each_block_value} = ${snippet};
${this.renderer.options.dev && b`@validate_each_argument(${this.vars.each_block_value});`}
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.renderer.options.dev && b`@validate_each_argument(${this.vars.each_block_value});`}
${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();`}
}
${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();`}
`);
}

Expand Down Expand Up @@ -504,12 +513,7 @@ export default class EachBlockWrapper extends Wrapper {
}
`);

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

if (all_dependencies.size) {
if (this.dependencies.size) {
const has_transitions = !!(this.block.has_intro_method || this.block.has_outro_method);

const for_loop_body = this.block.has_update_method
Expand Down Expand Up @@ -588,11 +592,7 @@ export default class EachBlockWrapper extends Wrapper {
${remove_old_blocks}
`;

block.chunks.update.push(b`
if (${block.renderer.dirty(Array.from(all_dependencies))}) {
${update}
}
`);
this.updates.push(update);
}

if (this.block.has_outros) {
Expand Down
37 changes: 37 additions & 0 deletions test/runtime/samples/each-block-keyed-else/_config.js
View file
Original file line number Diff line number Diff line change
@@ -0,0 +1,37 @@
export default {
props: {
animals: ['alpaca', 'baboon', 'capybara'],
foo: 'something else'
},

html: `
before
<p>alpaca</p>
<p>baboon</p>
<p>capybara</p>
after
`,

test({ assert, component, target }) {
component.animals = [];
assert.htmlEqual(target.innerHTML, `
before
<p>no animals, but rather something else</p>
after
`);

component.foo = 'something other';
assert.htmlEqual(target.innerHTML, `
before
<p>no animals, but rather something other</p>
after
`);

component.animals = ['wombat'];
assert.htmlEqual(target.innerHTML, `
before
<p>wombat</p>
after
`);
}
};
12 changes: 12 additions & 0 deletions test/runtime/samples/each-block-keyed-else/main.svelte
View file
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
<script>
export let animals;
export let foo;
</script>

before
{#each animals as animal (animal)}
<p>{animal}</p>
{:else}
<p>no animals, but rather {foo}</p>
{/each}
after
37 changes: 37 additions & 0 deletions test/runtime/samples/each-block-unkeyed-else-2/_config.js
View file
Original file line number Diff line number Diff line change
@@ -0,0 +1,37 @@
export default {
props: {
animals: ['alpaca', 'baboon', 'capybara'],
foo: 'something else'
},

html: `
before
<p>alpaca</p>
<p>baboon</p>
<p>capybara</p>
after
`,

test({ assert, component, target }) {
component.animals = [];
assert.htmlEqual(target.innerHTML, `
before
<p>no animals, but rather something else</p>
after
`);

component.foo = 'something other';
assert.htmlEqual(target.innerHTML, `
before
<p>no animals, but rather something other</p>
after
`);

component.animals = ['wombat'];
assert.htmlEqual(target.innerHTML, `
before
<p>wombat</p>
after
`);
}
};
12 changes: 12 additions & 0 deletions test/runtime/samples/each-block-unkeyed-else-2/main.svelte
View file
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
<script>
export let animals;
export let foo;
</script>

before
{#each animals as animal}
<p>{animal}</p>
{:else}
<p>no animals, but rather {foo}</p>
{/each}
after

0 comments on commit 404ed3d

Please sign in to comment.