hoist some event handlers, rewrite non-hoisted ones to avoid binding

src/generators/Generator.js

@@ -60,7 +60,7 @@ export default class Generator {
 		return alias;
 	}
 
-	contextualise ( block, expression, isEventHandler ) {
+	contextualise ( block, expression, context, isEventHandler ) {
 		this.addSourcemapLocations( expression );
 
 		const usedContexts = [];
@@ -70,17 +70,24 @@ export default class Generator {
 		const { contextDependencies, contexts, indexes } = block;
 
 		let scope = annotateWithScopes( expression );
+		let lexicalDepth = 0;
 
 		const self = this;
 
 		walk( expression, {
 			enter ( node, parent, key ) {
+				if ( /^Function/.test( node.type ) ) lexicalDepth += 1;
+
 				if ( node._scope ) {
 					scope = node._scope;
 					return;
 				}
 
-				if ( isReference( node, parent ) ) {
+				if ( node.type === 'ThisExpression' ) {
+					if ( lexicalDepth === 0 && context ) code.overwrite( node.start, node.end, context, true );
+				}
+
+				else if ( isReference( node, parent ) ) {
 					const { name } = flattenReference( node );
 					if ( scope.has( name ) ) return;
 
@@ -93,10 +100,10 @@ export default class Generator {
 					}
 
 					else if ( contexts.has( name ) ) {
-						const context = contexts.get( name );
+						const contextName = contexts.get( name );
-						if ( context !== name ) {
+						if ( contextName !== name ) {
 							// this is true for 'reserved' names like `root` and `component`
-							code.overwrite( node.start, node.start + name.length, context, true );
+							code.overwrite( node.start, node.start + name.length, contextName, true );
 						}
 
 						dependencies.push( ...contextDependencies.get( name ) );
@@ -133,6 +140,7 @@ export default class Generator {
 			},
 
 			leave ( node ) {
+				if ( /^Function/.test( node.type ) ) lexicalDepth -= 1;
 				if ( node._scope ) scope = scope.parent;
 			}
 		});

src/generators/dom/Block.js

@@ -53,6 +53,10 @@ export default class Block {
 		return new Block( Object.assign( {}, this, options, { parent: this } ) );
 	}
 
+	contextualise ( expression, context, isEventHandler ) {
+		return this.generator.contextualise( this, expression, context, isEventHandler );
+	}
+
 	createAnchor ( name, parentNode ) {
 		const renderStatement = `${this.generator.helper( 'createComment' )}()`;
 		this.addElement( name, renderStatement, parentNode, true );

src/generators/dom/index.js

@@ -271,7 +271,7 @@ export default function dom ( parsed, source, options ) {
 			throw new Error( `Components with shared helpers must be compiled to ES2015 modules (format: 'es')` );
 		}
 
-		const names = Array.from( generator.uses ).map( name => {
+		const names = Array.from( generator.uses ).sort().map( name => {
 			return name !== generator.alias( name ) ? `${name} as ${generator.alias( name )}` : name;
 		});
 

src/generators/dom/visitors/Component/attributes/addComponentAttributes.js

@@ -82,7 +82,7 @@ export default function addComponentAttributes ( generator, block, node, local )
 
 			const usedContexts = [];
 			attribute.expression.arguments.forEach( arg => {
-				const { contexts } = generator.contextualise( block, arg, true );
+				const { contexts } = generator.contextualise( block, arg, null, true );
 
 				contexts.forEach( context => {
 					if ( !~usedContexts.indexOf( context ) ) usedContexts.push( context );

src/generators/dom/visitors/EachBlock.js

@@ -134,7 +134,7 @@ export default function visitEachBlock ( generator, block, state, node ) {
 				}
 			}
 
-			destroyEach( ${localVars.iterations}, true, ${listName}.length );
+			${generator.helper( 'destroyEach' )}( ${localVars.iterations}, true, ${listName}.length );
 
 			${localVars.iterations}.length = ${listName}.length;
 		` );
@@ -154,7 +154,7 @@ export default function visitEachBlock ( generator, block, state, node ) {
 	}
 
 	block.builders.destroy.addBlock(
-		`${generator.helper( 'destroyEach' )}( ${localVars.iterations}, ${isToplevel ? 'detach' : 'false'} );` );
+		`${generator.helper( 'destroyEach' )}( ${localVars.iterations}, ${isToplevel ? 'detach' : 'false'}, 0 );` );
 
 	if ( node.else ) {
 		block.builders.destroy.addBlock( deindent`
@@ -197,7 +197,8 @@ export default function visitEachBlock ( generator, block, state, node ) {
 	});
 
 	const childState = Object.assign( {}, state, {
-		parentNode: null
+		parentNode: null,
+		inEachBlock: true
 	});
 
 	node.children.forEach( child => {

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

@@ -21,7 +21,7 @@ export default function visitBinding ( generator, block, state, node, attribute
 	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 });
+	let setter = getSetter({ block, name, keypath, context: '_svelte', attribute, dependencies, value });
 	let updateElement;
 
 	// <select> special case

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

@@ -72,32 +72,35 @@ export default function visitElement ( generator, block, state, node ) {
 		block.builders.create.addBlock( node.initialUpdate );
 	}
 
-	if ( childState.allUsedContexts.length ) {
+	if ( childState.allUsedContexts.length || childState.usesComponent ) {
-		const initialProps = childState.allUsedContexts.map( contextName => {
+		const initialProps = [];
-			if ( contextName === 'root' ) return `root: root`;
+		const updates = [];
 
-			const listName = block.listNames.get( contextName );
+		if ( childState.usesComponent ) {
-			const indexName = block.indexNames.get( contextName );
+			initialProps.push( `component: ${block.component}` );
-
+		}
-			return `${listName}: ${listName},\n${indexName}: ${indexName}`;
-		}).join( ',\n' );
 
-		const updates = childState.allUsedContexts.map( contextName => {
+		childState.allUsedContexts.forEach( contextName => {
-			if ( contextName === 'root' ) return `${name}.__svelte.root = root;`;
+			if ( contextName === 'root' ) return;
 
 			const listName = block.listNames.get( contextName );
 			const indexName = block.indexNames.get( contextName );
 
-			return `${name}.__svelte.${listName} = ${listName};\n${name}.__svelte.${indexName} = ${indexName};`;
+			initialProps.push( `${listName}: ${listName},\n${indexName}: ${indexName}` );
-		}).join( '\n' );
+			updates.push( `${name}._svelte.${listName} = ${listName};\n${name}._svelte.${indexName} = ${indexName};` );
-
+		});
-		block.builders.create.addBlock( deindent`
-			${name}.__svelte = {
-				${initialProps}
-			};
-		` );
 
-		block.builders.update.addBlock( updates );
+		if ( initialProps.length ) {
+			block.builders.create.addBlock( deindent`
+				${name}._svelte = {
+					${initialProps.join( ',\n' )}
+				};
+			` );
+		}
+
+		if ( updates.length ) {
+			block.builders.update.addBlock( updates.join( '\n' ) );
+		}
 	}
 
 	node.children.forEach( child => {

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

@@ -1,21 +1,26 @@
 import deindent from '../../../../utils/deindent.js';
+import CodeBuilder from '../../../../utils/CodeBuilder.js';
 import flattenReference from '../../../../utils/flattenReference.js';
 
 export default function visitEventHandler ( generator, block, state, node, attribute ) {
 	const name = attribute.name;
+	const isCustomEvent = generator.events.has( name );
+	const shouldHoist = !isCustomEvent && state.inEachBlock;
 
-	// 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
+		// TODO verify that it's a valid callee (i.e. built-in or declared method)
 		generator.code.prependRight( attribute.expression.start, `${block.component}.` );
+		if ( shouldHoist ) state.usesComponent = true; // this feels a bit hacky but it works!
 	}
 
+	const context = shouldHoist ? null : state.parentNode;
 	const usedContexts = [];
 	attribute.expression.arguments.forEach( arg => {
-		const { contexts } = generator.contextualise( block, arg, true );
+		const { contexts } = block.contextualise( arg, context, true );
 
 		contexts.forEach( context => {
 			if ( !~usedContexts.indexOf( context ) ) usedContexts.push( context );
@@ -23,35 +28,65 @@ export default function visitEventHandler ( generator, block, state, node, attri
 		});
 	});
 
-	// TODO hoist event handlers? can do `this.__component.method(...)`
+	const _this = context || 'this';
 	const declarations = usedContexts.map( name => {
-		if ( name === 'root' ) return 'var root = this.__svelte.root;';
+		if ( name === 'root' ) {
+			if ( shouldHoist ) state.usesComponent = true;
+			return 'var root = component.get();';
+		}
 
 		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}]`;
+		return `var ${listName} = ${_this}._svelte.${listName}, ${indexName} = ${_this}._svelte.${indexName}, ${name} = ${listName}[${indexName}];`;
 	});
 
-	const handlerName = block.getUniqueName( `${name}_handler` );
+	// get a name for the event handler that is globally unique
-	const handlerBody = ( declarations.length ? declarations.join( '\n' ) + '\n\n' : '' ) + `[✂${attribute.expression.start}-${attribute.expression.end}✂];`;
+	// if hoisted, locally unique otherwise
+	const handlerName = shouldHoist ?
+		generator.alias( `${name}_handler` ) :
+		block.getUniqueName( `${name}_handler` );
 
-	if ( generator.events.has( name ) ) {
+	// create the handler body
-		block.builders.create.addBlock( deindent`
+	const handlerBody = new CodeBuilder();
+
+	if ( state.usesComponent ) {
+		// TODO the element needs to know to create `thing._svelte = { component: component }`
+		handlerBody.addLine( `var component = this._svelte.component;` );
+	}
+
+	declarations.forEach( declaration => {
+		handlerBody.addLine( declaration );
+	});
+
+	handlerBody.addLine( `[✂${attribute.expression.start}-${attribute.expression.end}✂];` );
+
+	const handler = isCustomEvent ?
+		deindent`
 			var ${handlerName} = ${generator.alias( 'template' )}.events.${name}.call( ${block.component}, ${state.parentNode}, function ( event ) {
 				${handlerBody}
-			}.bind( ${state.parentNode} ) );
+			});
-		` );
+		` :
+		deindent`
+			function ${handlerName} ( event ) {
+				${handlerBody}
+			}
+		`;
+
+	if ( shouldHoist ) {
+		generator.addBlock({
+			render: () => handler
+		});
+	} else {
+		block.builders.create.addBlock( handler );
+	}
 
+	if ( isCustomEvent ) {
 		block.builders.destroy.addLine( deindent`
 			${handlerName}.teardown();
 		` );
 	} else {
-		block.builders.create.addBlock( deindent`
+		block.builders.create.addLine( deindent`
-			function ${handlerName} ( event ) {
-				${handlerBody}
-			}
-
 			${generator.helper( 'addEventListener' )}( ${state.parentNode}, '${name}', ${handlerName} );
 		` );
 

src/shared/dom.js

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

test/js/samples/event-handlers-custom/expected.js

@@ -1,4 +1,4 @@
-import { createElement, detachNode, insertNode, createText, appendNode, assign, dispatchObservers, noop, proto } from "svelte/shared.js";
+import { appendNode, assign, createElement, createText, detachNode, dispatchObservers, insertNode, noop, proto } from "svelte/shared.js";
 
 var template = (function () {
 	return {
@@ -7,7 +7,6 @@ var template = (function () {
 				console.log( bar );
 			}
 		},
-
 		events: {
 			foo ( node, callback ) {
 				// code goes here
@@ -21,7 +20,6 @@ function create_main_fragment ( root, component ) {
 
 	var foo_handler = template.events.foo.call( component, button, function ( event ) {
 		var root = component.get();
-
 		component.foo( root.bar );
 	});
 

test/runtime/samples/event-handler-custom-each/_config.js

@@ -0,0 +1,32 @@
+export default {
+	html: `
+		<button>foo</button>
+		<button>bar</button>
+		<button>baz</button>
+
+		<p>fromDom: </p>
+		<p>fromState: </p>
+	`,
+
+	test ( assert, component, target, window ) {
+		const event = new window.MouseEvent( 'click' );
+
+		const buttons = target.querySelectorAll( 'button' );
+
+		buttons[1].dispatchEvent( event );
+
+		assert.htmlEqual( target.innerHTML, `
+			<button>foo</button>
+			<button>bar</button>
+			<button>baz</button>
+
+			<p>fromDom: bar</p>
+			<p>fromState: bar</p>
+		` );
+
+		assert.equal( component.get( 'fromDom' ), 'bar' );
+		assert.equal( component.get( 'fromState' ), 'bar' );
+
+		component.destroy();
+	}
+};

test/runtime/samples/event-handler-custom-each/main.html

@@ -0,0 +1,34 @@
+{{#each items as item}}
+	<button on:tap='set({ fromDom: this.textContent, fromState: item })'>{{item}}</button>
+{{/each}}
+
+<p>fromDom: {{fromDom}}</p>
+<p>fromState: {{fromState}}</p>
+
+<script>
+	export default {
+		data: () => ({
+			x: 0,
+			y: 0,
+			fromDom: '',
+			fromState: '',
+			items: [ 'foo', 'bar', 'baz' ]
+		}),
+
+		events: {
+			tap ( node, callback ) {
+				function clickHandler ( event ) {
+					callback();
+				}
+
+				node.addEventListener( 'click', clickHandler, false );
+
+				return {
+					teardown () {
+						node.addEventListener( 'click', clickHandler, false );
+					}
+				};
+			}
+		}
+	};
+</script>

test/runtime/samples/event-handler-each/_config.js

@@ -0,0 +1,32 @@
+export default {
+	data: {
+		items: [
+			'foo',
+			'bar',
+			'baz'
+		],
+		selected: 'foo'
+	},
+
+	html: `
+		<button>foo</button>
+		<button>bar</button>
+		<button>baz</button>
+		<p>selected: foo</p>
+	`,
+
+	test ( assert, component, target, window ) {
+		const buttons = target.querySelectorAll( 'button' );
+		const event = new window.MouseEvent( 'click' );
+
+		buttons[1].dispatchEvent( event );
+		assert.htmlEqual( target.innerHTML, `
+			<button>foo</button>
+			<button>bar</button>
+			<button>baz</button>
+			<p>selected: bar</p>
+		` );
+
+		component.destroy();
+	}
+};

test/runtime/samples/event-handler-each/main.html

@@ -0,0 +1,5 @@
+{{#each items as item}}
+	<button on:click='set({ selected: item })'>{{item}}</button>
+{{/each}}
+
+<p>selected: {{selected}}</p>

test/runtime/samples/event-handler/_config.js

@@ -1,13 +1,21 @@
 export default {
-	html: '<button>toggle</button>\n\n<!---->',
+	html: `
+		<button>toggle</button>
+	`,
+
 	test ( assert, component, target, window ) {
 		const button = target.querySelector( 'button' );
 		const event = new window.MouseEvent( 'click' );
 
 		button.dispatchEvent( event );
-		assert.equal( target.innerHTML, '<button>toggle</button>\n\n<p>hello!</p><!---->' );
+		assert.htmlEqual( target.innerHTML, `
+			<button>toggle</button>
+			<p>hello!</p>
+		` );
 
 		button.dispatchEvent( event );
-		assert.equal( target.innerHTML, '<button>toggle</button>\n\n<!---->' );
+		assert.htmlEqual( target.innerHTML, `
+			<button>toggle</button>
+		` );
 	}
 };