From fb9edf2f5739408081afddc35194294ceaac56b5 Mon Sep 17 00:00:00 2001
From: Rich Harris <richard.a.harris@gmail.com>
Date: Mon, 10 Apr 2017 12:00:27 -0400
Subject: [PATCH] align component code more closely with post-refactor element
 code

---
 .../dom/visitors/Component/Attribute.js       |  67 +++++++++
 .../addComponentBinding.js => Binding.js}     |  10 +-
 .../dom/visitors/Component/Component.js       |  36 ++++-
 .../dom/visitors/Component/EventHandler.js    |  35 +++++
 src/generators/dom/visitors/Component/Ref.js  |  13 ++
 .../attributes/addComponentAttributes.js      | 132 ------------------
 6 files changed, 150 insertions(+), 143 deletions(-)
 create mode 100644 src/generators/dom/visitors/Component/Attribute.js
 rename src/generators/dom/visitors/Component/{attributes/addComponentBinding.js => Binding.js} (86%)
 create mode 100644 src/generators/dom/visitors/Component/EventHandler.js
 create mode 100644 src/generators/dom/visitors/Component/Ref.js
 delete mode 100644 src/generators/dom/visitors/Component/attributes/addComponentAttributes.js

diff --git a/src/generators/dom/visitors/Component/Attribute.js b/src/generators/dom/visitors/Component/Attribute.js
new file mode 100644
index 0000000000..601bc7049f
--- /dev/null
+++ b/src/generators/dom/visitors/Component/Attribute.js
@@ -0,0 +1,67 @@
+export default function visitAttribute ( generator, block, state, node, attribute, local ) {
+	if ( attribute.value === true ) {
+		// attributes without values, e.g. <textarea readonly>
+		local.staticAttributes.push({
+			name: attribute.name,
+			value: true
+		});
+	}
+
+	else if ( attribute.value.length === 0 ) {
+		local.staticAttributes.push({
+			name: attribute.name,
+			value: `''`
+		});
+	}
+
+	else if ( attribute.value.length === 1 ) {
+		const value = attribute.value[0];
+
+		if ( value.type === 'Text' ) {
+			// static attributes
+			const result = isNaN( value.data ) ? JSON.stringify( value.data ) : value.data;
+			local.staticAttributes.push({
+				name: attribute.name,
+				value: result
+			});
+		}
+
+		else {
+			// simple dynamic attributes
+			const { dependencies, string } = generator.contextualise( block, value.expression );
+
+			// TODO only update attributes that have changed
+			local.dynamicAttributes.push({
+				name: attribute.name,
+				value: string,
+				dependencies
+			});
+		}
+	}
+
+	else {
+		// complex dynamic attributes
+		const allDependencies = [];
+
+		const value = ( attribute.value[0].type === 'Text' ? '' : `"" + ` ) + (
+			attribute.value.map( chunk => {
+				if ( chunk.type === 'Text' ) {
+					return JSON.stringify( chunk.data );
+				} else {
+					const { dependencies, string } = generator.contextualise( block, chunk.expression );
+					dependencies.forEach( dependency => {
+						if ( !~allDependencies.indexOf( dependency ) ) allDependencies.push( dependency );
+					});
+
+					return `( ${string} )`;
+				}
+			}).join( ' + ' )
+		);
+
+		local.dynamicAttributes.push({
+			name: attribute.name,
+			value,
+			dependencies: allDependencies
+		});
+	}
+}
\ No newline at end of file
diff --git a/src/generators/dom/visitors/Component/attributes/addComponentBinding.js b/src/generators/dom/visitors/Component/Binding.js
similarity index 86%
rename from src/generators/dom/visitors/Component/attributes/addComponentBinding.js
rename to src/generators/dom/visitors/Component/Binding.js
index f9a7e035d2..289c796118 100644
--- a/src/generators/dom/visitors/Component/attributes/addComponentBinding.js
+++ b/src/generators/dom/visitors/Component/Binding.js
@@ -1,8 +1,8 @@
-import deindent from '../../../../../utils/deindent.js';
-import flattenReference from '../../../../../utils/flattenReference.js';
-import getSetter from '../../shared/binding/getSetter.js';
+import deindent from '../../../../utils/deindent.js';
+import flattenReference from '../../../../utils/flattenReference.js';
+import getSetter from '../shared/binding/getSetter.js';
 
-export default function addComponentBinding ( generator, node, attribute, block, local ) {
+export default function visitBinding ( generator, block, state, node, attribute, local ) {
 	const { name, keypath } = flattenReference( attribute.value );
 	const { snippet, contexts, dependencies } = generator.contextualise( block, attribute.value );
 
@@ -62,4 +62,4 @@ export default function addComponentBinding ( generator, node, attribute, block,
 			${updating} = false;
 		}
 	` );
-}
+}
\ No newline at end of file
diff --git a/src/generators/dom/visitors/Component/Component.js b/src/generators/dom/visitors/Component/Component.js
index 577a78fa1a..5446d54276 100644
--- a/src/generators/dom/visitors/Component/Component.js
+++ b/src/generators/dom/visitors/Component/Component.js
@@ -1,7 +1,10 @@
 import deindent from '../../../../utils/deindent.js';
 import CodeBuilder from '../../../../utils/CodeBuilder.js';
 import visit from '../../visit.js';
-import addComponentAttributes from './attributes/addComponentAttributes.js';
+import visitAttribute from './Attribute.js';
+import visitEventHandler from './EventHandler.js';
+import visitBinding from './Binding.js';
+import visitRef from './Ref.js';
 
 function capDown ( name ) {
 	return `${name[0].toLowerCase()}${name.slice( 1 )}`;
@@ -19,16 +22,37 @@ function stringifyProps ( props ) {
 	return `{ ${joined} }`;
 }
 
+const order = {
+	Attribute: 1,
+	EventHandler: 2,
+	Binding: 3,
+	Ref: 4
+};
+
+const visitors = {
+	Attribute: visitAttribute,
+	EventHandler: visitEventHandler,
+	Binding: visitBinding,
+	Ref: visitRef
+};
+
 export default function visitComponent ( generator, block, state, node ) {
 	const hasChildren = node.children.length > 0;
 	const name = block.getUniqueName( capDown( node.name === ':Self' ? generator.name : node.name ) );
 
+	const childState = Object.assign( {}, state, {
+		parentNode: null
+	});
+
 	const local = {
 		name,
 		namespace: state.namespace,
 		isComponent: true,
 
 		allUsedContexts: [],
+		staticAttributes: [],
+		dynamicAttributes: [],
+		bindings: [],
 
 		create: new CodeBuilder(),
 		update: new CodeBuilder()
@@ -38,7 +62,11 @@ export default function visitComponent ( generator, block, state, node ) {
 
 	generator.hasComponents = true;
 
-	addComponentAttributes( generator, block, node, local );
+	node.attributes
+		.sort( ( a, b ) => order[ a.type ] - order[ b.type ] )
+		.forEach( attribute => {
+			visitors[ attribute.type ]( generator, block, childState, node, attribute, local );
+		});
 
 	if ( local.allUsedContexts.length ) {
 		const initialProps = local.allUsedContexts.map( contextName => {
@@ -81,10 +109,6 @@ export default function visitComponent ( generator, block, state, node ) {
 			name: generator.getUniqueName( `create_${name}_yield_fragment` ) // TODO should getUniqueName happen inside Fragment? probably
 		});
 
-		const childState = Object.assign( {}, state, {
-			parentNode: null
-		});
-
 		node.children.forEach( child => {
 			visit( generator, childBlock, childState, child );
 		});
diff --git a/src/generators/dom/visitors/Component/EventHandler.js b/src/generators/dom/visitors/Component/EventHandler.js
new file mode 100644
index 0000000000..263f88e4ad
--- /dev/null
+++ b/src/generators/dom/visitors/Component/EventHandler.js
@@ -0,0 +1,35 @@
+import deindent from '../../../../utils/deindent.js';
+
+export default function visitEventHandler ( generator, block, state, node, attribute, local ) {
+	// TODO verify that it's a valid callee (i.e. built-in or declared method)
+	generator.addSourcemapLocations( attribute.expression );
+	generator.code.prependRight( attribute.expression.start, `${block.component}.` );
+
+	const usedContexts = [];
+	attribute.expression.arguments.forEach( arg => {
+		const { contexts } = generator.contextualise( block, arg, null, true );
+
+		contexts.forEach( context => {
+			if ( !~usedContexts.indexOf( context ) ) usedContexts.push( context );
+			if ( !~local.allUsedContexts.indexOf( context ) ) local.allUsedContexts.push( context );
+		});
+	});
+
+	// TODO hoist event handlers? can do `this.__component.method(...)`
+	const declarations = usedContexts.map( name => {
+		if ( name === 'root' ) return 'var root = this._context.root;';
+
+		const listName = block.listNames.get( name );
+		const indexName = block.indexNames.get( name );
+
+		return `var ${listName} = this._context.${listName}, ${indexName} = this._context.${indexName}, ${name} = ${listName}[${indexName}]`;
+	});
+
+	const handlerBody = ( declarations.length ? declarations.join( '\n' ) + '\n\n' : '' ) + `[✂${attribute.expression.start}-${attribute.expression.end}✂];`;
+
+	local.create.addBlock( deindent`
+		${local.name}.on( '${attribute.name}', function ( event ) {
+			${handlerBody}
+		});
+	` );	
+}
\ No newline at end of file
diff --git a/src/generators/dom/visitors/Component/Ref.js b/src/generators/dom/visitors/Component/Ref.js
new file mode 100644
index 0000000000..727cf8f29a
--- /dev/null
+++ b/src/generators/dom/visitors/Component/Ref.js
@@ -0,0 +1,13 @@
+import deindent from '../../../../utils/deindent.js';
+
+export default function visitRef ( generator, block, state, node, attribute, local ) {
+	generator.usesRefs = true;
+
+	local.create.addLine(
+		`${block.component}.refs.${attribute.name} = ${local.name};`
+	);
+
+	block.builders.destroy.addLine( deindent`
+		if ( ${block.component}.refs.${attribute.name} === ${local.name} ) ${block.component}.refs.${attribute.name} = null;
+	` );
+}
\ No newline at end of file
diff --git a/src/generators/dom/visitors/Component/attributes/addComponentAttributes.js b/src/generators/dom/visitors/Component/attributes/addComponentAttributes.js
deleted file mode 100644
index 88a4d9ddf2..0000000000
--- a/src/generators/dom/visitors/Component/attributes/addComponentAttributes.js
+++ /dev/null
@@ -1,132 +0,0 @@
-import addComponentBinding from './addComponentBinding.js';
-import deindent from '../../../../../utils/deindent.js';
-
-export default function addComponentAttributes ( generator, block, node, local ) {
-	local.staticAttributes = [];
-	local.dynamicAttributes = [];
-	local.bindings = [];
-
-	node.attributes.forEach( attribute => {
-		if ( attribute.type === 'Attribute' ) {
-			if ( attribute.value === true ) {
-				// attributes without values, e.g. <textarea readonly>
-				local.staticAttributes.push({
-					name: attribute.name,
-					value: true
-				});
-			}
-
-			else if ( attribute.value.length === 0 ) {
-				local.staticAttributes.push({
-					name: attribute.name,
-					value: `''`
-				});
-			}
-
-			else if ( attribute.value.length === 1 ) {
-				const value = attribute.value[0];
-
-				if ( value.type === 'Text' ) {
-					// static attributes
-					const result = isNaN( value.data ) ? JSON.stringify( value.data ) : value.data;
-					local.staticAttributes.push({
-						name: attribute.name,
-						value: result
-					});
-				}
-
-				else {
-					// simple dynamic attributes
-					const { dependencies, string } = generator.contextualise( block, value.expression );
-
-					// TODO only update attributes that have changed
-					local.dynamicAttributes.push({
-						name: attribute.name,
-						value: string,
-						dependencies
-					});
-				}
-			}
-
-			else {
-				// complex dynamic attributes
-				const allDependencies = [];
-
-				const value = ( attribute.value[0].type === 'Text' ? '' : `"" + ` ) + (
-					attribute.value.map( chunk => {
-						if ( chunk.type === 'Text' ) {
-							return JSON.stringify( chunk.data );
-						} else {
-							const { dependencies, string } = generator.contextualise( block, chunk.expression );
-							dependencies.forEach( dependency => {
-								if ( !~allDependencies.indexOf( dependency ) ) allDependencies.push( dependency );
-							});
-
-							return `( ${string} )`;
-						}
-					}).join( ' + ' )
-				);
-
-				local.dynamicAttributes.push({
-					name: attribute.name,
-					value,
-					dependencies: allDependencies
-				});
-			}
-		}
-
-		else if ( attribute.type === 'EventHandler' ) {
-			// TODO verify that it's a valid callee (i.e. built-in or declared method)
-			generator.addSourcemapLocations( attribute.expression );
-			generator.code.prependRight( attribute.expression.start, `${block.component}.` );
-
-			const usedContexts = [];
-			attribute.expression.arguments.forEach( arg => {
-				const { contexts } = generator.contextualise( block, arg, null, true );
-
-				contexts.forEach( context => {
-					if ( !~usedContexts.indexOf( context ) ) usedContexts.push( context );
-					if ( !~local.allUsedContexts.indexOf( context ) ) local.allUsedContexts.push( context );
-				});
-			});
-
-			// TODO hoist event handlers? can do `this.__component.method(...)`
-			const declarations = usedContexts.map( name => {
-				if ( name === 'root' ) return 'var root = this._context.root;';
-
-				const listName = block.listNames.get( name );
-				const indexName = block.indexNames.get( name );
-
-				return `var ${listName} = this._context.${listName}, ${indexName} = this._context.${indexName}, ${name} = ${listName}[${indexName}]`;
-			});
-
-			const handlerBody = ( declarations.length ? declarations.join( '\n' ) + '\n\n' : '' ) + `[✂${attribute.expression.start}-${attribute.expression.end}✂];`;
-
-			local.create.addBlock( deindent`
-				${local.name}.on( '${attribute.name}', function ( event ) {
-					${handlerBody}
-				});
-			` );
-		}
-
-		else if ( attribute.type === 'Binding' ) {
-			addComponentBinding( generator, node, attribute, block, local );
-		}
-
-		else if ( attribute.type === 'Ref' ) {
-			generator.usesRefs = true;
-
-			local.create.addLine(
-				`${block.component}.refs.${attribute.name} = ${local.name};`
-			);
-
-			block.builders.destroy.addLine( deindent`
-				if ( ${block.component}.refs.${attribute.name} === ${local.name} ) ${block.component}.refs.${attribute.name} = null;
-			` );
-		}
-
-		else {
-			throw new Error( `Not implemented: ${attribute.type}` );
-		}
-	});
-}