Skip to content

Commit

Permalink
use maps for keyed each block lookups - fixes #2430
Browse files Browse the repository at this point in the history
  • Loading branch information
Rich-Harris committed Apr 17, 2019
1 parent e7f5467 commit c988457
Show file tree
Hide file tree
Showing 6 changed files with 54 additions and 24 deletions.
4 changes: 2 additions & 2 deletions src/compile/render-dom/wrappers/EachBlock.ts
Original file line number Diff line number Diff line change
Expand Up @@ -294,7 +294,7 @@ export default class EachBlockWrapper extends Wrapper {
const lookup = block.get_unique_name(`${this.var}_lookup`);

block.add_variable(iterations, '[]');
block.add_variable(lookup, `@blank_object()`);
block.add_variable(lookup, `new Map()`);

if (this.fragment.nodes[0].is_dom_node()) {
this.block.first = this.fragment.nodes[0].var;
Expand All @@ -314,7 +314,7 @@ export default class EachBlockWrapper extends Wrapper {
for (var #i = 0; #i < ${this.vars.each_block_value}.${length}; #i += 1) {
let child_ctx = ${this.vars.get_each_context}(ctx, ${this.vars.each_block_value}, #i);
let key = ${get_key}(child_ctx);
${iterations}[#i] = ${lookup}[key] = ${create_each_block}(key, child_ctx);
${lookup}.set(key, ${iterations}[#i] = ${create_each_block}(key, child_ctx));
}
`);

Expand Down
32 changes: 16 additions & 16 deletions src/internal/keyed-each.js
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ import { on_outro } from './transitions.js';

export function destroy_block(block, lookup) {
block.d(1);
lookup[block.key] = null;
lookup.delete(block.key);
}

export function outro_and_destroy_block(block, lookup) {
Expand All @@ -27,14 +27,14 @@ export function update_keyed_each(old_blocks, changed, get_key, dynamic, ctx, li
while (i--) old_indexes[old_blocks[i].key] = i;

const new_blocks = [];
const new_lookup = {};
const deltas = {};
const new_lookup = new Map();
const deltas = new Map();

i = n;
while (i--) {
const child_ctx = get_context(ctx, list, i);
const key = get_key(child_ctx);
let block = lookup[key];
let block = lookup.get(key);

if (!block) {
block = create_each_block(key, child_ctx);
Expand All @@ -43,18 +43,18 @@ export function update_keyed_each(old_blocks, changed, get_key, dynamic, ctx, li
block.p(changed, child_ctx);
}

new_blocks[i] = new_lookup[key] = block;
new_lookup.set(key, new_blocks[i] = block);

if (key in old_indexes) deltas[key] = Math.abs(i - old_indexes[key]);
if (key in old_indexes) deltas.set(key, Math.abs(i - old_indexes[key]));
}

const will_move = {};
const did_move = {};
const will_move = new Set();
const did_move = new Set();

function insert(block) {
if (block.i) block.i(1);
block.m(node, next);
lookup[block.key] = block;
lookup.set(block.key, block);
next = block.first;
n--;
}
Expand All @@ -72,32 +72,32 @@ export function update_keyed_each(old_blocks, changed, get_key, dynamic, ctx, li
n--;
}

else if (!new_lookup[old_key]) {
else if (!new_lookup.has(old_key)) {
// remove old block
destroy(old_block, lookup);
o--;
}

else if (!lookup[new_key] || will_move[new_key]) {
else if (!lookup.has(new_key) || will_move.has(new_key)) {
insert(new_block);
}

else if (did_move[old_key]) {
else if (did_move.has(old_key)) {
o--;

} else if (deltas[new_key] > deltas[old_key]) {
did_move[new_key] = true;
} else if (deltas.get(new_key) > deltas.get(old_key)) {
did_move.add(new_key);
insert(new_block);

} else {
will_move[old_key] = true;
will_move.add(old_key);
o--;
}
}

while (o--) {
const old_block = old_blocks[o];
if (!new_lookup[old_block.key]) destroy(old_block, lookup);
if (!new_lookup.has(old_block.key)) destroy(old_block, lookup);
}

while (n) insert(new_blocks[n - 1]);
Expand Down
5 changes: 2 additions & 3 deletions test/js/samples/each-block-keyed-animated/expected.js
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,6 @@
import {
SvelteComponent,
append,
blank_object,
create_animation,
detach,
element,
Expand Down Expand Up @@ -73,7 +72,7 @@ function create_each_block(key_1, ctx) {
}

function create_fragment(ctx) {
var each_blocks = [], each_1_lookup = blank_object(), each_1_anchor;
var each_blocks = [], each_1_lookup = new Map(), each_1_anchor;

var each_value = ctx.things;

Expand All @@ -82,7 +81,7 @@ function create_fragment(ctx) {
for (var i = 0; i < each_value.length; i += 1) {
let child_ctx = get_each_context(ctx, each_value, i);
let key = get_key(child_ctx);
each_blocks[i] = each_1_lookup[key] = create_each_block(key, child_ctx);
each_1_lookup.set(key, each_blocks[i] = create_each_block(key, child_ctx));
}

return {
Expand Down
5 changes: 2 additions & 3 deletions test/js/samples/each-block-keyed/expected.js
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,6 @@
import {
SvelteComponent,
append,
blank_object,
destroy_block,
detach,
element,
Expand Down Expand Up @@ -57,7 +56,7 @@ function create_each_block(key_1, ctx) {
}

function create_fragment(ctx) {
var each_blocks = [], each_1_lookup = blank_object(), each_1_anchor;
var each_blocks = [], each_1_lookup = new Map(), each_1_anchor;

var each_value = ctx.things;

Expand All @@ -66,7 +65,7 @@ function create_fragment(ctx) {
for (var i = 0; i < each_value.length; i += 1) {
let child_ctx = get_each_context(ctx, each_value, i);
let key = get_key(child_ctx);
each_blocks[i] = each_1_lookup[key] = create_each_block(key, child_ctx);
each_1_lookup.set(key, each_blocks[i] = create_each_block(key, child_ctx));
}

return {
Expand Down
25 changes: 25 additions & 0 deletions test/runtime/samples/each-block-keyed-object-identity/_config.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,25 @@
export default {
props: {
todos: [
{ description: 'implement keyed each blocks' },
{ description: 'implement client-side hydration' }
]
},

html: `
<p>1: implement keyed each blocks</p>
<p>2: implement client-side hydration</p>
`,

test({ assert, component, target }) {
const [p1, p2] = target.querySelectorAll('p');

component.todos = [component.todos[1]];
assert.htmlEqual(target.innerHTML, '<p>1: implement client-side hydration</p>');

const [p3] = target.querySelectorAll('p');

assert.ok(!target.contains(p1), 'first <p> element should be removed');
assert.equal(p2, p3, 'second <p> element should be retained');
}
};
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
<script>
export let todos;
</script>

{#each todos as todo, i (todo)}
<p>{i+1}: {todo.description}</p>
{/each}

0 comments on commit c988457

Please sign in to comment.