Merge pull request #134 from sveltejs/gh-88

Deconflict variable names
sveltejs · Dec 6, 2016 · 26d3f93 · 26d3f93
2 parents 53c6b87 + 4d3dcb6
commit 26d3f93
Show file tree

Hide file tree

Showing 22 changed files with 152 additions and 67 deletions.
diff --git a/compiler/generate/index.js b/compiler/generate/index.js
@@ -9,7 +9,7 @@ import getOutro from './utils/getOutro.js';
 import visitors from './visitors/index.js';
 import processCss from './css/process.js';
 
-export default function generate ( parsed, source, options ) {
+export default function generate ( parsed, source, options, names ) {
 	const format = options.format || 'es';
 	const renderers = [];
 
@@ -149,15 +149,13 @@ export default function generate ( parsed, source, options ) {
 			};
 		},
 
-		// TODO use getName instead of counters
-		counters: {
-			if: 0,
-			each: 0
-		},
-
 		events: {},
 
-		getName: counter(),
+		getUniqueName: counter( names ),
+
+		getUniqueNameMaker () {
+			return counter( names );
+		},
 
 		cssId: parsed.css ? `svelte-${parsed.hash}` : '',
 
@@ -278,7 +276,7 @@ export default function generate ( parsed, source, options ) {
 		indexNames: {},
 		listNames: {},
 
-		counter: counter()
+		getUniqueName: generator.getUniqueNameMaker()
 	});
 
 	parsed.html.children.forEach( generator.visit );

diff --git a/compiler/generate/utils/counter.js b/compiler/generate/utils/counter.js
@@ -1,12 +1,14 @@
-export default function counter () {
+export default function counter ( used ) {
 	const counts = {};
 
-	return function ( label ) {
-		if ( label in counts ) {
-			return `${label}${counts[ label ]++}`;
+	used.forEach( name => counts[ name ] = 1 );
+
+	return function ( name ) {
+		if ( name in counts ) {
+			return `${name}${counts[ name ]++}`;
 		}
 
-		counts[ label ] = 1;
-		return label;
+		counts[ name ] = 1;
+		return name;
 	};
 }
diff --git a/compiler/generate/visitors/Component.js b/compiler/generate/visitors/Component.js
@@ -1,11 +1,10 @@
 import deindent from '../utils/deindent.js';
 import addComponentAttributes from './attributes/addComponentAttributes.js';
-import counter from '../utils/counter.js';
 
 export default {
 	enter ( generator, node ) {
 		const hasChildren = node.children.length > 0;
-		const name = generator.current.counter( `${node.name[0].toLowerCase()}${node.name.slice( 1 )}` );
+		const name = generator.current.getUniqueName( `${node.name[0].toLowerCase()}${node.name.slice( 1 )}` );
 
 		const local = {
 			name,
@@ -33,12 +32,12 @@ export default {
 		];
 		// Component has children
 		if ( hasChildren ) {
-			const yieldName = `render${name}YieldFragment`;
+			const yieldName = generator.current.getUniqueName( `render${name}YieldFragment` );
 
 			// {{YIELD STUFF}}
 			generator.push({
 				useAnchor: true,
-				name: generator.current.counter(yieldName),
+				name: yieldName,
 				target: 'target',
 				localElementDepth: 0,
 
@@ -48,7 +47,7 @@ export default {
 				detachStatements: [],
 				teardownStatements: [],
 
-				counter: counter()
+				getUniqueName: generator.getUniqueNameMaker()
 			});
 
 			node.children.forEach( generator.visit );
@@ -106,9 +105,15 @@ export default {
 
 		if ( local.dynamicAttributes.length ) {
 			const updates = local.dynamicAttributes.map( attribute => {
-				return deindent`
-					if ( ${attribute.dependencies.map( dependency => `'${dependency}' in changed` ).join( '||' )} ) ${name}_changes.${attribute.name} = ${attribute.value};
-				`;
+				if ( attribute.dependencies.length ) {
+					return deindent`
+						if ( ${attribute.dependencies.map( dependency => `'${dependency}' in changed` ).join( '||' )} ) ${name}_changes.${attribute.name} = ${attribute.value};
+					`;
+				}
+
+				// TODO this is an odd situation to encounter – I *think* it should only happen with
+				// each block indices, in which case it may be possible to optimise this
+				return `${name}_changes.${attribute.name} = ${attribute.value};`;
 			});
 
 			local.update.push( deindent`

diff --git a/compiler/generate/visitors/EachBlock.js b/compiler/generate/visitors/EachBlock.js
@@ -1,14 +1,13 @@
 import deindent from '../utils/deindent.js';
-import counter from '../utils/counter.js';
 
 export default {
 	enter ( generator, node ) {
-		const i = generator.counters.each++;
-		const name = `eachBlock_${i}`;
+		const name = generator.getUniqueName( `eachBlock` );
+		const renderer = generator.getUniqueName( `renderEachBlock` );
 		const elseName = `${name}_else`;
 		const iterations = `${name}_iterations`;
-		const renderer = `renderEachBlock_${i}`;
 		const renderElse = `${renderer}_else`;
+		const i = generator.current.getUniqueName( `i` );
 		const { params } = generator.current;
 
 		const listName = `${name}_value`;
@@ -26,9 +25,9 @@ export default {
 			var ${iterations} = [];
 			${node.else ? `var ${elseName} = null;` : ''}
 
-			for ( var i = 0; i < ${name}_value.length; i += 1 ) {
-				${iterations}[i] = ${renderer}( ${params}, ${listName}, ${listName}[i], i, component );
-				${!isToplevel ? `${iterations}[i].mount( ${anchor}.parentNode, ${anchor} );` : ''}
+			for ( var ${i} = 0; ${i} < ${name}_value.length; ${i} += 1 ) {
+				${iterations}[${i}] = ${renderer}( ${params}, ${listName}, ${listName}[${i}], ${i}, component );
+				${!isToplevel ? `${iterations}[${i}].mount( ${anchor}.parentNode, ${anchor} );` : ''}
 			}
 		` );
 		if ( node.else ) {
@@ -42,8 +41,8 @@ export default {
 
 		if ( isToplevel ) {
 			generator.current.mountStatements.push( deindent`
-				for ( var i = 0; i < ${iterations}.length; i += 1 ) {
-					${iterations}[i].mount( ${anchor}.parentNode, ${anchor} );
+				for ( var ${i} = 0; ${i} < ${iterations}.length; ${i} += 1 ) {
+					${iterations}[${i}].mount( ${anchor}.parentNode, ${anchor} );
 				}
 			` );
 			if ( node.else ) {
@@ -58,21 +57,22 @@ export default {
 		generator.current.updateStatements.push( deindent`
 			var ${name}_value = ${snippet};
 
-			for ( var i = 0; i < ${name}_value.length; i += 1 ) {
-				if ( !${iterations}[i] ) {
-					${iterations}[i] = ${renderer}( ${params}, ${listName}, ${listName}[i], i, component );
-					${iterations}[i].mount( ${anchor}.parentNode, ${anchor} );
+			for ( var ${i} = 0; ${i} < ${name}_value.length; ${i} += 1 ) {
+				if ( !${iterations}[${i}] ) {
+					${iterations}[${i}] = ${renderer}( ${params}, ${listName}, ${listName}[${i}], ${i}, component );
+					${iterations}[${i}].mount( ${anchor}.parentNode, ${anchor} );
 				} else {
-					${iterations}[i].update( changed, ${params}, ${listName}, ${listName}[i], i );
+					${iterations}[${i}].update( changed, ${params}, ${listName}, ${listName}[${i}], ${i} );
 				}
 			}
 
-			for ( var i = ${name}_value.length; i < ${iterations}.length; i += 1 ) {
-				${iterations}[i].teardown( true );
+			for ( var ${i} = ${name}_value.length; ${i} < ${iterations}.length; ${i} += 1 ) {
+				${iterations}[${i}].teardown( true );
 			}
 
 			${iterations}.length = ${listName}.length;
 		` );
+
 		if ( node.else ) {
 			generator.current.updateStatements.push( deindent`
 				if ( !${name}_value.length && ${elseName} ) {
@@ -87,10 +87,11 @@ export default {
 		}
 
 		generator.current.teardownStatements.push( deindent`
-			for ( var i = 0; i < ${iterations}.length; i += 1 ) {
-				${iterations}[i].teardown( ${isToplevel ? 'detach' : 'false'} );
+			for ( var ${i} = 0; ${i} < ${iterations}.length; ${i} += 1 ) {
+				${iterations}[${i}].teardown( ${isToplevel ? 'detach' : 'false'} );
 			}
 		` );
+
 		if ( node.else ) {
 			generator.current.teardownStatements.push( deindent`
 				if ( ${elseName} ) {
@@ -112,7 +113,7 @@ export default {
 				detachStatements: [],
 				teardownStatements: [],
 
-				counter: counter()
+				getUniqueName: generator.getUniqueNameMaker()
 			});
 			node.else.children.forEach( generator.visit );
 			generator.addRenderer( generator.current );
@@ -163,9 +164,7 @@ export default {
 			detachStatements: [],
 			teardownStatements: [],
 
-			counter: counter(),
-
-			parent: generator.current
+			getUniqueName: generator.getUniqueNameMaker()
 		});
 	},
 

diff --git a/compiler/generate/visitors/Element.js b/compiler/generate/visitors/Element.js
@@ -9,7 +9,7 @@ export default {
 			return Component.enter( generator, node );
 		}
 
-		const name = generator.current.counter( node.name );
+		const name = generator.current.getUniqueName( node.name );
 
 		const local = {
 			name,

diff --git a/compiler/generate/visitors/IfBlock.js b/compiler/generate/visitors/IfBlock.js
@@ -1,5 +1,4 @@
 import deindent from '../utils/deindent.js';
-import counter from '../utils/counter.js';
 
 // collect all the conditions and blocks in the if/elseif/else chain
 function generateBlock ( generator, node, name ) {
@@ -16,7 +15,7 @@ function generateBlock ( generator, node, name ) {
 		detachStatements: [],
 		teardownStatements: [],
 
-		counter: counter()
+		getUniqueName: generator.getUniqueNameMaker()
 	});
 	node.children.forEach( generator.visit );
 	generator.addRenderer( generator.current );
@@ -54,15 +53,13 @@ function getConditionsAndBlocks ( generator, node, _name, i = 0 ) {
 
 export default {
 	enter ( generator, node ) {
-		const i = generator.counters.if++;
-
 		const { params } = generator.current;
-		const name = `ifBlock_${i}`;
-		const getBlock = `getBlock_${i}`;
-		const currentBlock = `currentBlock_${i}`;
+		const name = generator.getUniqueName( `ifBlock` );
+		const getBlock = generator.getUniqueName( `getBlock` );
+		const currentBlock = generator.getUniqueName( `currentBlock` );
 
 		const isToplevel = generator.current.localElementDepth === 0;
-		const conditionsAndBlocks = getConditionsAndBlocks( generator, node, `renderIfBlock_${i}` );
+		const conditionsAndBlocks = getConditionsAndBlocks( generator, node, generator.getUniqueName( `renderIfBlock` ) );
 
 		const anchor = generator.createAnchor( name, `#if ${generator.source.slice( node.expression.start, node.expression.end )}` );
 

diff --git a/compiler/generate/visitors/MustacheTag.js b/compiler/generate/visitors/MustacheTag.js
@@ -2,7 +2,7 @@ import deindent from '../utils/deindent.js';
 
 export default {
 	enter ( generator, node ) {
-		const name = generator.current.counter( 'text' );
+		const name = generator.current.getUniqueName( 'text' );
 
 		generator.addSourcemapLocations( node.expression );
 		const { snippet } = generator.contextualise( node.expression );

diff --git a/compiler/generate/visitors/RawMustacheTag.js b/compiler/generate/visitors/RawMustacheTag.js
@@ -2,7 +2,7 @@ import deindent from '../utils/deindent.js';
 
 export default {
 	enter ( generator, node ) {
-		const name = generator.current.counter( 'raw' );
+		const name = generator.current.getUniqueName( 'raw' );
 
 		generator.addSourcemapLocations( node.expression );
 		const { snippet } = generator.contextualise( node.expression );
@@ -25,7 +25,7 @@ export default {
 			}
 		`;
 
-		if ( isToplevel ) { 
+		if ( isToplevel ) {
 			generator.current.mountStatements.push(mountStatement);
 		} else {
 			generator.current.initStatements.push(mountStatement);

diff --git a/compiler/generate/visitors/Text.js b/compiler/generate/visitors/Text.js
@@ -1,6 +1,6 @@
 export default {
 	enter ( generator, node ) {
-		const name = generator.current.counter( `text` );
+		const name = generator.current.getUniqueName( `text` );
 		generator.addElement( name, `document.createTextNode( ${JSON.stringify( node.data )} )` );
 	}
 };
diff --git a/compiler/generate/visitors/attributes/addElementAttributes.js b/compiler/generate/visitors/attributes/addElementAttributes.js
@@ -138,7 +138,7 @@ export default function addElementAttributes ( generator, node, local ) {
 				return `var ${listName} = this.__svelte.${listName}, ${indexName} = this.__svelte.${indexName}, ${name} = ${listName}[${indexName}]`;
 			});
 
-			const handlerName = generator.current.counter( `${attribute.name}Handler` );
+			const handlerName = generator.current.getUniqueName( `${attribute.name}Handler` );
 			const handlerBody = ( declarations.length ? declarations.join( '\n' ) + '\n\n' : '' ) + `[✂${attribute.expression.start}-${attribute.expression.end}✂];`;
 
 			if ( attribute.name in generator.events ) {

diff --git a/compiler/generate/visitors/attributes/binding/index.js b/compiler/generate/visitors/attributes/binding/index.js
@@ -17,7 +17,7 @@ export default function createBinding ( generator, node, attribute, current, loc
 		});
 	}
 
-	const handler = current.counter( `${local.name}ChangeHandler` );
+	const handler = current.getUniqueName( `${local.name}ChangeHandler` );
 	let setter;
 
 	let eventName = 'change';

diff --git a/compiler/index.js b/compiler/index.js
@@ -15,9 +15,9 @@ export function compile ( source, options = {} ) {
 		};
 	}
 
-	validate( parsed, source, options );
+	const { names } = validate( parsed, source, options );
 
-	return generate( parsed, source, options );
+	return generate( parsed, source, options, names );
 }
 
 export { parse, validate };
diff --git a/compiler/validate/html/index.js b/compiler/validate/html/index.js
@@ -0,0 +1,14 @@
+export default function validateHtml ( validator, html ) {
+	function visit ( node ) {
+		if ( node.type === 'EachBlock' ) {
+			if ( !~validator.names.indexOf( node.context ) ) validator.names.push( node.context );
+			if ( node.index && !~validator.names.indexOf( node.index ) ) validator.names.push( node.index );
+		}
+
+		if ( node.children ) {
+			node.children.forEach( visit );
+		}
+	}
+
+	html.children.forEach( visit );
+}
diff --git a/compiler/validate/index.js b/compiler/validate/index.js
@@ -1,4 +1,5 @@
 import validateJs from './js/index.js';
+import validateHtml from './html/index.js';
 import { getLocator } from 'locate-character';
 import getCodeFrame from '../utils/getCodeFrame.js';
 
@@ -39,16 +40,18 @@ export default function validate ( parsed, source, options ) {
 
 		templateProperties: {},
 
-		errors: [],
-		warnings: []
+		names: []
 	};
 
+	if ( parsed.html ) {
+		validateHtml( validator, parsed.html );
+	}
+
 	if ( parsed.js ) {
 		validateJs( validator, parsed.js );
 	}
 
 	return {
-		errors: validator.errors,
-		warnings: validator.warnings
+		names: validator.names
 	};
 }
diff --git a/test/compiler/names-deconflicted-nested/_config.js b/test/compiler/names-deconflicted-nested/_config.js
@@ -0,0 +1,15 @@
+export default {
+	data: {
+		array: [ [0,0,0], [0,0,0], [0,0,0] ]
+	},
+
+	html: `
+		<div>
+			<span>[ 0, 0 ]</span><span>[ 0, 1 ]</span><span>[ 0, 2 ]</span>
+		</div><div>
+			<span>[ 1, 0 ]</span><span>[ 1, 1 ]</span><span>[ 1, 2 ]</span>
+		</div><div>
+			<span>[ 2, 0 ]</span><span>[ 2, 1 ]</span><span>[ 2, 2 ]</span>
+		</div>
+	`
+};
diff --git a/test/compiler/names-deconflicted-nested/main.html b/test/compiler/names-deconflicted-nested/main.html
@@ -0,0 +1,7 @@
+{{#each array as row, i}}
+	<div>
+		{{#each row as cell, j}}
+			<span>[ {{i}}, {{j}} ]</span>
+		{{/each}}
+	</div>
+{{/each}}