refactor(ivy): remove unnecessary fac wrapper (#34076)

For injectables, we currently generate a factory function in the
injectable def (prov) that delegates to the factory function in
the factory def (fac). It looks something like this:

```
factory: function(t) { return Svc.fac(t); }
```

The extra wrapper function is unnecessary since the args for
the factory functions are the same. This commit changes the
compiler to generate this instead:

```
factory: Svc.fac
```

Because we are generating less code for each injectable, we
should see some modest code size savings. AIO's main bundle
is about 1 KB smaller.

PR Close #34076

aio/scripts/_payload-limits.json

@@ -12,7 +12,7 @@
     "master": {
       "uncompressed": {
         "runtime-es2015": 2987,
-        "main-es2015": 463671,
+        "main-es2015": 462449,
         "polyfills-es2015": 52503
       }
     }

integration/_payload-limits.json

@@ -3,7 +3,7 @@
     "master": {
       "uncompressed": {
         "runtime-es2015": 1485,
-        "main-es2015": 142186,
+        "main-es2015": 141476,
         "polyfills-es2015": 36808
       }
     }
@@ -21,7 +21,7 @@
     "master": {
       "uncompressed": {
         "runtime-es2015": 1485,
-        "main-es2015": 148320,
+        "main-es2015": 147610,
         "polyfills-es2015": 36808
       }
     }

packages/compiler-cli/ngcc/src/analysis/decoration_analyzer.ts

@@ -94,6 +94,11 @@ export class DecorationAnalyzer {
     new DirectiveDecoratorHandler(
         this.reflectionHost, this.evaluator, this.fullRegistry, NOOP_DEFAULT_IMPORT_RECORDER,
         this.isCore, /* annotateForClosureCompiler */ false),
+    // Pipe handler must be before injectable handler in list so pipe factories are printed
+    // before injectable factories (so injectable factories can delegate to them)
+    new PipeDecoratorHandler(
+        this.reflectionHost, this.evaluator, this.metaRegistry, NOOP_DEFAULT_IMPORT_RECORDER,
+        this.isCore),
     new InjectableDecoratorHandler(
         this.reflectionHost, NOOP_DEFAULT_IMPORT_RECORDER, this.isCore,
         /* strictCtorDeps */ false, /* errorOnDuplicateProv */ false),
@@ -101,9 +106,6 @@ export class DecorationAnalyzer {
         this.reflectionHost, this.evaluator, this.fullMetaReader, this.fullRegistry,
         this.scopeRegistry, this.referencesRegistry, this.isCore, /* routeAnalyzer */ null,
         this.refEmitter, NOOP_DEFAULT_IMPORT_RECORDER, /* annotateForClosureCompiler */ false),
-    new PipeDecoratorHandler(
-        this.reflectionHost, this.evaluator, this.metaRegistry, NOOP_DEFAULT_IMPORT_RECORDER,
-        this.isCore),
   ];
   migrations: Migration[] = [
     new UndecoratedParentMigration(),

packages/compiler-cli/ngcc/test/integration/ngcc_spec.ts

@@ -205,13 +205,43 @@ runInEachFileSystem(() => {
       expect(countOccurrences(after, 'ɵfac')).toEqual(1);
     });
 
+    // This is necessary to ensure XPipeDef.fac is defined when delegated from injectable def
+    it('should always generate factory def (fac) before injectable def (prov)', () => {
+      compileIntoFlatEs5Package('test-package', {
+        '/index.ts': `
+        import {Injectable, Pipe, PipeTransform} from '@angular/core';
+       
+        @Injectable()
+        @Pipe({
+          name: 'myTestPipe'
+        })
+        export class TestClass implements PipeTransform {
+          transform(value: any) { return value; }
+        }
+        `,
+      });
+
+      mainNgcc({
+        basePath: '/node_modules',
+        targetEntryPointPath: 'test-package',
+        propertiesToConsider: ['main'],
+      });
+
+      const jsContents = fs.readFile(_(`/node_modules/test-package/index.js`));
+      expect(jsContents)
+          .toContain(
+              `TestClass.ɵfac = function TestClass_Factory(t) { return new (t || TestClass)(); };\n` +
+              `TestClass.ɵpipe = ɵngcc0.ɵɵdefinePipe({ name: "myTestPipe", type: TestClass, pure: true });\n` +
+              `TestClass.ɵprov = ɵngcc0.ɵɵdefineInjectable({`);
+    });
+
     it('should add generic type for ModuleWithProviders and generate exports for private modules',
        () => {
          compileIntoApf('test-package', {
            '/index.ts': `
               import {ModuleWithProviders} from '@angular/core';
               import {InternalFooModule} from './internal';
-              
+
               export class FooModule {
                 static forRoot(): ModuleWithProviders {
                   return {
@@ -222,7 +252,7 @@ runInEachFileSystem(() => {
             `,
            '/internal.ts': `
               import {NgModule} from '@angular/core';
-  
+
               @NgModule()
               export class InternalFooModule {}
            `,

packages/compiler-cli/src/ngtsc/program.ts

@@ -629,6 +629,10 @@ export class NgtscProgram implements api.Program {
       new DirectiveDecoratorHandler(
           this.reflector, evaluator, metaRegistry, this.defaultImportTracker, this.isCore,
           this.closureCompilerEnabled),
+      // Pipe handler must be before injectable handler in list so pipe factories are printed
+      // before injectable factories (so injectable factories can delegate to them)
+      new PipeDecoratorHandler(
+          this.reflector, evaluator, metaRegistry, this.defaultImportTracker, this.isCore),
       new InjectableDecoratorHandler(
           this.reflector, this.defaultImportTracker, this.isCore,
           this.options.strictInjectionParameters || false),
@@ -636,8 +640,6 @@ export class NgtscProgram implements api.Program {
           this.reflector, evaluator, this.metaReader, metaRegistry, scopeRegistry,
           referencesRegistry, this.isCore, this.routeAnalyzer, this.refEmitter,
           this.defaultImportTracker, this.closureCompilerEnabled, this.options.i18nInLocale),
-      new PipeDecoratorHandler(
-          this.reflector, evaluator, metaRegistry, this.defaultImportTracker, this.isCore),
     ];
 
     return new IvyCompilation(

packages/compiler-cli/test/compliance/r3_view_compiler_di_spec.ts

@@ -90,9 +90,7 @@ describe('compiler compliance: dependency injection', () => {
     const def = `
       MyService.ɵprov = $r3$.ɵɵdefineInjectable({
         token: MyService,
-        factory: function(t) {
-          return MyService.ɵfac(t);
-        },
+        factory: MyService.ɵfac,
         providedIn: null
       });
     `;
@@ -342,16 +340,22 @@ describe('compiler compliance: dependency injection', () => {
        const result = compile(files, angularFiles);
        const source = result.source;
 
-       const MyPipeFactory = `
-        MyPipe.ɵfac = function MyPipe_Factory(t) { return new (t || MyPipe)($r3$.ɵɵdirectiveInject(Service)); };
+       // The prov definition must be last so MyPipe.fac is defined
+       const MyPipeDefs = `
+        MyPipe.ɵfac = function MyPipe_Factory(t) { return new (t || MyPipe)(i0.ɵɵdirectiveInject(Service)); };
+        MyPipe.ɵpipe = i0.ɵɵdefinePipe({ name: "myPipe", type: MyPipe, pure: true });
+        MyPipe.ɵprov = i0.ɵɵdefineInjectable({ token: MyPipe, factory: MyPipe.ɵfac, providedIn: null });
       `;
 
-       const MyOtherPipeFactory = `
+       // The prov definition must be last so MyOtherPipe.fac is defined
+       const MyOtherPipeDefs = `
         MyOtherPipe.ɵfac = function MyOtherPipe_Factory(t) { return new (t || MyOtherPipe)($r3$.ɵɵdirectiveInject(Service)); };
+        MyOtherPipe.ɵpipe = i0.ɵɵdefinePipe({ name: "myOtherPipe", type: MyOtherPipe, pure: true });
+        MyOtherPipe.ɵprov = i0.ɵɵdefineInjectable({ token: MyOtherPipe, factory: MyOtherPipe.ɵfac, providedIn: null });
       `;
 
-       expectEmit(source, MyPipeFactory, 'Invalid pipe factory function');
-       expectEmit(source, MyOtherPipeFactory, 'Invalid pipe factory function');
+       expectEmit(source, MyPipeDefs, 'Invalid pipe factory function');
+       expectEmit(source, MyOtherPipeDefs, 'Invalid pipe factory function');
        expect(source.match(/MyPipe\.ɵfac =/g) !.length).toBe(1);
        expect(source.match(/MyOtherPipe\.ɵfac =/g) !.length).toBe(1);
      });

packages/compiler/src/injectable_compiler_2.ts

@@ -68,7 +68,8 @@ export function compileInjectable(meta: R3InjectableMetadata): InjectableDef {
     } else if (useClassOnSelf) {
       result = compileFactoryFunction(factoryMeta);
     } else {
-      result = delegateToFactory(meta.useClass);
+      result = delegateToFactory(
+          meta.type as o.WrappedNodeExpr<any>, meta.useClass as o.WrappedNodeExpr<any>);
     }
   } else if (meta.useFactory !== undefined) {
     if (meta.userDeps !== undefined) {
@@ -99,7 +100,8 @@ export function compileInjectable(meta: R3InjectableMetadata): InjectableDef {
       expression: o.importExpr(Identifiers.inject).callFn([meta.useExisting]),
     });
   } else {
-    result = delegateToFactory(meta.internalType);
+    result = delegateToFactory(
+        meta.type as o.WrappedNodeExpr<any>, meta.internalType as o.WrappedNodeExpr<any>);
   }
 
   const token = meta.internalType;
@@ -117,11 +119,15 @@ export function compileInjectable(meta: R3InjectableMetadata): InjectableDef {
   };
 }
 
-function delegateToFactory(type: o.Expression) {
+function delegateToFactory(type: o.WrappedNodeExpr<any>, internalType: o.WrappedNodeExpr<any>) {
   return {
     statements: [],
-    // () => type.ɵfac(t)
-    factory: o.fn([new o.FnParam('t', o.DYNAMIC_TYPE)], [new o.ReturnStatement(type.callMethod(
-                                                            'ɵfac', [o.variable('t')]))])
+    // If types are the same, we can generate `factory: type.ɵfac`
+    // If types are different, we have to generate a wrapper function to ensure
+    // the internal type has been resolved (`factory: function(t) { return type.ɵfac(t); }`)
+    factory: type.node === internalType.node ?
+        internalType.prop('ɵfac') :
+        o.fn([new o.FnParam('t', o.DYNAMIC_TYPE)], [new o.ReturnStatement(internalType.callMethod(
+                                                       'ɵfac', [o.variable('t')]))])
   };
 }