Skip to content

Commit

Permalink
Merge pull request #456 from sveltejs/gh-433-b
Browse files Browse the repository at this point in the history
Avoid binding event handler callbacks, version 2
  • Loading branch information
Rich-Harris authored Apr 10, 2017
2 parents ec709cb + fb9edf2 commit 20298b1
Show file tree
Hide file tree
Showing 32 changed files with 758 additions and 537 deletions.
20 changes: 14 additions & 6 deletions src/generators/Generator.js
Original file line number Diff line number Diff line change
Expand Up @@ -60,27 +60,34 @@ export default class Generator {
return alias;
}

contextualise ( fragment, expression, isEventHandler ) {
contextualise ( block, expression, context, 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 );
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;

Expand All @@ -93,10 +100,10 @@ export default class Generator {
}

else if ( contexts.has( name ) ) {
const context = contexts.get( name );
if ( context !== name ) {
const contextName = contexts.get( 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 ) );
Expand Down Expand Up @@ -133,6 +140,7 @@ export default class Generator {
},

leave ( node ) {
if ( /^Function/.test( node.type ) ) lexicalDepth -= 1;
if ( node._scope ) scope = scope.parent;
}
});
Expand Down
8 changes: 6 additions & 2 deletions src/generators/dom/Block.js
Original file line number Diff line number Diff line change
Expand Up @@ -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} );` );
}
Expand All @@ -53,12 +53,16 @@ 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 );
}

createMountStatement ( name, parentNode ) {
mount ( name, parentNode ) {
if ( parentNode ) {
this.builders.create.addLine( `${this.generator.helper( 'appendNode' )}( ${name}, ${parentNode} );` );
} else {
Expand Down
2 changes: 1 addition & 1 deletion src/generators/dom/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -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;
});

Expand Down
67 changes: 67 additions & 0 deletions src/generators/dom/visitors/Component/Attribute.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,67 @@
export default function visitAttribute ( generator, block, state, node, attribute, local ) {
if ( attribute.value === true ) {
// attributes without values, e.g. <textarea readonly>
local.staticAttributes.push({
name: attribute.name,
value: true
});
}

else if ( attribute.value.length === 0 ) {
local.staticAttributes.push({
name: attribute.name,
value: `''`
});
}

else if ( attribute.value.length === 1 ) {
const value = attribute.value[0];

if ( value.type === 'Text' ) {
// static attributes
const result = isNaN( value.data ) ? JSON.stringify( value.data ) : value.data;
local.staticAttributes.push({
name: attribute.name,
value: result
});
}

else {
// simple dynamic attributes
const { dependencies, string } = generator.contextualise( block, value.expression );

// TODO only update attributes that have changed
local.dynamicAttributes.push({
name: attribute.name,
value: string,
dependencies
});
}
}

else {
// complex dynamic attributes
const allDependencies = [];

const value = ( attribute.value[0].type === 'Text' ? '' : `"" + ` ) + (
attribute.value.map( chunk => {
if ( chunk.type === 'Text' ) {
return JSON.stringify( chunk.data );
} else {
const { dependencies, string } = generator.contextualise( block, chunk.expression );
dependencies.forEach( dependency => {
if ( !~allDependencies.indexOf( dependency ) ) allDependencies.push( dependency );
});

return `( ${string} )`;
}
}).join( ' + ' )
);

local.dynamicAttributes.push({
name: attribute.name,
value,
dependencies: allDependencies
});
}
}
Original file line number Diff line number Diff line change
@@ -1,8 +1,8 @@
import deindent from '../../../../utils/deindent.js';
import flattenReference from '../../../../utils/flattenReference.js';
import getSetter from './binding/getSetter.js';
import getSetter from '../shared/binding/getSetter.js';

export default function addComponentBinding ( generator, node, attribute, block, local ) {
export default function visitBinding ( generator, block, state, node, attribute, local ) {
const { name, keypath } = flattenReference( attribute.value );
const { snippet, contexts, dependencies } = generator.contextualise( block, attribute.value );

Expand Down Expand Up @@ -62,4 +62,4 @@ export default function addComponentBinding ( generator, node, attribute, block,
${updating} = false;
}
` );
}
}
Original file line number Diff line number Diff line change
@@ -1,7 +1,10 @@
import deindent from '../../../utils/deindent.js';
import CodeBuilder from '../../../utils/CodeBuilder.js';
import visit from '../visit.js';
import addComponentAttributes from './attributes/addComponentAttributes.js';
import deindent from '../../../../utils/deindent.js';
import CodeBuilder from '../../../../utils/CodeBuilder.js';
import visit from '../../visit.js';
import visitAttribute from './Attribute.js';
import visitEventHandler from './EventHandler.js';
import visitBinding from './Binding.js';
import visitRef from './Ref.js';

function capDown ( name ) {
return `${name[0].toLowerCase()}${name.slice( 1 )}`;
Expand All @@ -19,16 +22,37 @@ function stringifyProps ( props ) {
return `{ ${joined} }`;
}

const order = {
Attribute: 1,
EventHandler: 2,
Binding: 3,
Ref: 4
};

const visitors = {
Attribute: visitAttribute,
EventHandler: visitEventHandler,
Binding: visitBinding,
Ref: visitRef
};

export default function visitComponent ( generator, block, state, node ) {
const hasChildren = node.children.length > 0;
const name = block.getUniqueName( capDown( node.name === ':Self' ? generator.name : node.name ) );

const childState = Object.assign( {}, state, {
parentNode: null
});

const local = {
name,
namespace: state.namespace,
isComponent: true,

allUsedContexts: [],
staticAttributes: [],
dynamicAttributes: [],
bindings: [],

create: new CodeBuilder(),
update: new CodeBuilder()
Expand All @@ -38,7 +62,11 @@ export default function visitComponent ( generator, block, state, node ) {

generator.hasComponents = true;

addComponentAttributes( generator, block, node, local );
node.attributes
.sort( ( a, b ) => order[ a.type ] - order[ b.type ] )
.forEach( attribute => {
visitors[ attribute.type ]( generator, block, childState, node, attribute, local );
});

if ( local.allUsedContexts.length ) {
const initialProps = local.allUsedContexts.map( contextName => {
Expand Down Expand Up @@ -81,10 +109,6 @@ export default function visitComponent ( generator, block, state, node ) {
name: generator.getUniqueName( `create_${name}_yield_fragment` ) // TODO should getUniqueName happen inside Fragment? probably
});

const childState = Object.assign( {}, state, {
parentNode: null
});

node.children.forEach( child => {
visit( generator, childBlock, childState, child );
});
Expand Down
35 changes: 35 additions & 0 deletions src/generators/dom/visitors/Component/EventHandler.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,35 @@
import deindent from '../../../../utils/deindent.js';

export default function visitEventHandler ( generator, block, state, node, attribute, local ) {
// TODO verify that it's a valid callee (i.e. built-in or declared method)
generator.addSourcemapLocations( attribute.expression );
generator.code.prependRight( attribute.expression.start, `${block.component}.` );

const usedContexts = [];
attribute.expression.arguments.forEach( arg => {
const { contexts } = generator.contextualise( block, arg, null, 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._context.root;';

const listName = block.listNames.get( name );
const indexName = block.indexNames.get( name );

return `var ${listName} = this._context.${listName}, ${indexName} = this._context.${indexName}, ${name} = ${listName}[${indexName}]`;
});

const handlerBody = ( declarations.length ? declarations.join( '\n' ) + '\n\n' : '' ) + `[✂${attribute.expression.start}-${attribute.expression.end}✂];`;

local.create.addBlock( deindent`
${local.name}.on( '${attribute.name}', function ( event ) {
${handlerBody}
});
` );
}
13 changes: 13 additions & 0 deletions src/generators/dom/visitors/Component/Ref.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
import deindent from '../../../../utils/deindent.js';

export default function visitRef ( generator, block, state, node, attribute, local ) {
generator.usesRefs = true;

local.create.addLine(
`${block.component}.refs.${attribute.name} = ${local.name};`
);

block.builders.destroy.addLine( deindent`
if ( ${block.component}.refs.${attribute.name} === ${local.name} ) ${block.component}.refs.${attribute.name} = null;
` );
}
7 changes: 4 additions & 3 deletions src/generators/dom/visitors/EachBlock.js
Original file line number Diff line number Diff line change
Expand Up @@ -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;
` );
Expand All @@ -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`
Expand Down Expand Up @@ -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 => {
Expand Down
Loading

0 comments on commit 20298b1

Please sign in to comment.