fix(compiler): don’t double bind functions
This fixes a performance regressions introduced by 178fb79b5cafe36c993bb1f9f9a63e8bac162c66. Also makes properties in the directive wrapper private so that closure compiler can minify them better.
This commit is contained in:
		
							parent
							
								
									32feb8a532
								
							
						
					
					
						commit
						e391cacdf9
					
				| @ -29,9 +29,9 @@ export class DirectiveWrapperCompileResult { | ||||
| } | ||||
| 
 | ||||
| const CONTEXT_FIELD_NAME = 'context'; | ||||
| const CHANGES_FIELD_NAME = 'changes'; | ||||
| const CHANGED_FIELD_NAME = 'changed'; | ||||
| const EVENT_HANDLER_FIELD_NAME = 'eventHandler'; | ||||
| const CHANGES_FIELD_NAME = '_changes'; | ||||
| const CHANGED_FIELD_NAME = '_changed'; | ||||
| const EVENT_HANDLER_FIELD_NAME = '_eventHandler'; | ||||
| 
 | ||||
| const CURR_VALUE_VAR = o.variable('currValue'); | ||||
| const THROW_ON_CHANGE_VAR = o.variable('throwOnChange'); | ||||
| @ -129,14 +129,15 @@ class DirectiveWrapperBuilder implements ClassBuilder { | ||||
| 
 | ||||
| 
 | ||||
|     const fields: o.ClassField[] = [ | ||||
|       new o.ClassField(EVENT_HANDLER_FIELD_NAME, o.FUNCTION_TYPE), | ||||
|       new o.ClassField(EVENT_HANDLER_FIELD_NAME, o.FUNCTION_TYPE, [o.StmtModifier.Private]), | ||||
|       new o.ClassField(CONTEXT_FIELD_NAME, o.importType(this.dirMeta.type)), | ||||
|       new o.ClassField(CHANGED_FIELD_NAME, o.BOOL_TYPE), | ||||
|       new o.ClassField(CHANGED_FIELD_NAME, o.BOOL_TYPE, [o.StmtModifier.Private]), | ||||
|     ]; | ||||
|     const ctorStmts: o.Statement[] = | ||||
|         [o.THIS_EXPR.prop(CHANGED_FIELD_NAME).set(o.literal(false)).toStmt()]; | ||||
|     if (this.genChanges) { | ||||
|       fields.push(new o.ClassField(CHANGES_FIELD_NAME, new o.MapType(o.DYNAMIC_TYPE))); | ||||
|       fields.push(new o.ClassField( | ||||
|           CHANGES_FIELD_NAME, new o.MapType(o.DYNAMIC_TYPE), [o.StmtModifier.Private])); | ||||
|       ctorStmts.push(RESET_CHANGES_STMT); | ||||
|     } | ||||
| 
 | ||||
| @ -303,7 +304,11 @@ function addHandleEventMethod(hostListeners: BoundEventAst[], builder: Directive | ||||
| } | ||||
| 
 | ||||
| function addSubscribeMethod(dirMeta: CompileDirectiveMetadata, builder: DirectiveWrapperBuilder) { | ||||
|   const methodParams: o.FnParam[] = [new o.FnParam(EVENT_HANDLER_FIELD_NAME, o.DYNAMIC_TYPE)]; | ||||
|   const methodParams: o.FnParam[] = [ | ||||
|     new o.FnParam( | ||||
|         VIEW_VAR.name, o.importType(resolveIdentifier(Identifiers.AppView), [o.DYNAMIC_TYPE])), | ||||
|     new o.FnParam(EVENT_HANDLER_FIELD_NAME, o.DYNAMIC_TYPE) | ||||
|   ]; | ||||
|   const stmts: o.Statement[] = [ | ||||
|     o.THIS_EXPR.prop(EVENT_HANDLER_FIELD_NAME).set(o.variable(EVENT_HANDLER_FIELD_NAME)).toStmt() | ||||
|   ]; | ||||
| @ -313,17 +318,16 @@ function addSubscribeMethod(dirMeta: CompileDirectiveMetadata, builder: Directiv | ||||
|     methodParams.push(new o.FnParam(paramName, o.BOOL_TYPE)); | ||||
|     const subscriptionFieldName = `subscription${emitterIdx}`; | ||||
|     builder.fields.push(new o.ClassField(subscriptionFieldName, o.DYNAMIC_TYPE)); | ||||
|     stmts.push(new o.IfStmt( | ||||
|         o.variable(paramName), | ||||
|         [o.THIS_EXPR.prop(subscriptionFieldName) | ||||
|              .set(o.THIS_EXPR.prop(CONTEXT_FIELD_NAME) | ||||
|                       .prop(emitterPropName) | ||||
|                       .callMethod( | ||||
|                           o.BuiltinMethod.SubscribeObservable, | ||||
|                           [o.variable(EVENT_HANDLER_FIELD_NAME) | ||||
|                                .callMethod( | ||||
|                                    o.BuiltinMethod.Bind, [o.NULL_EXPR, o.literal(eventName)])])) | ||||
|              .toStmt()])); | ||||
|     stmts.push(new o.IfStmt(o.variable(paramName), [ | ||||
|       o.THIS_EXPR.prop(subscriptionFieldName) | ||||
|           .set(o.THIS_EXPR.prop(CONTEXT_FIELD_NAME) | ||||
|                    .prop(emitterPropName) | ||||
|                    .callMethod( | ||||
|                        o.BuiltinMethod.SubscribeObservable, | ||||
|                        [o.variable(EVENT_HANDLER_FIELD_NAME) | ||||
|                             .callMethod(o.BuiltinMethod.Bind, [VIEW_VAR, o.literal(eventName)])])) | ||||
|           .toStmt() | ||||
|     ])); | ||||
|     builder.destroyStmts.push( | ||||
|         o.THIS_EXPR.prop(subscriptionFieldName) | ||||
|             .and(o.THIS_EXPR.prop(subscriptionFieldName).callMethod('unsubscribe', [])) | ||||
| @ -424,7 +428,7 @@ export class DirectiveWrapperExpressions { | ||||
|   } | ||||
|   static subscribe( | ||||
|       dirMeta: CompileDirectiveMetadata, hostProps: BoundElementPropertyAst[], usedEvents: string[], | ||||
|       dirWrapper: o.Expression, eventListener: o.Expression): o.Statement[] { | ||||
|       dirWrapper: o.Expression, view: o.Expression, eventListener: o.Expression): o.Statement[] { | ||||
|     let needsSubscribe = false; | ||||
|     let eventFlags: o.Expression[] = []; | ||||
|     Object.keys(dirMeta.outputs).forEach((propName) => { | ||||
| @ -439,7 +443,9 @@ export class DirectiveWrapperExpressions { | ||||
|       } | ||||
|     }); | ||||
|     if (needsSubscribe) { | ||||
|       return [dirWrapper.callMethod('subscribe', [eventListener].concat(eventFlags)).toStmt()]; | ||||
|       return [ | ||||
|         dirWrapper.callMethod('subscribe', [view, eventListener].concat(eventFlags)).toStmt() | ||||
|       ]; | ||||
|     } else { | ||||
|       return []; | ||||
|     } | ||||
|  | ||||
| @ -59,8 +59,8 @@ function subscribeToRenderEvents( | ||||
|     compileElement.view.createMethod.addStmt( | ||||
|         disposableVar | ||||
|             .set(o.importExpr(resolveIdentifier(Identifiers.subscribeToRenderElement)).callFn([ | ||||
|               ViewProperties.renderer, compileElement.renderNode, | ||||
|               createInlineArray(eventAndTargetExprs), handleEventClosure(compileElement) | ||||
|               o.THIS_EXPR, compileElement.renderNode, createInlineArray(eventAndTargetExprs), | ||||
|               handleEventExpr(compileElement) | ||||
|             ])) | ||||
|             .toDeclStmt(o.FUNCTION_TYPE, [o.StmtModifier.Private])); | ||||
|   } | ||||
| @ -73,8 +73,8 @@ function subscribeToDirectiveEvents( | ||||
|   directives.forEach((dirAst) => { | ||||
|     const dirWrapper = compileElement.directiveWrapperInstance.get(dirAst.directive.type.reference); | ||||
|     compileElement.view.createMethod.addStmts(DirectiveWrapperExpressions.subscribe( | ||||
|         dirAst.directive, dirAst.hostProperties, usedEventNames, dirWrapper, | ||||
|         handleEventClosure(compileElement))); | ||||
|         dirAst.directive, dirAst.hostProperties, usedEventNames, dirWrapper, o.THIS_EXPR, | ||||
|         handleEventExpr(compileElement))); | ||||
|   }); | ||||
| } | ||||
| 
 | ||||
| @ -127,11 +127,9 @@ function generateHandleEventMethod( | ||||
|       handleEventStmts.finish(), o.BOOL_TYPE)); | ||||
| } | ||||
| 
 | ||||
| function handleEventClosure(compileElement: CompileElement) { | ||||
| function handleEventExpr(compileElement: CompileElement) { | ||||
|   const handleEventMethodName = getHandleEventMethodName(compileElement.nodeIndex); | ||||
|   return o.THIS_EXPR.callMethod( | ||||
|       'eventHandler', | ||||
|       [o.THIS_EXPR.prop(handleEventMethodName).callMethod(o.BuiltinMethod.Bind, [o.THIS_EXPR])]); | ||||
|   return o.THIS_EXPR.callMethod('eventHandler', [o.THIS_EXPR.prop(handleEventMethodName)]); | ||||
| } | ||||
| 
 | ||||
| type EventSummary = { | ||||
|  | ||||
| @ -375,7 +375,7 @@ export class DebugAppView<T> extends AppView<T> { | ||||
|     return (eventName: string, event?: any) => { | ||||
|       this._resetDebug(); | ||||
|       try { | ||||
|         return superHandler(eventName, event); | ||||
|         return superHandler.call(this, eventName, event); | ||||
|       } catch (e) { | ||||
|         this._rethrowWithContext(e); | ||||
|         throw e; | ||||
|  | ||||
| @ -17,6 +17,7 @@ import {Sanitizer} from '../security'; | ||||
| 
 | ||||
| import {AppElement} from './element'; | ||||
| import {ExpressionChangedAfterItHasBeenCheckedError} from './errors'; | ||||
| import {AppView} from './view'; | ||||
| 
 | ||||
| @Injectable() | ||||
| export class ViewUtils { | ||||
| @ -401,7 +402,7 @@ export function selectOrCreateRenderHostElement( | ||||
| } | ||||
| 
 | ||||
| export function subscribeToRenderElement( | ||||
|     renderer: Renderer, element: any, eventNamesAndTargets: InlineArray<string>, | ||||
|     view: AppView<any>, element: any, eventNamesAndTargets: InlineArray<string>, | ||||
|     listener: (eventName: string, event: any) => any) { | ||||
|   const disposables = createEmptyInlineArray(eventNamesAndTargets.length / 2); | ||||
|   for (var i = 0; i < eventNamesAndTargets.length; i += 2) { | ||||
| @ -409,10 +410,10 @@ export function subscribeToRenderElement( | ||||
|     const eventTarget = eventNamesAndTargets.get(i + 1); | ||||
|     let disposable: Function; | ||||
|     if (eventTarget) { | ||||
|       disposable = renderer.listenGlobal( | ||||
|           eventTarget, eventName, listener.bind(null, `${eventTarget}:${eventName}`)); | ||||
|       disposable = view.renderer.listenGlobal( | ||||
|           eventTarget, eventName, listener.bind(view, `${eventTarget}:${eventName}`)); | ||||
|     } else { | ||||
|       disposable = renderer.listen(element, eventName, listener.bind(null, eventName)); | ||||
|       disposable = view.renderer.listen(element, eventName, listener.bind(view, eventName)); | ||||
|     } | ||||
|     disposables.set(i / 2, disposable); | ||||
|   } | ||||
|  | ||||
		Loading…
	
	
			
			x
			
			
		
	
		Reference in New Issue
	
	Block a user