Merge pull request #568 from sveltejs/gh-543

ensure outroing each block iterations don't jump to the top

src/generators/dom/Block.js

@@ -5,10 +5,13 @@ export default class Block {
 	constructor ( options ) {
 		this.generator = options.generator;
 		this.name = options.name;
-		this.key = options.key;
 		this.expression = options.expression;
 		this.context = options.context;
 
+		// for keyed each blocks
+		this.key = options.key;
+		this.first = null;
+
 		this.contexts = options.contexts;
 		this.indexes = options.indexes;
 		this.contextDependencies = options.contextDependencies;
@@ -155,6 +158,10 @@ export default class Block {
 			properties.addBlock( `key: ${localKey},` );
 		}
 
+		if ( this.first ) {
+			properties.addBlock( `first: ${this.first},` );
+		}
+
 		if ( this.builders.mount.isEmpty() ) {
 			properties.addBlock( `mount: ${this.generator.helper( 'noop' )},` );
 		} else {

src/generators/dom/visitors/EachBlock.js

@@ -16,7 +16,6 @@ export default function visitEachBlock ( generator, block, state, node ) {
 	const { snippet } = block.contextualise( node.expression );
 
 	block.builders.create.addLine( `var ${each_block_value} = ${snippet};` );
-	block.builders.create.addLine( `var ${iterations} = [];` );
 
 	if ( node.key ) {
 		keyed( generator, block, state, node, snippet, vars );
@@ -26,23 +25,12 @@ export default function visitEachBlock ( generator, block, state, node ) {
 
 	const isToplevel = !state.parentNode;
 
-	if ( isToplevel ) {
-		block.builders.mount.addBlock( deindent`
-			for ( var ${i} = 0; ${i} < ${iterations}.length; ${i} += 1 ) {
-				${iterations}[${i}].${mountOrIntro}( ${block.target}, null );
-			}
-		` );
-	}
-
 	if ( node.needsAnchor ) {
 		block.addElement( anchor, `${generator.helper( 'createComment' )}()`, state.parentNode, true );
 	} else if ( node.next ) {
 		node.next.usedAsAnchor = true;
 	}
 
-	block.builders.destroy.addBlock(
-		`${generator.helper( 'destroyEach' )}( ${iterations}, ${isToplevel ? 'detach' : 'false'}, 0 );` );
-
 	if ( node.else ) {
 		const each_block_else = generator.getUniqueName( `${each_block}_else` );
 
@@ -109,104 +97,205 @@ export default function visitEachBlock ( generator, block, state, node ) {
 	}
 }
 
-function keyed ( generator, block, state, node, snippet, { each_block, create_each_block, each_block_value, iterations, i, params, anchor, mountOrIntro } ) {
+function keyed ( generator, block, state, node, snippet, { each_block, create_each_block, each_block_value, i, params, anchor, mountOrIntro } ) {
-	const fragment = block.getUniqueName( 'fragment' );
-	const value = block.getUniqueName( 'value' );
 	const key = block.getUniqueName( 'key' );
 	const lookup = block.getUniqueName( `${each_block}_lookup` );
-	const keys = block.getUniqueName( `${each_block}_keys` );
 	const iteration = block.getUniqueName( `${each_block}_iteration` );
-	const _iterations = block.getUniqueName( `_${each_block}_iterations` );
+	const head = block.getUniqueName( `${each_block}_head` );
+	const last = block.getUniqueName( `${each_block}_last` );
+	const expected = block.getUniqueName( `${each_block}_expected` );
+
+	if ( node.children[0] && node.children[0].type === 'Element' ) { // TODO or text/tag/raw
+		node._block.first = node.children[0]._state.parentNode; // TODO this is highly confusing
+	} else {
+		node._block.first = node._block.getUniqueName( 'first' );
+		node._block.addElement( node._block.first, `${generator.helper( 'createComment' )}()`, null, true );
+	}
 
 	block.builders.create.addBlock( deindent`
 		var ${lookup} = Object.create( null );
 
+		var ${head};
+		var ${last};
+
 		for ( var ${i} = 0; ${i} < ${each_block_value}.length; ${i} += 1 ) {
 			var ${key} = ${each_block_value}[${i}].${node.key};
-			${iterations}[${i}] = ${lookup}[ ${key} ] = ${create_each_block}( ${params}, ${each_block_value}, ${each_block_value}[${i}], ${i}, ${block.component}, ${key} );
+			var ${iteration} = ${lookup}[${key}] = ${create_each_block}( ${params}, ${each_block_value}, ${each_block_value}[${i}], ${i}, ${block.component}, ${key} );
-			${state.parentNode && `${iterations}[${i}].${mountOrIntro}( ${state.parentNode}, null );`}
+			${state.parentNode && `${iteration}.${mountOrIntro}( ${state.parentNode}, null );`}
+
+			if ( ${last} ) ${last}.next = ${iteration};
+			${iteration}.last = ${last};
+			${last} = ${iteration};
+
+			if ( ${i} === 0 ) ${head} = ${iteration};
 		}
 	` );
 
-	const consequent = node._block.hasUpdateMethod ?
+	if ( !state.parentNode ) {
-		deindent`
+		block.builders.mount.addBlock( deindent`
-			${_iterations}[${i}] = ${lookup}[ ${key} ] = ${lookup}[ ${key} ];
+			var ${iteration} = ${head};
-			${lookup}[ ${key} ].update( changed, ${params}, ${each_block_value}, ${each_block_value}[${i}], ${i} );
+			while ( ${iteration} ) {
-		` :
+				${iteration}.${mountOrIntro}( ${block.target}, null );
-		`${_iterations}[${i}] = ${lookup}[ ${key} ] = ${lookup}[ ${key} ];`;
+				${iteration} = ${iteration}.next;
+			}
+		` );
+	}
 
+	const dynamic = node._block.hasUpdateMethod;
 	const parentNode = state.parentNode || `${anchor}.parentNode`;
 
-	const hasIntros = node._block.hasIntroMethod;
+	let destroy;
-
+	if ( node._block.hasOutroMethod ) {
-	const destroy = node._block.hasOutroMethod ?
+		const fn = block.getUniqueName( `${each_block}_outro` );
-		deindent`
+		block.builders.create.addBlock( deindent`
-			function outro ( key ) {
+			function ${fn} ( iteration ) {
-				${lookup}[ key ].outro( function () {
+				iteration.outro( function () {
-					${lookup}[ key ].destroy( true );
+					iteration.destroy( true );
-					${lookup}[ key ] = null;
+					if ( iteration.next ) iteration.next.last = iteration.last;
+					if ( iteration.last ) iteration.last.next = iteration.next;
+					${lookup}[iteration.key] = null;
 				});
 			}
+		` );
 
-			for ( var ${i} = 0; ${i} < ${iterations}.length; ${i} += 1 ) {
+		destroy = deindent`
-				${key} = ${iterations}[${i}].key;
+			while ( ${expected} ) {
-				if ( !${keys}[ ${key} ] ) outro( ${key} );
+				${fn}( ${expected} );
+				${expected} = ${expected}.next;
 			}
-		` :
+
-		deindent`
+			for ( ${i} = 0; ${i} < discard_pile.length; ${i} += 1 ) {
-			for ( var ${i} = 0; ${i} < ${iterations}.length; ${i} += 1 ) {
+				if ( discard_pile[${i}].discard ) {
-				var ${iteration} = ${iterations}[${i}];
+					${fn}( discard_pile[${i}] );
-				if ( !${keys}[ ${iteration}.key ] ) ${iteration}.destroy( true );
+				}
+			}
+		`;
+	} else {
+		const fn = block.getUniqueName( `${each_block}_destroy` );
+		block.builders.create.addBlock( deindent`
+			function ${fn} ( iteration ) {
+				iteration.destroy( true );
+				if ( iteration.next && iteration.next.last === iteration ) iteration.next.last = iteration.last;
+				if ( iteration.last && iteration.last.next === iteration ) iteration.last.next = iteration.next;
+				${lookup}[iteration.key] = null;
+			}
+		` );
+
+		destroy = deindent`
+			while ( ${expected} ) {
+				${fn}( ${expected} );
+				${expected} = ${expected}.next;
+			}
+
+			for ( ${i} = 0; ${i} < discard_pile.length; ${i} += 1 ) {
+				var ${iteration} = discard_pile[${i}];
+				if ( ${iteration}.discard ) {
+					${fn}( ${iteration} );
+				}
 			}
 		`;
+	}
 
 	block.builders.update.addBlock( deindent`
 		var ${each_block_value} = ${snippet};
-		var ${_iterations} = [];
-		var ${keys} = Object.create( null );
 
-		var ${fragment} = document.createDocumentFragment();
+		var ${expected} = ${head};
+		var ${last};
 
-		// create new iterations as necessary
+		var discard_pile = [];
-		for ( var ${i} = 0; ${i} < ${each_block_value}.length; ${i} += 1 ) {
+
-			var ${value} = ${each_block_value}[${i}];
+		for ( ${i} = 0; ${i} < ${each_block_value}.length; ${i} += 1 ) {
-			var ${key} = ${value}.${node.key};
+			var ${key} = ${each_block_value}[${i}].${node.key};
-			${keys}[ ${key} ] = true;
+			var ${iteration} = ${lookup}[${key}];
+
+			${dynamic && `if ( ${iteration} ) ${iteration}.update( changed, ${params}, ${each_block_value}, ${each_block_value}[${i}], ${i} );`}
 
-			if ( ${lookup}[ ${key} ] ) {
+			if ( ${expected} ) {
-				${consequent}
+				if ( ${key} === ${expected}.key ) {
-				${hasIntros && `${_iterations}[${i}].mount( ${fragment}, null );`}
+					${expected} = ${expected}.next;
+				} else {
+					if ( ${iteration} ) {
+						// probably a deletion
+						do {
+							${expected}.discard = true;
+							discard_pile.push( ${expected} );
+							${expected} = ${expected}.next;
+						} while ( ${expected} && ${expected}.key !== ${key} );
+
+						${expected} = ${expected} && ${expected}.next;
+						${iteration}.discard = false;
+						${iteration}.last = ${last};
+						${iteration}.next = ${expected};
+
+						${iteration}.mount( ${parentNode}, ${expected} ? ${expected}.first : ${anchor} );
+					} else {
+						// key is being inserted
+						${iteration} = ${lookup}[${key}] = ${create_each_block}( ${params}, ${each_block_value}, ${each_block_value}[${i}], ${i}, ${block.component}, ${key} );
+						${iteration}.${mountOrIntro}( ${parentNode}, ${expected}.first );
+
+						if ( ${expected} ) ${expected}.last = ${iteration};
+						${iteration}.next = ${expected};
+					}
+				}
 			} else {
-				${_iterations}[${i}] = ${lookup}[ ${key} ] = ${create_each_block}( ${params}, ${each_block_value}, ${each_block_value}[${i}], ${i}, ${block.component}, ${key} );
+				// we're appending from this point forward
-				${hasIntros && `${_iterations}[${i}].intro( ${fragment}, null );`}
+				if ( ${iteration} ) {
+					${iteration}.discard = false;
+					${iteration}.next = null;
+					${iteration}.mount( ${parentNode}, ${anchor} );
+				} else {
+					${iteration} = ${lookup}[${key}] = ${create_each_block}( ${params}, ${each_block_value}, ${each_block_value}[${i}], ${i}, ${block.component}, ${key} );
+					${iteration}.${mountOrIntro}( ${parentNode}, ${anchor} );
+				}
 			}
 
-			${!hasIntros && `${_iterations}[${i}].mount( ${fragment}, null );`}
+			if ( ${last} ) ${last}.next = ${iteration};
+			${iteration}.last = ${last};
+			${node._block.hasIntroMethod && `${iteration}.intro( ${parentNode}, ${anchor} );`}
+			${last} = ${iteration};
 		}
 
-		// remove old iterations
+		if ( ${last} ) ${last}.next = null;
+
 		${destroy}
 
-		${parentNode}.insertBefore( ${fragment}, ${anchor} );
+		${head} = ${lookup}[${each_block_value}[0] && ${each_block_value}[0].${node.key}];
+	` );
 
-		${iterations} = ${_iterations};
+	block.builders.destroy.addBlock( deindent`
+		var ${iteration} = ${head};
+		while ( ${iteration} ) {
+			${iteration}.destroy( ${state.parentNode ? 'false' : 'detach'} );
+			${iteration} = ${iteration}.next;
+		}
 	` );
 }
 
 function unkeyed ( generator, block, state, node, snippet, { create_each_block, each_block_value, iterations, i, params, anchor, mountOrIntro } ) {
 	block.builders.create.addBlock( deindent`
+		var ${iterations} = [];
+
 		for ( var ${i} = 0; ${i} < ${each_block_value}.length; ${i} += 1 ) {
 			${iterations}[${i}] = ${create_each_block}( ${params}, ${each_block_value}, ${each_block_value}[${i}], ${i}, ${block.component} );
 			${state.parentNode && `${iterations}[${i}].${mountOrIntro}( ${state.parentNode}, null );`}
 		}
 	` );
 
+	if ( !state.parentNode ) {
+		block.builders.mount.addBlock( deindent`
+			for ( var ${i} = 0; ${i} < ${iterations}.length; ${i} += 1 ) {
+				${iterations}[${i}].${mountOrIntro}( ${block.target}, null );
+			}
+		` );
+	}
+
 	const dependencies = block.findDependencies( node.expression );
 	const allDependencies = new Set( node._block.dependencies );
 	dependencies.forEach( dependency => {
 		allDependencies.add( dependency );
 	});
 
+	// TODO do this for keyed blocks as well
 	const condition = Array.from( allDependencies )
 		.map( dependency => `'${dependency}' in changed` )
 		.join( ' || ' );
@@ -215,14 +304,23 @@ function unkeyed ( generator, block, state, node, snippet, { create_each_block,
 
 	if ( condition !== '' ) {
 		const forLoopBody = node._block.hasUpdateMethod ?
-			deindent`
+			node._block.hasIntroMethod ?
-				if ( ${iterations}[${i}] ) {
+				deindent`
-					${iterations}[${i}].update( changed, ${params}, ${each_block_value}, ${each_block_value}[${i}], ${i} );
+					if ( ${iterations}[${i}] ) {
-				} else {
+						${iterations}[${i}].update( changed, ${params}, ${each_block_value}, ${each_block_value}[${i}], ${i} );
-					${iterations}[${i}] = ${create_each_block}( ${params}, ${each_block_value}, ${each_block_value}[${i}], ${i}, ${block.component} );
+					} else {
-					${iterations}[${i}].${mountOrIntro}( ${parentNode}, ${anchor} );
+						${iterations}[${i}] = ${create_each_block}( ${params}, ${each_block_value}, ${each_block_value}[${i}], ${i}, ${block.component} );
-				}
+					}
-			` :
+					${iterations}[${i}].intro( ${parentNode}, ${anchor} );
+				` :
+				deindent`
+					if ( ${iterations}[${i}] ) {
+						${iterations}[${i}].update( changed, ${params}, ${each_block_value}, ${each_block_value}[${i}], ${i} );
+					} else {
+						${iterations}[${i}] = ${create_each_block}( ${params}, ${each_block_value}, ${each_block_value}[${i}], ${i}, ${block.component} );
+						${iterations}[${i}].mount( ${parentNode}, ${anchor} );
+					}
+				` :
 			deindent`
 				${iterations}[${i}] = ${create_each_block}( ${params}, ${each_block_value}, ${each_block_value}[${i}], ${i}, ${block.component} );
 				${iterations}[${i}].${mountOrIntro}( ${parentNode}, ${anchor} );
@@ -230,9 +328,10 @@ function unkeyed ( generator, block, state, node, snippet, { create_each_block,
 
 		const start = node._block.hasUpdateMethod ? '0' : `${iterations}.length`;
 
+		const outro = block.getUniqueName( 'outro' );
 		const destroy = node._block.hasOutroMethod ?
 			deindent`
-				function outro ( i ) {
+				function ${outro} ( i ) {
 					if ( ${iterations}[i] ) {
 						${iterations}[i].outro( function () {
 							${iterations}[i].destroy( true );
@@ -241,7 +340,7 @@ function unkeyed ( generator, block, state, node, snippet, { create_each_block,
 					}
 				}
 
-				for ( ; ${i} < ${iterations}.length; ${i} += 1 ) outro( ${i} );
+				for ( ; ${i} < ${iterations}.length; ${i} += 1 ) ${outro}( ${i} );
 			` :
 			deindent`
 				${generator.helper( 'destroyEach' )}( ${iterations}, true, ${each_block_value}.length );
@@ -260,4 +359,8 @@ function unkeyed ( generator, block, state, node, snippet, { create_each_block,
 			}
 		` );
 	}
+
+	block.builders.destroy.addBlock(
+		`${generator.helper( 'destroyEach' )}( ${iterations}, ${state.parentNode ? 'false' : 'detach'}, 0 );`
+	);
 }

src/shared/dom.js

@@ -18,7 +18,7 @@ export function detachBetween ( before, after ) {
 
 export function destroyEach ( iterations, detach, start ) {
 	for ( var i = start; i < iterations.length; i += 1 ) {
-		iterations[i].destroy( detach );
+		if ( iterations[i] ) iterations[i].destroy( detach );
 	}
 }
 

test/helpers.js

@@ -86,6 +86,7 @@ function cleanChildren ( node ) {
 				previous.data = previous.data.replace( /\s{2,}/, '\n' );
 
 				node.removeChild( child );
+				child = previous;
 			}
 		}
 

test/js/samples/each-block-changed-check/expected.js

@@ -4,6 +4,7 @@ function create_main_fragment ( state, component ) {
 	var text_1_value;
 
 	var each_block_value = state.comments;
+
 	var each_block_iterations = [];
 
 	for ( var i = 0; i < each_block_value.length; i += 1 ) {

test/runtime/samples/each-block-keyed-dynamic/_config.js

@@ -0,0 +1,35 @@
+export default {
+	data: {
+		todos: [
+			{ id: 123, description: 'buy milk' },
+			{ id: 234, description: 'drink milk' }
+		]
+	},
+
+	html: `
+		<p>buy milk</p>
+		<p>drink milk</p>
+	`,
+
+	test ( assert, component, target ) {
+		const [ p1, p2 ] = target.querySelectorAll( 'p' );
+
+		component.set({
+			todos: [
+				{ id: 123, description: 'buy beer' },
+				{ id: 234, description: 'drink beer' }
+			]
+		});
+		assert.htmlEqual( target.innerHTML, `
+			<p>buy beer</p>
+			<p>drink beer</p>
+		` );
+
+		const [ p3, p4 ] = target.querySelectorAll( 'p' );
+
+		assert.equal( p1, p3 );
+		assert.equal( p2, p4 );
+
+		component.destroy();
+	}
+};

test/runtime/samples/each-block-keyed-dynamic/main.html

@@ -0,0 +1,3 @@
+{{#each todos as todo @id}}
+	<p>{{todo.description}}</p>
+{{/each}}

test/runtime/samples/each-block-keyed-random-permute/_config.js

@@ -0,0 +1,48 @@
+const VALUES = Array.from( 'abcdefghijklmnopqrstuvwxyz' );
+
+function toObjects ( array ) {
+	return array.split( '' ).map( x => ({ id: x }) );
+}
+
+function permute () {
+	const values = VALUES.slice();
+	const number = Math.floor(Math.random() * VALUES.length);
+	const permuted = [];
+	for (let i = 0; i < number; i++) {
+		permuted.push( ...values.splice( Math.floor( Math.random() * ( number - i ) ), 1 ) );
+	}
+
+	return permuted.join( '' );
+}
+
+export default {
+	data: {
+		values: toObjects( 'abc' )
+	},
+
+	html: `(a)(b)(c)`,
+
+	test ( assert, component, target ) {
+		function test ( sequence ) {
+			component.set({ values: toObjects( sequence ) });
+			assert.htmlEqual( target.innerHTML, sequence.split( '' ).map( x => `(${x})` ).join( '' ) );
+		}
+
+		// first, some fixed tests so that we can debug them
+		test( 'abc' );
+		test( 'abcd' );
+		test( 'abecd' );
+		test( 'fabecd' );
+		test( 'fabed' );
+		test( 'beadf' );
+		test( 'ghbeadf' );
+		test( 'gf' );
+		test( 'gc' );
+		test( 'g' );
+		test( '' );
+		test( 'abc' );
+
+		// then, we party
+		for ( let i = 0; i < 100; i += 1 ) test( permute() );
+	}
+};

test/runtime/samples/each-block-keyed-random-permute/main.html

@@ -0,0 +1,3 @@
+{{#each values as value @id}}
+	({{value.id}})
+{{/each}}

test/runtime/samples/transition-js-each-block-keyed-intro-outro/_config.js

@@ -0,0 +1,65 @@
+export default {
+	data: {
+		things: [
+			{ name: 'a' },
+			{ name: 'b' },
+			{ name: 'c' }
+		]
+	},
+
+	test ( assert, component, target, window, raf ) {
+		const divs = target.querySelectorAll( 'div' );
+		divs[0].i = 0; // for debugging
+		divs[1].i = 1;
+		divs[2].i = 2;
+
+		assert.equal( divs[0].foo, 0 );
+		assert.equal( divs[1].foo, 0 );
+		assert.equal( divs[2].foo, 0 );
+
+		raf.tick( 100 );
+		assert.equal( divs[0].foo, 1 );
+		assert.equal( divs[1].foo, 1 );
+		assert.equal( divs[2].foo, 1 );
+
+		component.set({
+			things: [
+				{ name: 'a' },
+				{ name: 'c' }
+			]
+		});
+
+		const divs2 = target.querySelectorAll( 'div' );
+		assert.strictEqual( divs[0], divs2[0] );
+		assert.strictEqual( divs[1], divs2[1] );
+		assert.strictEqual( divs[2], divs2[2] );
+
+		raf.tick( 150 );
+		assert.equal( divs[0].foo, 1 );
+		assert.equal( divs[1].foo, 0.5 );
+		assert.equal( divs[2].foo, 1 );
+
+		component.set({
+			things: [
+				{ name: 'a' },
+				{ name: 'b' },
+				{ name: 'c' }
+			]
+		});
+
+		raf.tick( 175 );
+		assert.equal( divs[0].foo, 1 );
+		assert.equal( divs[1].foo, 0.75 );
+		assert.equal( divs[2].foo, 1 );
+
+		raf.tick( 225 );
+		const divs3 = target.querySelectorAll( 'div' );
+		assert.strictEqual( divs[0], divs3[0] );
+		assert.strictEqual( divs[1], divs3[1] );
+		assert.strictEqual( divs[2], divs3[2] );
+
+		assert.equal( divs[0].foo, 1 );
+		assert.equal( divs[1].foo, 1 );
+		assert.equal( divs[2].foo, 1 );
+	}
+};

test/runtime/samples/transition-js-each-block-keyed-intro-outro/main.html

@@ -0,0 +1,18 @@
+{{#each things as thing @name}}
+	<div transition:foo>{{thing.name}}</div>
+{{/each}}
+
+<script>
+	export default {
+		transitions: {
+			foo: function ( node, params ) {
+				return {
+					duration: 100,
+					tick: t => {
+						node.foo = t;
+					}
+				};
+			}
+		}
+	};
+</script>

test/runtime/samples/transition-js-each-block-keyed-outro/_config.js

@@ -17,6 +17,11 @@ export default {
 			]
 		});
 
+		const divs2 = target.querySelectorAll( 'div' );
+		assert.strictEqual( divs[0], divs2[0] );
+		assert.strictEqual( divs[1], divs2[1] );
+		assert.strictEqual( divs[2], divs2[2] );
+
 		raf.tick( 50 );
 		assert.equal( divs[0].foo, undefined );
 		assert.equal( divs[1].foo, 0.5 );