Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

hoist variables where appropriate #506

Merged
merged 1 commit into from
Apr 19, 2017
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
21 changes: 20 additions & 1 deletion src/generators/dom/Block.js
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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 ) );
Expand Down Expand Up @@ -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();` );
}
Expand Down Expand Up @@ -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}
Expand Down
3 changes: 1 addition & 2 deletions src/generators/dom/visitors/Component/Binding.js
Original file line number Diff line number Diff line change
Expand Up @@ -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 ) {
Expand Down
11 changes: 8 additions & 3 deletions src/generators/dom/visitors/Element/Attribute.js
Original file line number Diff line number Diff line change
Expand Up @@ -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';

Expand All @@ -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}
Expand Down
12 changes: 6 additions & 6 deletions src/generators/dom/visitors/Element/Binding.js
Original file line number Diff line number Diff line change
Expand Up @@ -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!' );
Expand All @@ -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 ?
Expand Down Expand Up @@ -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} );
Expand All @@ -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}
Expand Down
2 changes: 1 addition & 1 deletion src/generators/dom/visitors/Element/meta/Window.js
Original file line number Diff line number Diff line change
Expand Up @@ -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;` );
}

Expand Down
4 changes: 2 additions & 2 deletions src/generators/dom/visitors/MustacheTag.js
Original file line number Diff line number Diff line change
Expand Up @@ -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} ) ) {
Expand Down
5 changes: 3 additions & 2 deletions test/js/samples/collapses-text-around-comments/expected.js
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down
16 changes: 8 additions & 8 deletions test/js/samples/each-block-changed-check/expected.js
Original file line number Diff line number Diff line change
@@ -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 = [];
Expand All @@ -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 {
Expand Down Expand Up @@ -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 );
Expand Down