perf(ivy): remove unused event argument in listener instructions (#35097)

Currently Ivy always generates the `$event` function argument, even if it isn't being used by the listener expressions. This can lead to unnecessary bytes being generated, because optimizers won't remove unused arguments by default. These changes add some logic to avoid adding the argument when it isn't required.

PR Close #35097
This commit is contained in:
Kristiyan Kostadinov 2020-02-01 13:19:31 +01:00 committed by Miško Hevery
parent 39ef57971c
commit 9228d7f15d
9 changed files with 149 additions and 29 deletions

View File

@ -1138,7 +1138,7 @@ describe('compiler compliance: bindings', () => {
hostBindings: function MyDirective_HostBindings(rf, ctx) {
if (rf & 1) {
$r3$.ɵɵlistener("mousedown", function MyDirective_mousedown_HostBindingHandler($event) { return ctx.mousedown(); })("mouseup", function MyDirective_mouseup_HostBindingHandler($event) { return ctx.mouseup(); })("click", function MyDirective_click_HostBindingHandler($event) { return ctx.click(); });
$r3$.ɵɵlistener("mousedown", function MyDirective_mousedown_HostBindingHandler() { return ctx.mousedown(); })("mouseup", function MyDirective_mouseup_HostBindingHandler() { return ctx.mouseup(); })("click", function MyDirective_click_HostBindingHandler() { return ctx.click(); });
}
}
`;
@ -1171,7 +1171,7 @@ describe('compiler compliance: bindings', () => {
hostBindings: function MyComponent_HostBindings(rf, ctx) {
if (rf & 1) {
$r3$.ɵɵcomponentHostSyntheticListener("@animation.done", function MyComponent_animation_animation_done_HostBindingHandler($event) { return ctx.done(); })("@animation.start", function MyComponent_animation_animation_start_HostBindingHandler($event) { return ctx.start(); });
$r3$.ɵɵcomponentHostSyntheticListener("@animation.done", function MyComponent_animation_animation_done_HostBindingHandler() { return ctx.done(); })("@animation.start", function MyComponent_animation_animation_start_HostBindingHandler() { return ctx.start(); });
}
}
`;
@ -1209,8 +1209,8 @@ describe('compiler compliance: bindings', () => {
hostBindings: function MyComponent_HostBindings(rf, ctx) {
if (rf & 1) {
$r3$.ɵɵcomponentHostSyntheticListener("@animation.done", function MyComponent_animation_animation_done_HostBindingHandler($event) { return ctx.done(); })("@animation.start", function MyComponent_animation_animation_start_HostBindingHandler($event) { return ctx.start(); });
$r3$.ɵɵlistener("mousedown", function MyComponent_mousedown_HostBindingHandler($event) { return ctx.mousedown(); })("mouseup", function MyComponent_mouseup_HostBindingHandler($event) { return ctx.mouseup(); })("click", function MyComponent_click_HostBindingHandler($event) { return ctx.click(); });
$r3$.ɵɵcomponentHostSyntheticListener("@animation.done", function MyComponent_animation_animation_done_HostBindingHandler() { return ctx.done(); })("@animation.start", function MyComponent_animation_animation_start_HostBindingHandler() { return ctx.start(); });
$r3$.ɵɵlistener("mousedown", function MyComponent_mousedown_HostBindingHandler() { return ctx.mousedown(); })("mouseup", function MyComponent_mouseup_HostBindingHandler() { return ctx.mouseup(); })("click", function MyComponent_click_HostBindingHandler() { return ctx.click(); });
}
}
`;

View File

@ -398,7 +398,7 @@ describe('compiler compliance: directives', () => {
template: function MyComponent_Template(rf, ctx) {
if (rf & 1) {
$r3$.ɵɵelementStart(0, "div", 0);
$r3$.ɵɵlistener("someDirective", function MyComponent_Template_div_someDirective_0_listener($event) { return ctx.noop(); });
$r3$.ɵɵlistener("someDirective", function MyComponent_Template_div_someDirective_0_listener() { return ctx.noop(); });
$r3$.ɵɵelementEnd();
}
},

View File

@ -1807,7 +1807,7 @@ describe('i18n support in the template compiler', () => {
template: function MyComponent_Template(rf, ctx) {
if (rf & 1) {
$r3$.ɵɵelementStart(0, "div", 0);
$r3$.ɵɵlistener("click", function MyComponent_Template_div_click_0_listener($event) { return ctx.onClick(); });
$r3$.ɵɵlistener("click", function MyComponent_Template_div_click_0_listener() { return ctx.onClick(); });
$r3$.ɵɵi18n(1, $I18N_1$);
$r3$.ɵɵelementEnd();
}

View File

@ -138,14 +138,14 @@ describe('compiler compliance: listen()', () => {
const $s$ = $r3$.ɵɵgetCurrentView();
$r3$.ɵɵelementStart(0, "div");
$r3$.ɵɵelementStart(1, "div", 1);
$r3$.ɵɵlistener("click", function MyComponent_div_0_Template_div_click_1_listener($event) {
$r3$.ɵɵlistener("click", function MyComponent_div_0_Template_div_click_1_listener() {
$r3$.ɵɵrestoreView($s$);
const $comp$ = $r3$.ɵɵnextContext();
return $comp$.onClick($comp$.foo);
});
$r3$.ɵɵelementEnd();
$r3$.ɵɵelementStart(2, "button", 1);
$r3$.ɵɵlistener("click", function MyComponent_div_0_Template_button_click_2_listener($event) {
$r3$.ɵɵlistener("click", function MyComponent_div_0_Template_button_click_2_listener() {
$r3$.ɵɵrestoreView($s$);
const $comp2$ = $r3$.ɵɵnextContext();
return $comp2$.onClick2($comp2$.bar);
@ -204,7 +204,7 @@ describe('compiler compliance: listen()', () => {
if (rf & 1) {
const $s$ = $r3$.ɵɵgetCurrentView();
$r3$.ɵɵelementStart(0, "button", 0);
$r3$.ɵɵlistener("click", function MyComponent_Template_button_click_0_listener($event) {
$r3$.ɵɵlistener("click", function MyComponent_Template_button_click_0_listener() {
$r3$.ɵɵrestoreView($s$);
const $user$ = $r3$.ɵɵreference(3);
return ctx.onClick($user$.value);
@ -254,9 +254,9 @@ describe('compiler compliance: listen()', () => {
template: function MyComponent_Template(rf, ctx) {
if (rf & 1) {
$r3$.ɵɵelementStart(0, "div", 0);
$r3$.ɵɵlistener("click", function MyComponent_Template_div_click_0_listener($event) {
$r3$.ɵɵlistener("click", function MyComponent_Template_div_click_0_listener() {
return ctx.click();
})("change", function MyComponent_Template_div_change_0_listener($event) {
})("change", function MyComponent_Template_div_change_0_listener() {
return ctx.change();
});
$r3$.ɵɵelementEnd();
@ -296,10 +296,10 @@ describe('compiler compliance: listen()', () => {
template: function MyComponent_Template(rf, ctx) {
if (rf & 1) {
$r3$.ɵɵelementStart(0, "div", 0);
$r3$.ɵɵlistener("click", function MyComponent_Template_div_click_0_listener($event) { return ctx.click(); })("change", function MyComponent_Template_div_change_0_listener($event) { return ctx.change(); });
$r3$.ɵɵlistener("click", function MyComponent_Template_div_click_0_listener() { return ctx.click(); })("change", function MyComponent_Template_div_change_0_listener() { return ctx.change(); });
$r3$.ɵɵelementEnd();
$r3$.ɵɵelementStart(1, "some-comp", 1);
$r3$.ɵɵlistener("update", function MyComponent_Template_some_comp_update_1_listener($event) { return ctx.update(); })("delete", function MyComponent_Template_some_comp_delete_1_listener($event) { return ctx.delete(); });
$r3$.ɵɵlistener("update", function MyComponent_Template_some_comp_update_1_listener() { return ctx.update(); })("delete", function MyComponent_Template_some_comp_delete_1_listener() { return ctx.delete(); });
$r3$.ɵɵelementEnd();
}
}
@ -334,7 +334,7 @@ describe('compiler compliance: listen()', () => {
template: function MyComponent_Template(rf, ctx) {
if (rf & 1) {
$r3$.ɵɵtemplate(0, MyComponent_ng_template_0_Template, 0, 0, "ng-template", 0);
$r3$.ɵɵlistener("click", function MyComponent_Template_ng_template_click_0_listener($event) { return ctx.click(); })("change", function MyComponent_Template_ng_template_change_0_listener($event) { return ctx.change(); });
$r3$.ɵɵlistener("click", function MyComponent_Template_ng_template_click_0_listener() { return ctx.click(); })("change", function MyComponent_Template_ng_template_change_0_listener() { return ctx.change(); });
}
}
`;
@ -343,5 +343,108 @@ describe('compiler compliance: listen()', () => {
expectEmit(result.source, template, 'Incorrect template');
});
it('should not generate the $event argument if it is not being used in a template', () => {
const files = {
app: {
'spec.ts': `
import {Component} from '@angular/core';
@Component({
template: \`<div (click)="onClick();"></div>\`
})
export class MyComponent {
onClick() {}
}
`
}
};
const template = `
consts: [[${AttributeMarker.Bindings}, "click"]],
template: function MyComponent_Template(rf, ctx) {
if (rf & 1) {
$r3$.ɵɵelementStart(0, "div", 0);
$r3$.ɵɵlistener("click", function MyComponent_Template_div_click_0_listener() {
return ctx.onClick();
});
$r3$.ɵɵelementEnd();
}
}
`;
const result = compile(files, angularFiles);
expectEmit(result.source, template, 'Incorrect template');
});
it('should not generate the $event argument if it is not being used in a host listener', () => {
const files = {
app: {
'spec.ts': `
import {Component, HostListener} from '@angular/core';
@Component({
template: '',
host: {
'(mousedown)': 'mousedown()'
}
})
export class MyComponent {
mousedown() {}
@HostListener('click')
click() {}
}
`
}
};
const template = `
hostBindings: function MyComponent_HostBindings(rf, ctx) {
if (rf & 1) {
i0.ɵɵlistener("mousedown", function MyComponent_mousedown_HostBindingHandler() {
return ctx.mousedown();
})("click", function MyComponent_click_HostBindingHandler() {
return ctx.click();
});
}
}
`;
const result = compile(files, angularFiles);
expectEmit(result.source, template, 'Incorrect host bindings');
});
it('should generate the $event argument if it is being used in a host listener', () => {
const files = {
app: {
'spec.ts': `
import {Directive, HostListener} from '@angular/core';
@Directive()
export class MyComponent {
@HostListener('click', ['$event.target'])
click(t: EventTarget) {}
}
`
}
};
const template = `
hostBindings: function MyComponent_HostBindings(rf, ctx) {
if (rf & 1) {
i0.ɵɵlistener("click", function MyComponent_click_HostBindingHandler($event) {
return ctx.click($event.target);
});
}
}
`;
const result = compile(files, angularFiles);
expectEmit(result.source, template, 'Incorrect host bindings');
});
});

View File

@ -336,7 +336,7 @@ describe('compiler compliance: styling', () => {
hostVars: 1,
hostBindings: function MyAnimDir_HostBindings(rf, ctx) {
if (rf & 1) {
$r3$.ɵɵcomponentHostSyntheticListener("@myAnim.start", function MyAnimDir_animation_myAnim_start_HostBindingHandler($event) { return ctx.onStart(); })("@myAnim.done", function MyAnimDir_animation_myAnim_done_HostBindingHandler($event) { return ctx.onDone(); });
$r3$.ɵɵcomponentHostSyntheticListener("@myAnim.start", function MyAnimDir_animation_myAnim_start_HostBindingHandler() { return ctx.onStart(); })("@myAnim.done", function MyAnimDir_animation_myAnim_done_HostBindingHandler() { return ctx.onDone(); });
} if (rf & 2) {
$r3$.ɵɵupdateSyntheticHostBinding("@myAnim", ctx.myAnimState);
}

View File

@ -54,7 +54,7 @@ describe('compiler compliance: template', () => {
if (rf & 1) {
const $s$ = $i0$.ɵɵgetCurrentView();
$i0$.ɵɵelementStart(0, "div", 2);
$i0$.ɵɵlistener("click", function MyComponent_ul_0_li_1_div_1_Template_div_click_0_listener($event){
$i0$.ɵɵlistener("click", function MyComponent_ul_0_li_1_div_1_Template_div_click_0_listener(){
$i0$.ɵɵrestoreView($s$);
const $inner$ = ctx.$implicit;
const $middle$ = $i0$.ɵɵnextContext().$implicit;
@ -146,7 +146,7 @@ describe('compiler compliance: template', () => {
if (rf & 1) {
const $s$ = $r3$.ɵɵgetCurrentView();
$r3$.ɵɵelementStart(0, "div", 1);
$r3$.ɵɵlistener("click", function MyComponent_div_0_Template_div_click_0_listener($event) {
$r3$.ɵɵlistener("click", function MyComponent_div_0_Template_div_click_0_listener() {
$r3$.ɵɵrestoreView($s$);
const $d$ = ctx.$implicit;
const $i$ = ctx.index;
@ -201,7 +201,7 @@ describe('compiler compliance: template', () => {
if (rf & 1) {
const $_r2$ = i0.ɵɵgetCurrentView();
$r3$.ɵɵelementStart(0, "div", 2);
$r3$.ɵɵlistener("click", function MyComponent_div_0_Template_div_click_0_listener($event) {
$r3$.ɵɵlistener("click", function MyComponent_div_0_Template_div_click_0_listener() {
i0.ɵɵrestoreView($_r2$);
const $ctx_r1$ = i0.ɵɵnextContext();
return $ctx_r1$.greet($ctx_r1$);

View File

@ -2504,7 +2504,7 @@ runInEachFileSystem(os => {
const hostBindingsFn = `
hostBindings: function FooCmp_HostBindings(rf, ctx) {
if (rf & 1) {
i0.ɵɵlistener("click", function FooCmp_click_HostBindingHandler($event) { return ctx.onClick(); })("click", function FooCmp_click_HostBindingHandler($event) { return ctx.onDocumentClick($event.target); }, false, i0.ɵɵresolveDocument)("scroll", function FooCmp_scroll_HostBindingHandler($event) { return ctx.onWindowScroll(); }, false, i0.ɵɵresolveWindow);
i0.ɵɵlistener("click", function FooCmp_click_HostBindingHandler() { return ctx.onClick(); })("click", function FooCmp_click_HostBindingHandler($event) { return ctx.onDocumentClick($event.target); }, false, i0.ɵɵresolveDocument)("scroll", function FooCmp_scroll_HostBindingHandler() { return ctx.onWindowScroll(); }, false, i0.ɵɵresolveWindow);
}
}
`;
@ -2635,7 +2635,7 @@ runInEachFileSystem(os => {
hostVars: 4,
hostBindings: function FooCmp_HostBindings(rf, ctx) {
if (rf & 1) {
i0.ɵɵlistener("click", function FooCmp_click_HostBindingHandler($event) { return ctx.onClick($event); })("click", function FooCmp_click_HostBindingHandler($event) { return ctx.onBodyClick($event); }, false, i0.ɵɵresolveBody)("change", function FooCmp_change_HostBindingHandler($event) { return ctx.onChange(ctx.arg1, ctx.arg2, ctx.arg3); });
i0.ɵɵlistener("click", function FooCmp_click_HostBindingHandler($event) { return ctx.onClick($event); })("click", function FooCmp_click_HostBindingHandler($event) { return ctx.onBodyClick($event); }, false, i0.ɵɵresolveBody)("change", function FooCmp_change_HostBindingHandler() { return ctx.onChange(ctx.arg1, ctx.arg2, ctx.arg3); });
}
if (rf & 2) {
i0.ɵɵhostProperty("prop", ctx.bar);
@ -2703,7 +2703,7 @@ runInEachFileSystem(os => {
selector: '[test]',
})
class Dir {
@HostListener('change', ['arg'])
@HostListener('change', ['$event', 'arg'])
onChange(event: any, arg: any): void {}
}
`);
@ -2713,7 +2713,7 @@ runInEachFileSystem(os => {
const hostBindingsFn = `
hostBindings: function Dir_HostBindings(rf, ctx) {
if (rf & 1) {
i0.ɵɵlistener("change", function Dir_change_HostBindingHandler($event) { return ctx.onChange(ctx.arg); });
i0.ɵɵlistener("change", function Dir_change_HostBindingHandler($event) { return ctx.onChange($event, ctx.arg); });
}
}
`;

View File

@ -70,7 +70,8 @@ export type InterpolationFunction = (args: o.Expression[]) => o.Expression;
export function convertActionBinding(
localResolver: LocalResolver | null, implicitReceiver: o.Expression, action: cdAst.AST,
bindingId: string, interpolationFunction?: InterpolationFunction,
baseSourceSpan?: ParseSourceSpan): ConvertActionBindingResult {
baseSourceSpan?: ParseSourceSpan,
implicitReceiverAccesses?: Set<string>): ConvertActionBindingResult {
if (!localResolver) {
localResolver = new DefaultLocalResolver();
}
@ -98,7 +99,8 @@ export function convertActionBinding(
action);
const visitor = new _AstToIrVisitor(
localResolver, implicitReceiver, bindingId, interpolationFunction, baseSourceSpan);
localResolver, implicitReceiver, bindingId, interpolationFunction, baseSourceSpan,
implicitReceiverAccesses);
const actionStmts: o.Statement[] = [];
flattenStatements(actionWithoutBuiltins.visit(visitor, _Mode.Statement), actionStmts);
prependTemporaryDecls(visitor.temporaryCount, bindingId, actionStmts);
@ -313,7 +315,7 @@ class _AstToIrVisitor implements cdAst.AstVisitor {
constructor(
private _localResolver: LocalResolver, private _implicitReceiver: o.Expression,
private bindingId: string, private interpolationFunction: InterpolationFunction|undefined,
private baseSourceSpan?: ParseSourceSpan) {}
private baseSourceSpan?: ParseSourceSpan, private implicitReceiverAccesses?: Set<string>) {}
visitBinary(ast: cdAst.Binary, mode: _Mode): any {
let op: o.BinaryOperator;
@ -493,6 +495,7 @@ class _AstToIrVisitor implements cdAst.AstVisitor {
this.usesImplicitReceiver = prevUsesImplicitReceiver;
result = varExpr.callFn(args);
}
this.addImplicitReceiverAccess(ast.name);
}
if (result == null) {
result = receiver.callMethod(ast.name, args, this.convertSourceSpan(ast.span));
@ -525,6 +528,7 @@ class _AstToIrVisitor implements cdAst.AstVisitor {
// receiver has been replaced with a resolved local expression.
this.usesImplicitReceiver = prevUsesImplicitReceiver;
}
this.addImplicitReceiverAccess(ast.name);
}
if (result == null) {
result = receiver.prop(ast.name);
@ -549,6 +553,7 @@ class _AstToIrVisitor implements cdAst.AstVisitor {
// Restore the previous "usesImplicitReceiver" state since the implicit
// receiver has been replaced with a resolved local expression.
this.usesImplicitReceiver = prevUsesImplicitReceiver;
this.addImplicitReceiverAccess(ast.name);
} else {
// Otherwise it's an error.
const receiver = ast.name;
@ -780,6 +785,13 @@ class _AstToIrVisitor implements cdAst.AstVisitor {
return null;
}
}
/** Adds the name of an AST to the list of implicit receiver accesses. */
private addImplicitReceiverAccess(name: string) {
if (this.implicitReceiverAccesses) {
this.implicitReceiverAccesses.add(name);
}
}
}
function flattenStatements(arg: any, output: o.Statement[]) {

View File

@ -69,13 +69,14 @@ export function prepareEventListenerParameters(
Supported list of global targets: ${Array.from(GLOBAL_TARGET_RESOLVERS.keys())}.`);
}
const eventArgumentName = '$event';
const implicitReceiverAccesses = new Set<string>();
const implicitReceiverExpr = (scope === null || scope.bindingLevel === 0) ?
o.variable(CONTEXT_NAME) :
scope.getOrCreateSharedContextVar(0);
const bindingExpr = convertActionBinding(
scope, implicitReceiverExpr, handler, 'b', () => error('Unexpected interpolation'),
eventAst.handlerSpan);
eventAst.handlerSpan, implicitReceiverAccesses);
const statements = [];
if (scope) {
statements.push(...scope.restoreViewStatement());
@ -86,9 +87,13 @@ export function prepareEventListenerParameters(
const eventName: string =
type === ParsedEventType.Animation ? prepareSyntheticListenerName(name, phase !) : name;
const fnName = handlerName && sanitizeIdentifier(handlerName);
const fnArgs = [new o.FnParam('$event', o.DYNAMIC_TYPE)];
const handlerFn = o.fn(fnArgs, statements, o.INFERRED_TYPE, null, fnName);
const fnArgs: o.FnParam[] = [];
if (implicitReceiverAccesses.has(eventArgumentName)) {
fnArgs.push(new o.FnParam(eventArgumentName, o.DYNAMIC_TYPE));
}
const handlerFn = o.fn(fnArgs, statements, o.INFERRED_TYPE, null, fnName);
const params: o.Expression[] = [o.literal(eventName), handlerFn];
if (target) {
params.push(