fix(ivy): move HostListeners generation to factory function (#26480)

PR Close #26480

packages/compiler-cli/test/ngtsc/ngtsc_spec.ts

@@ -469,7 +469,7 @@ describe('ngtsc behavioral tests', () => {
           @HostBinding('class.someclass')
           get someClass(): boolean { return false; }
 
-          @HostListener('onChange', ['arg'])
+          @HostListener('change', ['arg1', 'arg2', 'arg3'])
           onChange(event: any, arg: any): void {}
         }
     `);
@@ -483,8 +483,51 @@ describe('ngtsc behavioral tests', () => {
     expect(jsContents)
         .toContain(
             'i0.ɵelementProperty(elIndex, "class.someclass", i0.ɵbind(i0.ɵload(dirIndex).someClass))');
-    expect(jsContents).toContain('i0.ɵload(dirIndex).onClick($event)');
+
-    expect(jsContents).toContain('i0.ɵload(dirIndex).onChange(i0.ɵload(dirIndex).arg)');
+    const factoryDef = `
+      factory: function FooCmp_Factory(t) {
+        var f = new (t || FooCmp)();
+        i0.ɵlistener("click", function FooCmp_click_HostBindingHandler($event) {
+          return f.onClick($event);
+        });
+        i0.ɵlistener("change", function FooCmp_change_HostBindingHandler($event) {
+          return f.onChange(f.arg1, f.arg2, f.arg3);
+        });
+        return f;
+      }
+    `;
+    expect(jsContents).toContain(factoryDef.replace(/\s+/g, ' ').trim());
+  });
+
+  it('should generate host listeners for directives with base factories', () => {
+    env.tsconfig();
+    env.write(`test.ts`, `
+        import {Directive, HostListener} from '@angular/core';
+
+        class Base {}
+
+        @Directive({
+          selector: '[test]',
+        })
+        class Dir extends Base {
+          @HostListener('change', ['arg'])
+          onChange(event: any, arg: any): void {}
+        }
+    `);
+
+    env.driveMain();
+    const jsContents = env.getContents('test.js');
+    const factoryDef = `
+      factory: function Dir_Factory(t) {
+        var f = ɵDir_BaseFactory((t || Dir));
+        i0.ɵlistener("change", function Dir_change_HostBindingHandler($event) {
+          return f.onChange(f.arg);
+        });
+        return f;
+      }
+    `;
+    expect(jsContents).toContain(factoryDef.replace(/\s+/g, ' ').trim());
+    expect(jsContents).toContain('var ɵDir_BaseFactory = i0.ɵgetInheritedFactory(Dir)');
   });
 
   it('should correctly recognize local symbols', () => {

packages/compiler/src/injectable_compiler_2.ts

@@ -42,6 +42,7 @@ export function compileInjectable(meta: R3InjectableMetadata): InjectableDef {
     type: meta.type,
     deps: meta.ctorDeps,
     injectFn: Identifiers.inject,
+    extraStatementFn: null,
   };
 
   if (meta.useClass !== undefined) {

packages/compiler/src/render3/r3_factory.ts

@@ -49,6 +49,11 @@ export interface R3ConstructorFactoryMetadata {
    * function could be different, and other options control how it will be invoked.
    */
   injectFn: o.ExternalReference;
+
+  /**
+   * Function that allows extra statements to be inserted into factory function.
+   */
+  extraStatementFn: ((instance: o.Expression) => o.Statement[])|null;
 }
 
 export enum R3FactoryDelegateType {
@@ -189,8 +194,7 @@ export function compileFactoryFunction(meta: R3FactoryMetadata):
         ]);
 
     statements.push(delegateFactoryStmt);
-    const r = makeConditionalFactory(delegateFactory.callFn([]));
+    retExpr = makeConditionalFactory(delegateFactory.callFn([]));
-    retExpr = r;
   } else if (isDelegatedMetadata(meta)) {
     // This type is created with a delegated factory. If a type parameter is not specified, call
     // the factory instead.
@@ -204,10 +208,22 @@ export function compileFactoryFunction(meta: R3FactoryMetadata):
   } else if (isExpressionFactoryMetadata(meta)) {
     // TODO(alxhub): decide whether to lower the value here or in the caller
     retExpr = makeConditionalFactory(meta.expression);
+  } else if (meta.extraStatementFn) {
+    // if extraStatementsFn is specified and the 'makeConditionalFactory' function
+    // was not invoked, we need to create a reference to the instance, so we can
+    // pass it as an argument to the 'extraStatementFn' function while calling it
+    const variable = o.variable('f');
+    body.push(variable.set(ctorExpr).toDeclStmt());
+    retExpr = variable;
   } else {
     retExpr = ctorExpr;
   }
 
+  if (meta.extraStatementFn) {
+    const extraStmts = meta.extraStatementFn(retExpr);
+    body.push(...extraStmts);
+  }
+
   return {
     factory: o.fn(
         [new o.FnParam('t', o.DYNAMIC_TYPE)], [...body, new o.ReturnStatement(retExpr)],

packages/compiler/src/render3/r3_module_compiler.ts

@@ -102,6 +102,7 @@ export function compileInjector(meta: R3InjectorMetadata): R3InjectorDef {
     type: meta.type,
     deps: meta.deps,
     injectFn: R3.inject,
+    extraStatementFn: null,
   });
   const expression = o.importExpr(R3.defineInjector).callFn([mapToMapExpression({
     factory: result.factory,
@@ -152,4 +153,4 @@ function accessExportScope(module: o.Expression): o.Expression {
 function tupleTypeOf(exp: R3Reference[]): o.Type {
   const types = exp.map(ref => o.typeofExpr(ref.type));
   return exp.length > 0 ? o.expressionType(o.literalArr(types)) : o.NONE_TYPE;
-}
+}

packages/compiler/src/render3/r3_pipe_compiler.ts

@@ -43,6 +43,7 @@ export function compilePipeFromMetadata(metadata: R3PipeMetadata) {
     type: metadata.type,
     deps: metadata.deps,
     injectFn: R3.directiveInject,
+    extraStatementFn: null,
   });
   definitionMapValues.push({key: 'factory', value: templateFactory.factory, quoted: false});
 

packages/compiler/src/render3/view/compiler.ts

@@ -12,6 +12,7 @@ import {CompileReflector} from '../../compile_reflector';
 import {BindingForm, convertActionBinding, convertPropertyBinding} from '../../compiler_util/expression_converter';
 import {ConstantPool, DefinitionKind} from '../../constant_pool';
 import * as core from '../../core';
+import {ParsedEvent} from '../../expression_parser/ast';
 import {LifecycleHooks} from '../../lifecycle_reflector';
 import * as o from '../../output/output_ast';
 import {typeSourceSpan} from '../../parse_util';
@@ -49,6 +50,7 @@ function baseDirectiveFields(
     type: meta.type,
     deps: meta.deps,
     injectFn: R3.directiveInject,
+    extraStatementFn: createFactoryExtraStatementsFn(meta, bindingParser)
   });
   definitionMap.set('factory', result.factory);
 
@@ -634,26 +636,6 @@ function createHostBindingsFunction(
     }
   }
 
-  // Calculate host event bindings
-  const eventBindings =
-      bindingParser.createDirectiveHostEventAsts(directiveSummary, hostBindingSourceSpan);
-  if (eventBindings) {
-    for (const binding of eventBindings) {
-      const bindingExpr = convertActionBinding(
-          null, bindingContext, binding.handler, 'b', () => error('Unexpected interpolation'));
-      const bindingName = binding.name && sanitizeIdentifier(binding.name);
-      const typeName = meta.name;
-      const functionName =
-          typeName && bindingName ? `${typeName}_${bindingName}_HostBindingHandler` : null;
-      const handler = o.fn(
-          [new o.FnParam('$event', o.DYNAMIC_TYPE)],
-          [...bindingExpr.stmts, new o.ReturnStatement(bindingExpr.allowDefault)], o.INFERRED_TYPE,
-          null, functionName);
-      statements.push(
-          o.importExpr(R3.listener).callFn([o.literal(binding.name), handler]).toStmt());
-    }
-  }
-
   if (statements.length > 0) {
     const typeName = meta.name;
     return o.fn(
@@ -667,6 +649,32 @@ function createHostBindingsFunction(
   return null;
 }
 
+function createFactoryExtraStatementsFn(meta: R3DirectiveMetadata, bindingParser: BindingParser):
+    ((instance: o.Expression) => o.Statement[])|null {
+  const eventBindings =
+      bindingParser.createDirectiveHostEventAsts(metadataAsSummary(meta), meta.typeSourceSpan);
+  return eventBindings && eventBindings.length ?
+      (instance: o.Expression) => createHostListeners(instance, eventBindings, meta) :
+      null;
+}
+
+function createHostListeners(
+    bindingContext: o.Expression, eventBindings: ParsedEvent[],
+    meta: R3DirectiveMetadata): o.Statement[] {
+  return eventBindings.map(binding => {
+    const bindingExpr = convertActionBinding(
+        null, bindingContext, binding.handler, 'b', () => error('Unexpected interpolation'));
+    const bindingName = binding.name && sanitizeIdentifier(binding.name);
+    const typeName = meta.name;
+    const functionName =
+        typeName && bindingName ? `${typeName}_${bindingName}_HostBindingHandler` : null;
+    const handler = o.fn(
+        [new o.FnParam('$event', o.DYNAMIC_TYPE)], [...bindingExpr.render3Stmts], o.INFERRED_TYPE,
+        null, functionName);
+    return o.importExpr(R3.listener).callFn([o.literal(binding.name), handler]).toStmt();
+  });
+}
+
 function metadataAsSummary(meta: R3DirectiveMetadata): CompileDirectiveSummary {
   // clang-format off
   return {

packages/core/test/render3/listeners_spec.ts

@@ -11,7 +11,7 @@ import {container, containerRefreshEnd, containerRefreshStart, element, elementE
 import {RenderFlags} from '../../src/render3/interfaces/definition';
 
 import {getRendererFactory2} from './imported_renderer2';
-import {ComponentFixture, containerEl, renderComponent, renderToHtml, requestAnimationFrame} from './render_util';
+import {ComponentFixture, containerEl, renderToHtml, requestAnimationFrame} from './render_util';
 
 
 describe('event listeners', () => {
@@ -86,8 +86,10 @@ describe('event listeners', () => {
   beforeEach(() => { comps = []; });
 
   it('should call function on event emit', () => {
-    const comp = renderComponent(MyComp);
+    const fixture = new ComponentFixture(MyComp);
-    const button = containerEl.querySelector('button') !;
+    const comp = fixture.component;
+    const button = fixture.hostElement.querySelector('button') !;
+
     button.click();
     expect(comp.counter).toEqual(1);
 
@@ -96,8 +98,9 @@ describe('event listeners', () => {
   });
 
   it('should retain event handler return values using document', () => {
-    const preventDefaultComp = renderComponent(PreventDefaultComp);
+    const fixture = new ComponentFixture(PreventDefaultComp);
-    const button = containerEl.querySelector('button') !;
+    const preventDefaultComp = fixture.component;
+    const button = fixture.hostElement.querySelector('button') !;
 
     button.click();
     expect(preventDefaultComp.event !.preventDefault).not.toHaveBeenCalled();
@@ -112,9 +115,10 @@ describe('event listeners', () => {
   });
 
   it('should retain event handler return values with renderer2', () => {
-    const preventDefaultComp =
+    const fixture =
-        renderComponent(PreventDefaultComp, {rendererFactory: getRendererFactory2(document)});
+        new ComponentFixture(PreventDefaultComp, {rendererFactory: getRendererFactory2(document)});
-    const button = containerEl.querySelector('button') !;
+    const preventDefaultComp = fixture.component;
+    const button = fixture.hostElement.querySelector('button') !;
 
     button.click();
     expect(preventDefaultComp.event !.preventDefault).not.toHaveBeenCalled();
@@ -479,6 +483,44 @@ describe('event listeners', () => {
     expect(events).toEqual(['click!', 'click!']);
   });
 
+  it('should support listeners with specified set of args', () => {
+    class MyComp {
+      counter = 0;
+      data = {a: 1, b: 2};
+
+      onClick(a: any, b: any) { this.counter += a + b; }
+
+      static ngComponentDef = defineComponent({
+        type: MyComp,
+        selectors: [['comp']],
+        consts: 2,
+        vars: 0,
+        /** <button (click)="onClick(data.a, data.b)"> Click me </button> */
+        template: function CompTemplate(rf: RenderFlags, ctx: any) {
+          if (rf & RenderFlags.Create) {
+            elementStart(0, 'button');
+            {
+              listener('click', function() { return ctx.onClick(ctx.data.a, ctx.data.b); });
+              text(1, 'Click me');
+            }
+            elementEnd();
+          }
+        },
+        factory: () => new MyComp()
+      });
+    }
+
+    const fixture = new ComponentFixture(MyComp);
+    const comp = fixture.component;
+    const button = fixture.hostElement.querySelector('button') !;
+
+    button.click();
+    expect(comp.counter).toEqual(3);
+
+    button.click();
+    expect(comp.counter).toEqual(6);
+  });
+
   it('should destroy listeners in nested views', () => {
 
     /**