refactor a lot of attribute code

src/generators/Generator.js

@@ -60,14 +60,14 @@ export default class Generator {
 		return alias;
 	}
 
-	contextualise ( fragment, expression, isEventHandler ) {
+	contextualise ( block, expression, isEventHandler ) {
 		this.addSourcemapLocations( expression );
 
 		const usedContexts = [];
 		const dependencies = [];
 
 		const { code, helpers } = this;
-		const { contextDependencies, contexts, indexes } = fragment;
+		const { contextDependencies, contexts, indexes } = block;
 
 		let scope = annotateWithScopes( expression );
 

src/generators/dom/Block.js

@@ -39,7 +39,7 @@ export default class Block {
 				`var ${name} = ${renderStatement};`
 			);
 
-			this.createMountStatement( name, parentNode );
+			this.mount( name, parentNode );
 		} else {
 			this.builders.create.addLine( `${this.generator.helper( 'appendNode' )}( ${renderStatement}, ${parentNode} );` );
 		}
@@ -58,7 +58,7 @@ export default class Block {
 		this.addElement( name, renderStatement, parentNode, true );
 	}
 
-	createMountStatement ( name, parentNode ) {
+	mount ( name, parentNode ) {
 		if ( parentNode ) {
 			this.builders.create.addLine( `${this.generator.helper( 'appendNode' )}( ${name}, ${parentNode} );` );
 		} else {

src/generators/dom/visitors/Element/Attribute.js

@@ -0,0 +1,133 @@
+import attributeLookup from './lookup.js';
+import deindent from '../../../../utils/deindent.js';
+import getStaticAttributeValue from './getStaticAttributeValue.js';
+
+export default function visitAttribute ( generator, block, state, node, attribute ) {
+	const name = attribute.name;
+
+	let metadata = state.namespace ? null : attributeLookup[ name ];
+	if ( metadata && metadata.appliesTo && !~metadata.appliesTo.indexOf( node.name ) ) metadata = null;
+
+	let dynamic = false;
+
+	const isIndirectlyBoundValue = name === 'value' && (
+		node.name === 'option' || // TODO check it's actually bound
+		node.name === 'input' && /^(checkbox|radio)$/.test( getStaticAttributeValue( node, 'type' ) )
+	);
+
+	const propertyName = isIndirectlyBoundValue ? '__value' : metadata && metadata.propertyName;
+
+	// xlink is a special case... we could maybe extend this to generic
+	// namespaced attributes but I'm not sure that's applicable in
+	// HTML5?
+	const method = name.slice( 0, 6 ) === 'xlink:' ? 'setXlinkAttribute' : 'setAttribute';
+
+	// attributes without values, e.g. <textarea readonly>
+	if ( attribute.value === true ) {
+		if ( propertyName ) {
+			block.builders.create.addLine(
+				`${state.parentNode}.${propertyName} = true;`
+			);
+		} else {
+			block.builders.create.addLine(
+				`${generator.helper( method )}( ${state.parentNode}, '${name}', true );`
+			);
+		}
+
+		// special case – autofocus. has to be handled in a bit of a weird way
+		if ( name === 'autofocus' ) {
+			block.autofocus = state.parentNode;
+		}
+	}
+
+	// empty string
+	else if ( attribute.value.length === 0 ) {
+		if ( propertyName ) {
+			block.builders.create.addLine(
+				`${state.parentNode}.${propertyName} = '';`
+			);
+		} else {
+			block.builders.create.addLine(
+				`${generator.helper( method )}( ${state.parentNode}, '${name}', '' );`
+			);
+		}
+	}
+
+	// static text or a single {{tag}}
+	else if ( attribute.value.length === 1 ) {
+		const value = attribute.value[0];
+
+		if ( value.type === 'Text' ) {
+			// static attributes
+			const result = JSON.stringify( value.data );
+
+			if ( propertyName ) {
+				block.builders.create.addLine(
+					`${state.parentNode}.${propertyName} = ${result};`
+				);
+			} else {
+				if ( name === 'xmlns' ) {
+					// special case
+					// TODO this attribute must be static – enforce at compile time
+					state.namespace = value.data;
+				}
+
+				block.builders.create.addLine(
+					`${generator.helper( method )}( ${state.parentNode}, '${name}', ${result} );`
+				);
+			}
+		}
+
+		else {
+			dynamic = true;
+
+			// dynamic – but potentially non-string – attributes
+			const { snippet } = generator.contextualise( block, value.expression );
+
+			const last = `last_${state.parentNode}_${name.replace( /-/g, '_')}`;
+			block.builders.create.addLine( `var ${last} = ${snippet};` );
+
+			const updater = propertyName ?
+				`${state.parentNode}.${propertyName} = ${last};` :
+				`${generator.helper( method )}( ${state.parentNode}, '${name}', ${last} );`;
+
+			block.builders.create.addLine( updater );
+
+			block.builders.update.addBlock( deindent`
+				if ( ( ${block.tmp()} = ${snippet} ) !== ${last} ) {
+					${last} = ${block.tmp()};
+					${updater}
+				}
+			` );
+		}
+	}
+
+	else {
+		dynamic = true;
+
+		const value = ( attribute.value[0].type === 'Text' ? '' : `"" + ` ) + (
+			attribute.value.map( chunk => {
+				if ( chunk.type === 'Text' ) {
+					return JSON.stringify( chunk.data );
+				} else {
+					const { snippet } = generator.contextualise( block, chunk.expression );
+					return `( ${snippet} )`;
+				}
+			}).join( ' + ' )
+		);
+
+		const updater = propertyName ?
+			`${state.parentNode}.${propertyName} = ${value};` :
+			`${generator.helper( method )}( ${state.parentNode}, '${name}', ${value} );`;
+
+		block.builders.create.addLine( updater );
+		block.builders.update.addLine( updater );
+	}
+
+	if ( isIndirectlyBoundValue ) {
+		const updateValue = `${state.parentNode}.value = ${state.parentNode}.__value;`;
+
+		block.builders.create.addLine( updateValue );
+		if ( dynamic ) block.builders.update.addLine( updateValue );
+	}
+}

src/generators/dom/visitors/Element/Binding.js

@@ -1,16 +1,16 @@
-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';
 import getStaticAttributeValue from './getStaticAttributeValue.js';
 
-export default function addElementBinding ( generator, node, block, state, attribute, local ) {
+export default function visitBinding ( generator, block, state, node, attribute ) {
 	const { name, keypath } = flattenReference( attribute.value );
 	const { snippet, contexts, dependencies } = generator.contextualise( block, attribute.value );
 
 	if ( dependencies.length > 1 ) throw new Error( 'An unexpected situation arose. Please raise an issue at https://github.com/sveltejs/svelte/issues — thanks!' );
 
 	contexts.forEach( context => {
-		if ( !~local.allUsedContexts.indexOf( context ) ) local.allUsedContexts.push( context );
+		if ( !~state.allUsedContexts.indexOf( context ) ) state.allUsedContexts.push( context );
 	});
 
 	const handler = block.getUniqueName( `${state.parentNode}_change_handler` );
@@ -18,7 +18,7 @@ export default function addElementBinding ( generator, node, block, state, attri
 	const isMultipleSelect = node.name === 'select' && node.attributes.find( attr => attr.name.toLowerCase() === 'multiple' ); // TODO use getStaticAttributeValue
 	const type = getStaticAttributeValue( node, 'type' );
 	const bindingGroup = attribute.name === 'group' ? getBindingGroup( generator, keypath ) : null;
-	const value = getBindingValue( generator, block, state, local, node, attribute, isMultipleSelect, bindingGroup, type );
+	const value = getBindingValue( generator, block, state, node, attribute, isMultipleSelect, bindingGroup, type );
 	const eventName = getBindingEventName( node );
 
 	let setter = getSetter({ block, name, keypath, context: '__svelte', attribute, dependencies, value });
@@ -124,7 +124,7 @@ function getBindingEventName ( node ) {
 	return 'change';
 }
 
-function getBindingValue ( generator, block, state, local, node, attribute, isMultipleSelect, bindingGroup, type ) {
+function getBindingValue ( generator, block, state, node, attribute, isMultipleSelect, bindingGroup, type ) {
 	// <select multiple bind:value='selected>
 	if ( isMultipleSelect ) {
 		return `[].map.call( ${state.parentNode}.selectedOptions, function ( option ) { return option.__value; })`;

src/generators/dom/visitors/Element/Element.js

@@ -1,13 +1,30 @@
 import deindent from '../../../../utils/deindent.js';
 import visit from '../../visit.js';
-import addElementAttributes from './attributes/addElementAttributes.js';
 import visitComponent from '../Component/Component.js';
 import visitWindow from './meta/Window.js';
+import visitAttribute from './Attribute.js';
+import visitEventHandler from './EventHandler.js';
+import visitBinding from './Binding.js';
+import visitRef from './Ref.js';
 
 const meta = {
 	':Window': visitWindow
 };
 
+const order = {
+	Attribute: 1,
+	EventHandler: 2,
+	Binding: 3,
+	Ref: 4
+};
+
+const visitors = {
+	Attribute: visitAttribute,
+	EventHandler: visitEventHandler,
+	Binding: visitBinding,
+	Ref: visitRef
+};
+
 export default function visitElement ( generator, block, state, node ) {
 	if ( node.name in meta ) {
 		return meta[ node.name ]( generator, block, node );
@@ -22,10 +39,12 @@ export default function visitElement ( generator, block, state, node ) {
 	const childState = Object.assign( {}, state, {
 		isTopLevel: false,
 		parentNode: name,
-		namespace: node.name === 'svg' ? 'http://www.w3.org/2000/svg' : state.namespace
+		namespace: node.name === 'svg' ? 'http://www.w3.org/2000/svg' : state.namespace,
+		allUsedContexts: []
 	});
 
 	block.builders.create.addLine( `var ${name} = ${getRenderStatement( generator, childState.namespace, node.name )};` );
+	block.mount( name, state.parentNode );
 
 	if ( !state.parentNode ) {
 		block.builders.detach.addLine( `${generator.helper( 'detachNode' )}( ${name} );` );
@@ -36,14 +55,25 @@ export default function visitElement ( generator, block, state, node ) {
 		block.builders.create.addLine( `${generator.helper( 'setAttribute' )}( ${name}, '${generator.cssId}', '' );` );
 	}
 
-	const local = {
-		allUsedContexts: []
-	};
+	node.attributes
+		.sort( ( a, b ) => order[ a.type ] - order[ b.type ] )
+		.forEach( attribute => {
+			visitors[ attribute.type ]( generator, block, childState, node, attribute );
+		});
 
-	addElementAttributes( generator, block, childState, node, local );
+	// special case – bound <option> without a value attribute
+	if ( node.name === 'option' && !node.attributes.find( attribute => attribute.type === 'Attribute' && attribute.name === 'value' ) ) { 	// TODO check it's bound
+		const statement = `${name}.__value = ${name}.textContent;`;
+		block.builders.update.addLine( statement );
+		node.initialUpdate = statement;
+	}
+
+	if ( node.initialUpdate ) {
+		block.builders.create.addBlock( node.initialUpdate );
+	}
 
-	if ( local.allUsedContexts.length ) {
-		const initialProps = local.allUsedContexts.map( contextName => {
+	if ( childState.allUsedContexts.length ) {
+		const initialProps = childState.allUsedContexts.map( contextName => {
 			if ( contextName === 'root' ) return `root: root`;
 
 			const listName = block.listNames.get( contextName );
@@ -52,7 +82,7 @@ export default function visitElement ( generator, block, state, node ) {
 			return `${listName}: ${listName},\n${indexName}: ${indexName}`;
 		}).join( ',\n' );
 
-		const updates = local.allUsedContexts.map( contextName => {
+		const updates = childState.allUsedContexts.map( contextName => {
 			if ( contextName === 'root' ) return `${name}.__svelte.root = root;`;
 
 			const listName = block.listNames.get( contextName );
@@ -70,30 +100,17 @@ export default function visitElement ( generator, block, state, node ) {
 		block.builders.update.addBlock( updates );
 	}
 
-	// special case – bound <option> without a value attribute
-	if ( node.name === 'option' && !node.attributes.find( attribute => attribute.type === 'Attribute' && attribute.name === 'value' ) ) { 	// TODO check it's bound
-		const statement = `${name}.__value = ${name}.textContent;`;
-		block.builders.update.addLine( statement );
-		node.initialUpdate = statement;
-	}
-
-	block.createMountStatement( name, state.parentNode );
-
 	node.children.forEach( child => {
 		visit( generator, block, childState, child );
 	});
-
-	if ( node.initialUpdate ) {
-		block.builders.create.addBlock( node.initialUpdate );
-	}
 }
 
 function getRenderStatement ( generator, namespace, name ) {
-	if ( namespace ) {
 	if ( namespace === 'http://www.w3.org/2000/svg' ) {
 		return `${generator.helper( 'createSvgElement' )}( '${name}' )`;
 	}
 
+	if ( namespace ) {
 		return `document.createElementNS( '${namespace}', '${name}' )`;
 	}
 

src/generators/dom/visitors/Element/EventHandler.js

@@ -0,0 +1,62 @@
+import deindent from '../../../../utils/deindent.js';
+import flattenReference from '../../../../utils/flattenReference.js';
+
+export default function visitEventHandler ( generator, block, state, node, attribute ) {
+	const name = attribute.name;
+
+	// TODO verify that it's a valid callee (i.e. built-in or declared method)
+	generator.addSourcemapLocations( attribute.expression );
+
+	const flattened = flattenReference( attribute.expression.callee );
+	if ( flattened.name !== 'event' && flattened.name !== 'this' ) {
+		// allow event.stopPropagation(), this.select() etc
+		generator.code.prependRight( attribute.expression.start, `${block.component}.` );
+	}
+
+	const usedContexts = [];
+	attribute.expression.arguments.forEach( arg => {
+		const { contexts } = generator.contextualise( block, arg, true );
+
+		contexts.forEach( context => {
+			if ( !~usedContexts.indexOf( context ) ) usedContexts.push( context );
+			if ( !~state.allUsedContexts.indexOf( context ) ) state.allUsedContexts.push( context );
+		});
+	});
+
+	// TODO hoist event handlers? can do `this.__component.method(...)`
+	const declarations = usedContexts.map( name => {
+		if ( name === 'root' ) return 'var root = this.__svelte.root;';
+
+		const listName = block.listNames.get( name );
+		const indexName = block.indexNames.get( name );
+
+		return `var ${listName} = this.__svelte.${listName}, ${indexName} = this.__svelte.${indexName}, ${name} = ${listName}[${indexName}]`;
+	});
+
+	const handlerName = block.getUniqueName( `${name}_handler` );
+	const handlerBody = ( declarations.length ? declarations.join( '\n' ) + '\n\n' : '' ) + `[✂${attribute.expression.start}-${attribute.expression.end}✂];`;
+
+	if ( generator.events.has( name ) ) {
+		block.builders.create.addBlock( deindent`
+			var ${handlerName} = ${generator.alias( 'template' )}.events.${name}.call( ${block.component}, ${state.parentNode}, function ( event ) {
+				${handlerBody}
+			}.bind( ${state.parentNode} ) );
+		` );
+
+		block.builders.destroy.addLine( deindent`
+			${handlerName}.teardown();
+		` );
+	} else {
+		block.builders.create.addBlock( deindent`
+			function ${handlerName} ( event ) {
+				${handlerBody}
+			}
+
+			${generator.helper( 'addEventListener' )}( ${state.parentNode}, '${name}', ${handlerName} );
+		` );
+
+		block.builders.destroy.addLine( deindent`
+			${generator.helper( 'removeEventListener' )}( ${state.parentNode}, '${name}', ${handlerName} );
+		` );
+	}
+}

src/generators/dom/visitors/Element/Ref.js

@@ -0,0 +1,15 @@
+import deindent from '../../../../utils/deindent.js';
+
+export default function visitRef ( generator, block, state, node, attribute ) {
+	const name = attribute.name;
+
+	block.builders.create.addLine(
+		`${block.component}.refs.${name} = ${state.parentNode};`
+	);
+
+	block.builders.destroy.addLine( deindent`
+		if ( ${block.component}.refs.${name} === ${state.parentNode} ) ${block.component}.refs.${name} = null;
+	` );
+
+	generator.usesRefs = true; // so this component.refs object is created
+}

src/generators/dom/visitors/Element/attributes/addElementAttributes.js

@@ -1,229 +0,0 @@
-import attributeLookup from './lookup.js';
-import addElementBinding from './addElementBinding';
-import deindent from '../../../../../utils/deindent.js';
-import flattenReference from '../../../../../utils/flattenReference.js';
-import getStaticAttributeValue from './getStaticAttributeValue.js';
-
-export default function addElementAttributes ( generator, block, state, node, local ) {
-	node.attributes.forEach( attribute => {
-		const name = attribute.name;
-
-		if ( attribute.type === 'Attribute' ) {
-			let metadata = state.namespace ? null : attributeLookup[ name ];
-			if ( metadata && metadata.appliesTo && !~metadata.appliesTo.indexOf( node.name ) ) metadata = null;
-
-			let dynamic = false;
-
-			const isIndirectlyBoundValue = name === 'value' && (
-				node.name === 'option' || // TODO check it's actually bound
-				node.name === 'input' && /^(checkbox|radio)$/.test( getStaticAttributeValue( node, 'type' ) )
-			);
-
-			const propertyName = isIndirectlyBoundValue ? '__value' : metadata && metadata.propertyName;
-
-			const isXlink = name.slice( 0, 6 ) === 'xlink:';
-
-			// xlink is a special case... we could maybe extend this to generic
-			// namespaced attributes but I'm not sure that's applicable in
-			// HTML5?
-			const method = isXlink ? 'setXlinkAttribute' : 'setAttribute';
-
-			if ( attribute.value === true ) {
-				// attributes without values, e.g. <textarea readonly>
-				if ( propertyName ) {
-					block.builders.create.addLine(
-						`${state.parentNode}.${propertyName} = true;`
-					);
-				} else {
-					block.builders.create.addLine(
-						`${generator.helper( method )}( ${state.parentNode}, '${name}', true );`
-					);
-				}
-
-				// special case – autofocus. has to be handled in a bit of a weird way
-				if ( name === 'autofocus' ) {
-					block.autofocus = state.parentNode;
-				}
-			}
-
-			else if ( attribute.value.length === 0 ) {
-				if ( propertyName ) {
-					block.builders.create.addLine(
-						`${state.parentNode}.${propertyName} = '';`
-					);
-				} else {
-					block.builders.create.addLine(
-						`${generator.helper( method )}( ${state.parentNode}, '${name}', '' );`
-					);
-				}
-			}
-
-			else if ( attribute.value.length === 1 ) {
-				const value = attribute.value[0];
-
-				let result = '';
-
-				if ( value.type === 'Text' ) {
-					// static attributes
-					result = JSON.stringify( value.data );
-
-					let addAttribute = false;
-					if ( name === 'xmlns' ) {
-						// special case
-						// TODO this attribute must be static – enforce at compile time
-						state.namespace = value.data;
-						addAttribute = true;
-					} else if ( propertyName ) {
-						block.builders.create.addLine(
-							`${state.parentNode}.${propertyName} = ${result};`
-						);
-					} else {
-						addAttribute = true;
-					}
-
-					if ( addAttribute ) {
-						block.builders.create.addLine(
-							`${generator.helper( method )}( ${state.parentNode}, '${name}', ${result} );`
-						);
-					}
-				}
-
-				else {
-					dynamic = true;
-
-					// dynamic – but potentially non-string – attributes
-					const { snippet } = generator.contextualise( block, value.expression );
-
-					const last = `last_${state.parentNode}_${name.replace( /-/g, '_')}`;
-					block.builders.create.addLine( `var ${last} = ${snippet};` );
-
-					let updater;
-					if ( propertyName ) {
-						updater = `${state.parentNode}.${propertyName} = ${last};`;
-					} else {
-						updater = `${generator.helper( method )}( ${state.parentNode}, '${name}', ${last} );`;
-					}
-
-					block.builders.create.addLine( updater );
-
-					block.builders.update.addBlock( deindent`
-						if ( ( ${block.tmp()} = ${snippet} ) !== ${last} ) {
-							${last} = ${block.tmp()};
-							${updater}
-						}
-					` );
-				}
-			}
-
-			else {
-				dynamic = true;
-
-				const value = ( attribute.value[0].type === 'Text' ? '' : `"" + ` ) + (
-					attribute.value.map( chunk => {
-						if ( chunk.type === 'Text' ) {
-							return JSON.stringify( chunk.data );
-						} else {
-							const { snippet } = generator.contextualise( block, chunk.expression );
-							return `( ${snippet} )`;
-						}
-					}).join( ' + ' )
-				);
-
-				let updater;
-				if (propertyName) {
-					updater = `${state.parentNode}.${propertyName} = ${value};`;
-				} else {
-					updater = `${generator.helper( method )}( ${state.parentNode}, '${name}', ${value} );`;
-				}
-
-				block.builders.create.addLine( updater );
-				block.builders.update.addLine( updater );
-			}
-
-			if ( isIndirectlyBoundValue ) {
-				const updateValue = `${state.parentNode}.value = ${state.parentNode}.__value;`;
-
-				block.builders.create.addLine( updateValue );
-				if ( dynamic ) block.builders.update.addLine( updateValue );
-			}
-		}
-
-		else if ( attribute.type === 'EventHandler' ) {
-			// TODO verify that it's a valid callee (i.e. built-in or declared method)
-			generator.addSourcemapLocations( attribute.expression );
-
-			const flattened = flattenReference( attribute.expression.callee );
-			if ( flattened.name !== 'event' && flattened.name !== 'this' ) {
-				// allow event.stopPropagation(), this.select() etc
-				generator.code.prependRight( attribute.expression.start, `${block.component}.` );
-			}
-
-			const usedContexts = [];
-			attribute.expression.arguments.forEach( arg => {
-				const { contexts } = generator.contextualise( block, arg, 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.__svelte.root;';
-
-				const listName = block.listNames.get( name );
-				const indexName = block.indexNames.get( name );
-
-				return `var ${listName} = this.__svelte.${listName}, ${indexName} = this.__svelte.${indexName}, ${name} = ${listName}[${indexName}]`;
-			});
-
-			const handlerName = block.getUniqueName( `${name}_handler` );
-			const handlerBody = ( declarations.length ? declarations.join( '\n' ) + '\n\n' : '' ) + `[✂${attribute.expression.start}-${attribute.expression.end}✂];`;
-
-			if ( generator.events.has( name ) ) {
-				block.builders.create.addBlock( deindent`
-					var ${handlerName} = ${generator.alias( 'template' )}.events.${name}.call( ${block.component}, ${state.parentNode}, function ( event ) {
-						${handlerBody}
-					}.bind( ${state.parentNode} ) );
-				` );
-
-				block.builders.destroy.addLine( deindent`
-					${handlerName}.teardown();
-				` );
-			} else {
-				block.builders.create.addBlock( deindent`
-					function ${handlerName} ( event ) {
-						${handlerBody}
-					}
-
-					${generator.helper( 'addEventListener' )}( ${state.parentNode}, '${name}', ${handlerName} );
-				` );
-
-				block.builders.destroy.addLine( deindent`
-					${generator.helper( 'removeEventListener' )}( ${state.parentNode}, '${name}', ${handlerName} );
-				` );
-			}
-		}
-
-		else if ( attribute.type === 'Binding' ) {
-			addElementBinding( generator, node, block, state, attribute, local );
-		}
-
-		else if ( attribute.type === 'Ref' ) {
-			generator.usesRefs = true;
-
-			block.builders.create.addLine(
-				`${block.component}.refs.${name} = ${state.parentNode};`
-			);
-
-			block.builders.destroy.addLine( deindent`
-				if ( ${block.component}.refs.${name} === ${state.parentNode} ) ${block.component}.refs.${name} = null;
-			` );
-		}
-
-		else {
-			throw new Error( `Not implemented: ${attribute.type}` );
-		}
-	});
-}