Skip to content

Commit 20298b1

Browse files
Rich-HarrisRich-Harris
authored
Merge pull request #456 from sveltejs/gh-433-b
Avoid binding event handler callbacks, version 2
2 parents ec709cb + fb9edf2 commit 20298b1

32 files changed

+758
-537
lines changed

‎src/generators/Generator.js

+14-6
Original file line numberDiff line numberDiff line change
@@ -60,27 +60,34 @@ export default class Generator {
6060
return alias;
6161
}
6262

63-
contextualise ( fragment, expression, isEventHandler ) {
63+
contextualise ( block, expression, context, isEventHandler ) {
6464
this.addSourcemapLocations( expression );
6565

6666
const usedContexts = [];
6767
const dependencies = [];
6868

6969
const { code, helpers } = this;
70-
const { contextDependencies, contexts, indexes } = fragment;
70+
const { contextDependencies, contexts, indexes } = block;
7171

7272
let scope = annotateWithScopes( expression );
73+
let lexicalDepth = 0;
7374

7475
const self = this;
7576

7677
walk( expression, {
7778
enter ( node, parent, key ) {
79+
if ( /^Function/.test( node.type ) ) lexicalDepth += 1;
80+
7881
if ( node._scope ) {
7982
scope = node._scope;
8083
return;
8184
}
8285

83-
if ( isReference( node, parent ) ) {
86+
if ( node.type === 'ThisExpression' ) {
87+
if ( lexicalDepth === 0 && context ) code.overwrite( node.start, node.end, context, true );
88+
}
89+
90+
else if ( isReference( node, parent ) ) {
8491
const { name } = flattenReference( node );
8592
if ( scope.has( name ) ) return;
8693

@@ -93,10 +100,10 @@ export default class Generator {
93100
}
94101

95102
else if ( contexts.has( name ) ) {
96-
const context = contexts.get( name );
97-
if ( context !== name ) {
103+
const contextName = contexts.get( name );
104+
if ( contextName !== name ) {
98105
// this is true for 'reserved' names like `root` and `component`
99-
code.overwrite( node.start, node.start + name.length, context, true );
106+
code.overwrite( node.start, node.start + name.length, contextName, true );
100107
}
101108

102109
dependencies.push( ...contextDependencies.get( name ) );
@@ -133,6 +140,7 @@ export default class Generator {
133140
},
134141

135142
leave ( node ) {
143+
if ( /^Function/.test( node.type ) ) lexicalDepth -= 1;
136144
if ( node._scope ) scope = scope.parent;
137145
}
138146
});

‎src/generators/dom/Block.js

+6-2
Original file line numberDiff line numberDiff line change
@@ -39,7 +39,7 @@ export default class Block {
3939
`var ${name} = ${renderStatement};`
4040
);
4141

42-
this.createMountStatement( name, parentNode );
42+
this.mount( name, parentNode );
4343
} else {
4444
this.builders.create.addLine( `${this.generator.helper( 'appendNode' )}( ${renderStatement}, ${parentNode} );` );
4545
}
@@ -53,12 +53,16 @@ export default class Block {
5353
return new Block( Object.assign( {}, this, options, { parent: this } ) );
5454
}
5555

56+
contextualise ( expression, context, isEventHandler ) {
57+
return this.generator.contextualise( this, expression, context, isEventHandler );
58+
}
59+
5660
createAnchor ( name, parentNode ) {
5761
const renderStatement = `${this.generator.helper( 'createComment' )}()`;
5862
this.addElement( name, renderStatement, parentNode, true );
5963
}
6064

61-
createMountStatement ( name, parentNode ) {
65+
mount ( name, parentNode ) {
6266
if ( parentNode ) {
6367
this.builders.create.addLine( `${this.generator.helper( 'appendNode' )}( ${name}, ${parentNode} );` );
6468
} else {

‎src/generators/dom/index.js

+1-1
Original file line numberDiff line numberDiff line change
@@ -271,7 +271,7 @@ export default function dom ( parsed, source, options ) {
271271
throw new Error( `Components with shared helpers must be compiled to ES2015 modules (format: 'es')` );
272272
}
273273

274-
const names = Array.from( generator.uses ).map( name => {
274+
const names = Array.from( generator.uses ).sort().map( name => {
275275
return name !== generator.alias( name ) ? `${name} as ${generator.alias( name )}` : name;
276276
});
277277

‎src/generators/dom/visitors/Component/Attribute.js

+67
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,67 @@
1+
export default function visitAttribute ( generator, block, state, node, attribute, local ) {
2+
if ( attribute.value === true ) {
3+
// attributes without values, e.g. <textarea readonly>
4+
local.staticAttributes.push({
5+
name: attribute.name,
6+
value: true
7+
});
8+
}
9+
10+
else if ( attribute.value.length === 0 ) {
11+
local.staticAttributes.push({
12+
name: attribute.name,
13+
value: `''`
14+
});
15+
}
16+
17+
else if ( attribute.value.length === 1 ) {
18+
const value = attribute.value[0];
19+
20+
if ( value.type === 'Text' ) {
21+
// static attributes
22+
const result = isNaN( value.data ) ? JSON.stringify( value.data ) : value.data;
23+
local.staticAttributes.push({
24+
name: attribute.name,
25+
value: result
26+
});
27+
}
28+
29+
else {
30+
// simple dynamic attributes
31+
const { dependencies, string } = generator.contextualise( block, value.expression );
32+
33+
// TODO only update attributes that have changed
34+
local.dynamicAttributes.push({
35+
name: attribute.name,
36+
value: string,
37+
dependencies
38+
});
39+
}
40+
}
41+
42+
else {
43+
// complex dynamic attributes
44+
const allDependencies = [];
45+
46+
const value = ( attribute.value[0].type === 'Text' ? '' : `"" + ` ) + (
47+
attribute.value.map( chunk => {
48+
if ( chunk.type === 'Text' ) {
49+
return JSON.stringify( chunk.data );
50+
} else {
51+
const { dependencies, string } = generator.contextualise( block, chunk.expression );
52+
dependencies.forEach( dependency => {
53+
if ( !~allDependencies.indexOf( dependency ) ) allDependencies.push( dependency );
54+
});
55+
56+
return `( ${string} )`;
57+
}
58+
}).join( ' + ' )
59+
);
60+
61+
local.dynamicAttributes.push({
62+
name: attribute.name,
63+
value,
64+
dependencies: allDependencies
65+
});
66+
}
67+
}

‎src/generators/dom/visitors/attributes/addComponentBinding.js ‎src/generators/dom/visitors/Component/Binding.js

+3-3
Original file line numberDiff line numberDiff line change
@@ -1,8 +1,8 @@
11
import deindent from '../../../../utils/deindent.js';
22
import flattenReference from '../../../../utils/flattenReference.js';
3-
import getSetter from './binding/getSetter.js';
3+
import getSetter from '../shared/binding/getSetter.js';
44

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

@@ -62,4 +62,4 @@ export default function addComponentBinding ( generator, node, attribute, block,
6262
${updating} = false;
6363
}
6464
` );
65-
}
65+
}

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

+33-9
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,10 @@
1-
import deindent from '../../../utils/deindent.js';
2-
import CodeBuilder from '../../../utils/CodeBuilder.js';
3-
import visit from '../visit.js';
4-
import addComponentAttributes from './attributes/addComponentAttributes.js';
1+
import deindent from '../../../../utils/deindent.js';
2+
import CodeBuilder from '../../../../utils/CodeBuilder.js';
3+
import visit from '../../visit.js';
4+
import visitAttribute from './Attribute.js';
5+
import visitEventHandler from './EventHandler.js';
6+
import visitBinding from './Binding.js';
7+
import visitRef from './Ref.js';
58

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

25+
const order = {
26+
Attribute: 1,
27+
EventHandler: 2,
28+
Binding: 3,
29+
Ref: 4
30+
};
31+
32+
const visitors = {
33+
Attribute: visitAttribute,
34+
EventHandler: visitEventHandler,
35+
Binding: visitBinding,
36+
Ref: visitRef
37+
};
38+
2239
export default function visitComponent ( generator, block, state, node ) {
2340
const hasChildren = node.children.length > 0;
2441
const name = block.getUniqueName( capDown( node.name === ':Self' ? generator.name : node.name ) );
2542

43+
const childState = Object.assign( {}, state, {
44+
parentNode: null
45+
});
46+
2647
const local = {
2748
name,
2849
namespace: state.namespace,
2950
isComponent: true,
3051

3152
allUsedContexts: [],
53+
staticAttributes: [],
54+
dynamicAttributes: [],
55+
bindings: [],
3256

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

3963
generator.hasComponents = true;
4064

41-
addComponentAttributes( generator, block, node, local );
65+
node.attributes
66+
.sort( ( a, b ) => order[ a.type ] - order[ b.type ] )
67+
.forEach( attribute => {
68+
visitors[ attribute.type ]( generator, block, childState, node, attribute, local );
69+
});
4270

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

84-
const childState = Object.assign( {}, state, {
85-
parentNode: null
86-
});
87-
88112
node.children.forEach( child => {
89113
visit( generator, childBlock, childState, child );
90114
});

‎src/generators/dom/visitors/Component/EventHandler.js

+35
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,35 @@
1+
import deindent from '../../../../utils/deindent.js';
2+
3+
export default function visitEventHandler ( generator, block, state, node, attribute, local ) {
4+
// TODO verify that it's a valid callee (i.e. built-in or declared method)
5+
generator.addSourcemapLocations( attribute.expression );
6+
generator.code.prependRight( attribute.expression.start, `${block.component}.` );
7+
8+
const usedContexts = [];
9+
attribute.expression.arguments.forEach( arg => {
10+
const { contexts } = generator.contextualise( block, arg, null, true );
11+
12+
contexts.forEach( context => {
13+
if ( !~usedContexts.indexOf( context ) ) usedContexts.push( context );
14+
if ( !~local.allUsedContexts.indexOf( context ) ) local.allUsedContexts.push( context );
15+
});
16+
});
17+
18+
// TODO hoist event handlers? can do `this.__component.method(...)`
19+
const declarations = usedContexts.map( name => {
20+
if ( name === 'root' ) return 'var root = this._context.root;';
21+
22+
const listName = block.listNames.get( name );
23+
const indexName = block.indexNames.get( name );
24+
25+
return `var ${listName} = this._context.${listName}, ${indexName} = this._context.${indexName}, ${name} = ${listName}[${indexName}]`;
26+
});
27+
28+
const handlerBody = ( declarations.length ? declarations.join( '\n' ) + '\n\n' : '' ) + `[✂${attribute.expression.start}-${attribute.expression.end}✂];`;
29+
30+
local.create.addBlock( deindent`
31+
${local.name}.on( '${attribute.name}', function ( event ) {
32+
${handlerBody}
33+
});
34+
` );
35+
}

‎src/generators/dom/visitors/Component/Ref.js

+13
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,13 @@
1+
import deindent from '../../../../utils/deindent.js';
2+
3+
export default function visitRef ( generator, block, state, node, attribute, local ) {
4+
generator.usesRefs = true;
5+
6+
local.create.addLine(
7+
`${block.component}.refs.${attribute.name} = ${local.name};`
8+
);
9+
10+
block.builders.destroy.addLine( deindent`
11+
if ( ${block.component}.refs.${attribute.name} === ${local.name} ) ${block.component}.refs.${attribute.name} = null;
12+
` );
13+
}

‎src/generators/dom/visitors/EachBlock.js

+4-3
Original file line numberDiff line numberDiff line change
@@ -134,7 +134,7 @@ export default function visitEachBlock ( generator, block, state, node ) {
134134
}
135135
}
136136
137-
destroyEach( ${localVars.iterations}, true, ${listName}.length );
137+
${generator.helper( 'destroyEach' )}( ${localVars.iterations}, true, ${listName}.length );
138138
139139
${localVars.iterations}.length = ${listName}.length;
140140
` );
@@ -154,7 +154,7 @@ export default function visitEachBlock ( generator, block, state, node ) {
154154
}
155155

156156
block.builders.destroy.addBlock(
157-
`${generator.helper( 'destroyEach' )}( ${localVars.iterations}, ${isToplevel ? 'detach' : 'false'} );` );
157+
`${generator.helper( 'destroyEach' )}( ${localVars.iterations}, ${isToplevel ? 'detach' : 'false'}, 0 );` );
158158

159159
if ( node.else ) {
160160
block.builders.destroy.addBlock( deindent`
@@ -197,7 +197,8 @@ export default function visitEachBlock ( generator, block, state, node ) {
197197
});
198198

199199
const childState = Object.assign( {}, state, {
200-
parentNode: null
200+
parentNode: null,
201+
inEachBlock: true
201202
});
202203

203204
node.children.forEach( child => {

0 commit comments

Comments
 (0)