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.

Sign up for GitHub

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

Jump to bottom

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

Closed
wants to merge 1 commit into from
Closed

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

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
8 changes: 4 additions & 4 deletions packages/compiler-cli/test/compliance/r3_view_compiler_binding_spec.ts
View file
Open in desktop
Expand Up @@ -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(); });
}
}
`;
Expand Down Expand Up @@ -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(); });
}
}
`;
Expand Down Expand Up @@ -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(); });
}
}
`;
Expand Down
2 changes: 1 addition & 1 deletion packages/compiler-cli/test/compliance/r3_view_compiler_directives_spec.ts
View file
Open in desktop
Expand Up @@ -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();
}
},
Expand Down
2 changes: 1 addition & 1 deletion packages/compiler-cli/test/compliance/r3_view_compiler_i18n_spec.ts
View file
Open in desktop
Expand Up @@ -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();
}
Expand Down
119 changes: 111 additions & 8 deletions packages/compiler-cli/test/compliance/r3_view_compiler_listener_spec.ts
View file
Open in desktop
Expand Up @@ -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);
Expand Down Expand Up @@ -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);
Expand Down Expand Up @@ -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();
Expand Down Expand Up @@ -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();
}
}
Expand Down Expand Up @@ -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(); });
}
}
`;
Expand All @@ -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');
});

});
2 changes: 1 addition & 1 deletion packages/compiler-cli/test/compliance/r3_view_compiler_styling_spec.ts
View file
Open in desktop
Expand Up @@ -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);
}
Expand Down
6 changes: 3 additions & 3 deletions packages/compiler-cli/test/compliance/r3_view_compiler_template_spec.ts
View file
Open in desktop
Expand Up @@ -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;
Expand Down Expand Up @@ -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;
Expand Down Expand Up @@ -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$);
Expand Down
8 changes: 4 additions & 4 deletions packages/compiler-cli/test/ngtsc/ngtsc_spec.ts
View file
Open in desktop
Expand Up @@ -2212,7 +2212,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);
}
}
`;
Expand Down Expand Up @@ -2343,7 +2343,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);
Expand Down Expand Up @@ -2411,7 +2411,7 @@ runInEachFileSystem(os => {
selector: '[test]',
})
class Dir {
@HostListener('change', ['arg'])
@HostListener('change', ['$event', 'arg'])
onChange(event: any, arg: any): void {}
}
`);
Expand All @@ -2421,7 +2421,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); });
}
}
`;
Expand Down
18 changes: 15 additions & 3 deletions packages/compiler/src/compiler_util/expression_converter.ts
View file
Open in desktop
Expand Up @@ -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();
}
Expand Down Expand Up @@ -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);
Expand Down Expand Up @@ -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;
Expand Down Expand Up @@ -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));
Expand Down Expand Up @@ -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);
Expand All @@ -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;
Expand Down Expand Up @@ -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[]) {
Expand Down