hoist variables where appropriate (#504)

Author: Rich-Harris

src/generators/dom/Block.js

@@ -30,6 +30,7 @@ export default class Block {
 		};
 
 		this.aliases = new Map();
+		this.variables = new Map();
 		this.getUniqueName = this.generator.getUniqueNameMaker( options.params );
 
 		// unique names
@@ -62,6 +63,14 @@ export default class Block {
 		}
 	}
 
+	addVariable ( name, init ) {
+		if ( this.variables.has( name ) && this.variables.get( name ) !== init ) {
+			throw new Error( `Variable '${name}' already initialised with a different value` );
+		}
+
+		this.variables.set( name, init );
+	}
+
 	alias ( name ) {
 		if ( !this.aliases.has( name ) ) {
 			this.aliases.set( name, this.getUniqueName( name ) );
@@ -96,6 +105,17 @@ export default class Block {
 	}
 
 	render () {
+		if ( this.variables.size ) {
+			const variables = Array.from( this.variables.keys() )
+				.map( key => {
+					const init = this.variables.get( key );
+					return init !== undefined ? `${key} = ${init}` : key;
+				})
+				.join( ', ' );
+
+			this.builders.create.addBlockAtStart( `var ${variables};` );
+		}
+
 		if ( this.autofocus ) {
 			this.builders.create.addLine( `${this.autofocus}.focus();` );
 		}
@@ -134,7 +154,6 @@ export default class Block {
 			if ( this.builders.update.isEmpty() ) {
 				properties.addBlock( `update: ${this.generator.helper( 'noop' )},` );
 			} else {
-				if ( this._tmp ) this.builders.update.addBlockAtStart( `var ${this._tmp};` );
 				properties.addBlock( deindent`
 					update: function ( changed, ${this.params.join( ', ' )} ) {
 						${this.builders.update}

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

@@ -40,10 +40,9 @@ export default function visitBinding ( generator, block, state, node, attribute,
 	generator.hasComplexBindings = true;
 
 	const updating = block.getUniqueName( `${local.name}_updating` );
+	block.addVariable( updating, 'false' );
 
 	local.create.addBlock( deindent`
-		var ${updating} = false;
-
 		${block.component}._bindings.push( function () {
 			if ( ${local.name}._torndown ) return;
 			${local.name}.observe( '${attribute.name}', function ( value ) {

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

@@ -44,7 +44,7 @@ export default function visitAttribute ( generator, block, state, node, attribut
 		}
 
 		const last = block.getUniqueName( `${state.parentNode}_${name.replace( /[^a-zA-Z_$]/g, '_')}_value` );
-		block.builders.create.addLine( `var ${last} = ${value};` );
+		block.addVariable( last );
 
 		const isSelectValueAttribute = name === 'value' && state.parentNodeName === 'select';
 
@@ -66,20 +66,25 @@ export default function visitAttribute ( generator, block, state, node, attribut
 					}`;
 
 			updater = deindent`
-				var ${last} = ${last};
 				for ( var ${i} = 0; ${i} < ${state.parentNode}.options.length; ${i} += 1 ) {
 					var ${option} = ${state.parentNode}.options[${i}];
 
 					${ifStatement}
 				}
 			`;
+
+			block.builders.create.addLine( deindent`
+				${last} = ${value}
+				${updater}
+			` );
 		} else if ( propertyName ) {
+			block.builders.create.addLine( `${state.parentNode}.${propertyName} = ${last} = ${value};` );
 			updater = `${state.parentNode}.${propertyName} = ${last};`;
 		} else {
+			block.builders.create.addLine( `${generator.helper( method )}( ${state.parentNode}, '${name}', ${last} = ${value} );` );
 			updater = `${generator.helper( method )}( ${state.parentNode}, '${name}', ${last} );`;
 		}
 
-		block.builders.create.addLine( updater );
 		block.builders.update.addBlock( deindent`
 			if ( ${last} !== ( ${last} = ${value} ) ) {
 				${updater}

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

@@ -4,7 +4,7 @@ import getSetter from '../shared/binding/getSetter.js';
 import getStaticAttributeValue from './getStaticAttributeValue.js';
 
 export default function visitBinding ( generator, block, state, node, attribute ) {
-	const { name, keypath, parts } = flattenReference( attribute.value );
+	const { name, parts } = flattenReference( attribute.value );
 	const { snippet, contexts, dependencies } = block.contextualise( 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!' );
@@ -25,14 +25,16 @@ export default function visitBinding ( generator, block, state, node, attribute
 	const lock = block.alias( `${state.parentNode}_updating` );
 	let updateCondition = `!${lock}`;
 
+	block.addVariable( lock, 'false' );
+
 	// <select> special case
 	if ( node.name === 'select' ) {
 		if ( !isMultipleSelect ) {
 			setter = `var selectedOption = ${state.parentNode}.selectedOptions[0] || ${state.parentNode}.options[0];\n${setter}`;
 		}
 
 		const value = block.getUniqueName( 'value' );
-		const i = block.getUniqueName( 'i' );
+		const i = block.alias( 'i' );
 		const option = block.getUniqueName( 'option' );
 
 		const ifStatement = isMultipleSelect ?
@@ -84,7 +86,7 @@ export default function visitBinding ( generator, block, state, node, attribute
 
 		if ( attribute.name === 'currentTime' ) {
 			const frame = block.getUniqueName( `${state.parentNode}_animationframe` );
-			block.builders.create.addLine( `var ${frame};` );
+			block.addVariable( frame );
 			setter = deindent`
 				cancelAnimationFrame( ${frame} );
 				if ( !${state.parentNode}.paused ) ${frame} = requestAnimationFrame( ${handler} );
@@ -101,16 +103,14 @@ export default function visitBinding ( generator, block, state, node, attribute
 		else if ( attribute.name === 'paused' ) {
 			// this is necessary to prevent the audio restarting by itself
 			const last = block.getUniqueName( `${state.parentNode}_paused_value` );
-			block.builders.create.addLine( `var ${last} = true;` );
+			block.addVariable( last, 'true' );
 
 			updateCondition = `${last} !== ( ${last} = ${snippet} )`;
 			updateElement = `${state.parentNode}[ ${last} ? 'pause' : 'play' ]();`;
 		}
 	}
 
 	block.builders.create.addBlock( deindent`
-		var ${lock} = false;
-
 		function ${handler} () {
 			${lock} = true;
 			${setter}

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

@@ -98,7 +98,7 @@ export default function visitWindow ( generator, block, node ) {
 
 		const handlerBody = new CodeBuilder();
 		if ( event === 'scroll' ) { // TODO other bidirectional bindings...
-			block.builders.create.addLine( `var ${lock} = false;` );
+			block.addVariable( lock, 'false' );
 			handlerBody.addLine( `${lock} = true;` );
 		}
 

src/generators/dom/visitors/MustacheTag.js

@@ -6,8 +6,8 @@ export default function visitMustacheTag ( generator, block, state, node ) {
 
 	const { snippet } = block.contextualise( node.expression );
 
-	block.builders.create.addLine( `var ${value} = ${snippet};` );
-	block.addElement( name, `${generator.helper( 'createText' )}( ${value} )`, state.parentNode, true );
+	block.addVariable( value );
+	block.addElement( name, `${generator.helper( 'createText' )}( ${value} = ${snippet} )`, state.parentNode, true );
 
 	block.builders.update.addBlock( deindent`
 		if ( ${value} !== ( ${value} = ${snippet} ) ) {

test/js/samples/collapses-text-around-comments/expected.js

@@ -18,10 +18,11 @@ function add_css () {
 }
 
 function create_main_fragment ( root, component ) {
+	var text_value;
+
 	var p = createElement( 'p' );
 	setAttribute( p, 'svelte-3842350206', '' );
-	var text_value = root.foo;
-	var text = createText( text_value );
+	var text = createText( text_value = root.foo );
 	appendNode( text, p );
 
 	return {

test/js/samples/each-block-changed-check/expected.js

@@ -1,6 +1,8 @@
 import { appendNode, assign, createComment, createElement, createText, destroyEach, detachBetween, detachNode, dispatchObservers, insertNode, proto } from "svelte/shared.js";
 
 function create_main_fragment ( root, component ) {
+	var text_1_value;
+
 	var each_block_anchor = createComment();
 	var each_block_value = root.comments;
 	var each_block_iterations = [];
@@ -11,8 +13,7 @@ function create_main_fragment ( root, component ) {
 
 	var text = createText( "\n\n" );
 	var p = createElement( 'p' );
-	var text_1_value = root.foo;
-	var text_1 = createText( text_1_value );
+	var text_1 = createText( text_1_value = root.foo );
 	appendNode( text_1, p );
 
 	return {
@@ -63,23 +64,22 @@ function create_main_fragment ( root, component ) {
 }
 
 function create_each_block ( root, each_block_value, comment, i, component ) {
+	var text_value, text_2_value, text_4_value;
+
 	var div = createElement( 'div' );
 	div.className = "comment";
 	var strong = createElement( 'strong' );
 	appendNode( strong, div );
-	var text_value = i;
-	var text = createText( text_value );
+	var text = createText( text_value = i );
 	appendNode( text, strong );
 	appendNode( createText( "\n\n\t\t" ), div );
 	var span = createElement( 'span' );
 	appendNode( span, div );
 	span.className = "meta";
-	var text_2_value = comment.author;
-	var text_2 = createText( text_2_value );
+	var text_2 = createText( text_2_value = comment.author );
 	appendNode( text_2, span );
 	appendNode( createText( " wrote " ), span );
-	var text_4_value = root.elapsed(comment.time, root.time);
-	var text_4 = createText( text_4_value );
+	var text_4 = createText( text_4_value = root.elapsed(comment.time, root.time) );
 	appendNode( text_4, span );
 	appendNode( createText( " ago:" ), span );
 	appendNode( createText( "\n\n\t\t" ), div );