only cache values when it makes sense

src/generators/Generator.ts

@@ -125,7 +125,8 @@ export default class Generator {
 	) {
 		this.addSourcemapLocations(expression);
 
-		const usedContexts: string[] = [];
+		const usedContexts = new Set();
+		const usedIndexes = new Set();
 
 		const { code, helpers } = this;
 		const { contexts, indexes } = block;
@@ -168,12 +169,13 @@ export default class Generator {
 							);
 						}
 
-						if (!~usedContexts.indexOf(name)) usedContexts.push(name);
+						usedContexts.add(name);
 					} else if (helpers.has(name)) {
 						code.prependRight(node.start, `${self.alias('template')}.helpers.`);
 					} else if (indexes.has(name)) {
 						const context = indexes.get(name);
-						if (!~usedContexts.indexOf(context)) usedContexts.push(context);
+						usedContexts.add(context); // TODO is this right?
+						usedIndexes.add(name);
 					} else {
 						// handle shorthand properties
 						if (parent && parent.type === 'Property' && parent.shorthand) {
@@ -193,7 +195,7 @@ export default class Generator {
 							code.prependRight(node.start, `state.`);
 						}
 
-						if (!~usedContexts.indexOf('state')) usedContexts.push('state');
+						usedContexts.add('state');
 					}
 
 					this.skip();
@@ -221,6 +223,7 @@ export default class Generator {
 		return {
 			dependencies: Array.from(dependencies),
 			contexts: usedContexts,
+			indexes: usedIndexes,
 			snippet: `[✂${expression.start}-${expression.end}✂]`,
 		};
 	}

src/generators/dom/Block.ts

@@ -12,6 +12,7 @@ export interface BlockOptions {
 	key?: string;
 	contexts?: Map<string, string>;
 	indexes?: Map<string, string>;
+	changeableIndexes?: Map<string, boolean>;
 	contextDependencies?: Map<string, string[]>;
 	params?: string[];
 	indexNames?: Map<string, string>;
@@ -32,6 +33,7 @@ export default class Block {
 
 	contexts: Map<string, string>;
 	indexes: Map<string, string>;
+	changeableIndexes: Map<string, boolean>;
 	contextDependencies: Map<string, string[]>;
 	dependencies: Set<string>;
 	params: string[];
@@ -77,6 +79,7 @@ export default class Block {
 
 		this.contexts = options.contexts;
 		this.indexes = options.indexes;
+		this.changeableIndexes = options.changeableIndexes;
 		this.contextDependencies = options.contextDependencies;
 		this.dependencies = new Set();
 

src/generators/dom/preprocess.ts

@@ -189,7 +189,10 @@ const preprocessors = {
 		contexts.set(node.context, context);
 
 		const indexes = new Map(block.indexes);
-		if (node.index) indexes.set(indexName, node.context);
+		if (node.index) indexes.set(node.index, node.context);
+
+		const changeableIndexes = new Map(block.changeableIndexes);
+		if (node.index) changeableIndexes.set(node.index, node.key);
 
 		const contextDependencies = new Map(block.contextDependencies);
 		contextDependencies.set(node.context, dependencies);
@@ -203,6 +206,7 @@ const preprocessors = {
 			contextDependencies,
 			contexts,
 			indexes,
+			changeableIndexes,
 
 			listName,
 			indexName,
@@ -274,6 +278,11 @@ const preprocessors = {
 						}
 					}
 				});
+			} else if (attribute.type === 'EventHandler' && attribute.expression) {
+				attribute.expression.arguments.forEach((arg: Node) => {
+					const dependencies = block.findDependencies(arg);
+					block.addDependencies(dependencies);
+				});
 			} else if (attribute.type === 'Binding') {
 				const dependencies = block.findDependencies(attribute.value);
 				block.addDependencies(dependencies);
@@ -444,6 +453,7 @@ export default function preprocess(
 
 		contexts: new Map(),
 		indexes: new Map(),
+		changeableIndexes: new Map(),
 		contextDependencies: new Map(),
 
 		params: ['state'],

src/generators/dom/visitors/Element/meta/Window.ts

@@ -38,8 +38,8 @@ export default function visitWindow(
 			let usesState = false;
 
 			attribute.expression.arguments.forEach((arg: Node) => {
-				const { contexts } = block.contextualise(arg, null, true);
+				const { dependencies } = block.contextualise(arg, null, true);
-				if (contexts.length) usesState = true;
+				if (dependencies.length) usesState = true;
 			});
 
 			const flattened = flattenReference(attribute.expression.callee);

src/generators/dom/visitors/MustacheTag.ts

@@ -10,9 +10,15 @@ export default function visitMustacheTag(
 	state: State,
 	node: Node
 ) {
-	const { dependencies, snippet } = block.contextualise(node.expression);
+	const { dependencies, indexes, snippet } = block.contextualise(node.expression);
 
-	const shouldCache = node.expression.type !== 'Identifier' || block.contexts.has(node.expression.name);
+	const hasChangeableIndex = Array.from(indexes).some(index => block.changeableIndexes.get(index));
+
+	const shouldCache = (
+		node.expression.type !== 'Identifier' ||
+		block.contexts.has(node.expression.name) ||
+		hasChangeableIndex
+	);
 
 	const name = node._state.name;
 	const value = shouldCache && block.getUniqueName(`${name}_value`);
@@ -30,14 +36,16 @@ export default function visitMustacheTag(
 		true
 	);
 
-	if (dependencies.length) {
+	if (dependencies.length || hasChangeableIndex) {
 		const changedCheck = (
 			( block.hasOutroMethod ? `#outroing || ` : '' ) +
 			dependencies.map(dependency => `'${dependency}' in changed`).join(' || ')
 		);
 
+		const updateCachedValue = `${value} !== ( ${value} = ${snippet} )`;
+
 		const condition = shouldCache ?
-			`( ${changedCheck} ) && ${value} !== ( ${value} = ${snippet} )` :
+			( dependencies.length ? `( ${changedCheck} ) && ${updateCachedValue}` : updateCachedValue ) :
 			changedCheck;
 
 		block.builders.update.addConditionalLine(

src/utils/CodeBuilder.ts

@@ -26,10 +26,10 @@ export default class CodeBuilder {
 			this.result += `\n\t${line}`;
 		} else {
 			if (this.lastCondition) {
-				this.result += `\n}\n\n`;
+				this.result += `\n}`;
 			}
 
-			this.result += `if ( ${condition} ) {\n\t${line}`;
+			this.result += `${this.last === ChunkType.Block ? '\n\n' : '\n'}if ( ${condition} ) {\n\t${line}`;
 			this.lastCondition = condition;
 		}
 

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

@@ -6,7 +6,10 @@ export default {
 		]
 	},
 
-	html: '<p>implement keyed each blocks</p><p>implement client-side hydration</p>',
+	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' );
@@ -16,7 +19,7 @@ export default {
 				{ id: 234, description: 'implement client-side hydration' }
 			]
 		});
-		assert.htmlEqual( target.innerHTML, '<p>implement client-side hydration</p>' );
+		assert.htmlEqual( target.innerHTML, '<p>1: implement client-side hydration</p>' );
 
 		const [ p3 ] = target.querySelectorAll( 'p' );
 

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

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