From f82d04913fa8c5cd46d3c27d3b90c57ca9433124 Mon Sep 17 00:00:00 2001 From: Rich-Harris Date: Wed, 19 Apr 2017 09:38:34 -0400 Subject: [PATCH 1/3] hoist variables where appropriate (#504) --- src/generators/dom/Block.js | 21 ++++++++++++++++++- .../dom/visitors/Component/Binding.js | 3 +-- .../dom/visitors/Element/Attribute.js | 11 +++++++--- .../dom/visitors/Element/Binding.js | 12 +++++------ .../dom/visitors/Element/meta/Window.js | 2 +- src/generators/dom/visitors/MustacheTag.js | 4 ++-- .../expected.js | 5 +++-- .../each-block-changed-check/expected.js | 16 +++++++------- 8 files changed, 49 insertions(+), 25 deletions(-) diff --git a/src/generators/dom/Block.js b/src/generators/dom/Block.js index e14d8591f4d1..4ef169b65592 100644 --- a/src/generators/dom/Block.js +++ b/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} diff --git a/src/generators/dom/visitors/Component/Binding.js b/src/generators/dom/visitors/Component/Binding.js index 12bf41cbf098..c40d788cba95 100644 --- a/src/generators/dom/visitors/Component/Binding.js +++ b/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 ) { diff --git a/src/generators/dom/visitors/Element/Attribute.js b/src/generators/dom/visitors/Element/Attribute.js index 8dd976d406fd..1e83b5d33e74 100644 --- a/src/generators/dom/visitors/Element/Attribute.js +++ b/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} diff --git a/src/generators/dom/visitors/Element/Binding.js b/src/generators/dom/visitors/Element/Binding.js index 7159afbf13b9..2ca07f3f2e6d 100644 --- a/src/generators/dom/visitors/Element/Binding.js +++ b/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,6 +25,8 @@ export default function visitBinding ( generator, block, state, node, attribute const lock = block.alias( `${state.parentNode}_updating` ); let updateCondition = `!${lock}`; + block.addVariable( lock, 'false' ); + //