fix(ngcc): render new definitions using the inner name of the class (#33533)

When decorating classes with ivy definitions (e.g. `ɵfac` or `ɵdir`)
the inner name of the class declaration must be used.

This is because in ES5 the definitions are inside the class's IIFE
where the outer declaration has not yet been initialized.

PR Close #33533

packages/compiler-cli/ngcc/src/packages/transformer.ts

@@ -83,7 +83,7 @@ export class Transformer {
     // Transform the source files and source maps.
     const srcFormatter = this.getRenderingFormatter(reflectionHost, bundle);
 
-    const renderer = new Renderer(srcFormatter, this.fs, this.logger, bundle);
+    const renderer = new Renderer(reflectionHost, srcFormatter, this.fs, this.logger, bundle);
     let renderedFiles = renderer.renderProgram(
         decorationAnalyses, switchMarkerAnalyses, privateDeclarationsAnalyses);
 

packages/compiler-cli/ngcc/src/rendering/renderer.ts

@@ -20,6 +20,7 @@ import {Logger} from '../logging/logger';
 import {FileToWrite, getImportRewriter, stripExtension} from './utils';
 import {RenderingFormatter, RedundantDecoratorMap} from './rendering_formatter';
 import {extractSourceMap, renderSourceAndMap} from './source_maps';
+import {NgccReflectionHost} from '../host/ngcc_host';
 
 /**
  * A base-class for rendering an `AnalyzedFile`.
@@ -29,8 +30,8 @@ import {extractSourceMap, renderSourceAndMap} from './source_maps';
  */
 export class Renderer {
   constructor(
-      private srcFormatter: RenderingFormatter, private fs: FileSystem, private logger: Logger,
+      private host: NgccReflectionHost, private srcFormatter: RenderingFormatter,
-      private bundle: EntryPointBundle) {}
+      private fs: FileSystem, private logger: Logger, private bundle: EntryPointBundle) {}
 
   renderProgram(
       decorationAnalyses: DecorationAnalyses, switchMarkerAnalyses: SwitchMarkerAnalyses,
@@ -81,7 +82,8 @@ export class Renderer {
       this.srcFormatter.removeDecorators(outputText, decoratorsToRemove);
 
       compiledFile.compiledClasses.forEach(clazz => {
-        const renderedDefinition = renderDefinitions(compiledFile.sourceFile, clazz, importManager);
+        const renderedDefinition =
+            this.renderDefinitions(compiledFile.sourceFile, clazz, importManager);
         this.srcFormatter.addDefinitions(outputText, clazz, renderedDefinition);
 
         if (!isEntryPoint && clazz.reexports.length > 0) {
@@ -143,6 +145,30 @@ export class Renderer {
     });
     return decoratorsToRemove;
   }
+
+  /**
+ * Render the definitions as source code for the given class.
+ * @param sourceFile The file containing the class to process.
+ * @param clazz The class whose definitions are to be rendered.
+ * @param compilation The results of analyzing the class - this is used to generate the rendered
+ * definitions.
+ * @param imports An object that tracks the imports that are needed by the rendered definitions.
+ */
+  private renderDefinitions(
+      sourceFile: ts.SourceFile, compiledClass: CompiledClass, imports: ImportManager): string {
+    const printer = createPrinter();
+    const name = this.host.getInternalNameOfClass(compiledClass.declaration);
+    const translate = (stmt: Statement) =>
+        translateStatement(stmt, imports, NOOP_DEFAULT_IMPORT_RECORDER);
+    const print = (stmt: Statement) =>
+        printer.printNode(ts.EmitHint.Unspecified, translate(stmt), sourceFile);
+    const statements: Statement[] = compiledClass.compilation.map(
+        c => { return createAssignmentStatement(name, c.name, c.initializer); });
+    for (const c of compiledClass.compilation) {
+      statements.push(...c.statements);
+    }
+    return statements.map(print).join('\n');
+  }
 }
 
 /**
@@ -157,30 +183,6 @@ export function renderConstantPool(
       .join('\n');
 }
 
-/**
- * Render the definitions as source code for the given class.
- * @param sourceFile The file containing the class to process.
- * @param clazz The class whose definitions are to be rendered.
- * @param compilation The results of analyzing the class - this is used to generate the rendered
- * definitions.
- * @param imports An object that tracks the imports that are needed by the rendered definitions.
- */
-export function renderDefinitions(
-    sourceFile: ts.SourceFile, compiledClass: CompiledClass, imports: ImportManager): string {
-  const printer = createPrinter();
-  const name = compiledClass.declaration.name;
-  const translate = (stmt: Statement) =>
-      translateStatement(stmt, imports, NOOP_DEFAULT_IMPORT_RECORDER);
-  const print = (stmt: Statement) =>
-      printer.printNode(ts.EmitHint.Unspecified, translate(stmt), sourceFile);
-  const statements: Statement[] =
-      compiledClass.compilation.map(c => createAssignmentStatement(name, c.name, c.initializer));
-  for (const c of compiledClass.compilation) {
-    statements.push(...c.statements);
-  }
-  return statements.map(print).join('\n');
-}
-
 /**
  * Create an Angular AST statement node that contains the assignment of the
  * compiled decorator to be applied to the class.

packages/compiler-cli/ngcc/test/rendering/renderer_spec.ts

@@ -20,6 +20,7 @@ import {ModuleWithProvidersInfo} from '../../src/analysis/module_with_providers_
 import {PrivateDeclarationsAnalyzer, ExportInfo} from '../../src/analysis/private_declarations_analyzer';
 import {SwitchMarkerAnalyzer} from '../../src/analysis/switch_marker_analyzer';
 import {Esm2015ReflectionHost} from '../../src/host/esm2015_host';
+import {Esm5ReflectionHost} from '../../src/host/esm5_host';
 import {Renderer} from '../../src/rendering/renderer';
 import {MockLogger} from '../helpers/mock_logger';
 import {RenderingFormatter, RedundantDecoratorMap} from '../../src/rendering/rendering_formatter';
@@ -55,7 +56,8 @@ class TestRenderingFormatter implements RenderingFormatter {
 }
 
 function createTestRenderer(
-    packageName: string, files: TestFile[], dtsFiles?: TestFile[], mappingFiles?: TestFile[]) {
+    packageName: string, files: TestFile[], dtsFiles?: TestFile[], mappingFiles?: TestFile[],
+    isEs5 = false) {
   const logger = new MockLogger();
   loadTestFiles(files);
   if (dtsFiles) {
@@ -67,9 +69,10 @@ function createTestRenderer(
   const fs = getFileSystem();
   const isCore = packageName === '@angular/core';
   const bundle = makeTestEntryPointBundle(
-      'test-package', 'esm2015', isCore, getRootFiles(files), dtsFiles && getRootFiles(dtsFiles));
+      'test-package', 'esm5', isCore, getRootFiles(files), dtsFiles && getRootFiles(dtsFiles));
   const typeChecker = bundle.src.program.getTypeChecker();
-  const host = new Esm2015ReflectionHost(logger, isCore, typeChecker, bundle.dts);
+  const host = isEs5 ? new Esm5ReflectionHost(logger, isCore, typeChecker, bundle.dts) :
+                       new Esm2015ReflectionHost(logger, isCore, typeChecker, bundle.dts);
   const referencesRegistry = new NgccReferencesRegistry(host);
   const decorationAnalyses =
       new DecorationAnalyzer(fs, bundle, host, referencesRegistry).analyzeProgram();
@@ -86,7 +89,7 @@ function createTestRenderer(
   spyOn(testFormatter, 'rewriteSwitchableDeclarations').and.callThrough();
   spyOn(testFormatter, 'addModuleWithProvidersParams').and.callThrough();
 
-  const renderer = new Renderer(testFormatter, fs, logger, bundle);
+  const renderer = new Renderer(host, testFormatter, fs, logger, bundle);
 
   return {renderer,
           testFormatter,
@@ -284,6 +287,125 @@ A.ɵdir = ɵngcc0.ɵɵdefineDirective({ type: A, selectors: [["", "a", ""]] });
               .toBeGreaterThan(ngInjectorDef, 'setClassMetadata should follow ɵinj');
         });
 
+        it('should render directives using the inner class name if different from outer', () => {
+          const {renderer, decorationAnalyses, switchMarkerAnalyses, privateDeclarationsAnalyses,
+                 testFormatter} =
+              createTestRenderer(
+                  'test-package', [{
+                    name: _('/node_modules/test-package/src/file.js'),
+                    contents: `
+                      import { Directive } from '@angular/core';
+                      var OuterClass = /** @class */ (function () {
+                        function InnerClass() {}
+                        return InnerClass;
+                      }());
+                      OuterClass.decorators = [{ type: Directive, args: [{ selector: '[test]' }]
+                      export OuterClass;`
+                  }],
+                  undefined, undefined, /* isEs5 */ true);
+
+          renderer.renderProgram(
+              decorationAnalyses, switchMarkerAnalyses, privateDeclarationsAnalyses);
+
+          const addDefinitionsSpy = testFormatter.addDefinitions as jasmine.Spy;
+          const output = addDefinitionsSpy.calls.first().args[2];
+          expect(output).toContain('InnerClass.ɵfac');
+          expect(output).toContain('new (t || InnerClass)');
+          expect(output).toContain('InnerClass.ɵdir');
+          expect(output).toContain('type: InnerClass');
+          expect(output).toContain('ɵsetClassMetadata(InnerClass');
+        });
+
+        it('should render injectables using the inner class name if different from outer', () => {
+          const {renderer, decorationAnalyses, switchMarkerAnalyses, privateDeclarationsAnalyses,
+                 testFormatter} =
+              createTestRenderer(
+                  'test-package', [{
+                    name: _('/node_modules/test-package/src/file.js'),
+                    contents: `
+                      import { Injectable } from '@angular/core';
+                      var OuterClass = /** @class */ (function () {
+                        function InnerClass() {}
+                        return InnerClass;
+                      }());
+                      OuterClass.decorators = [{ type: Injectable }]
+                      export OuterClass;`
+                  }],
+                  undefined, undefined, /* isEs5 */ true);
+
+          renderer.renderProgram(
+              decorationAnalyses, switchMarkerAnalyses, privateDeclarationsAnalyses);
+
+          const addDefinitionsSpy = testFormatter.addDefinitions as jasmine.Spy;
+          const output = addDefinitionsSpy.calls.first().args[2];
+          expect(output).toContain('InnerClass.ɵfac');
+          expect(output).toContain('new (t || InnerClass)()');
+          expect(output).toContain('InnerClass.ɵprov');
+          expect(output).toContain('token: InnerClass');
+          expect(output).toContain('ɵsetClassMetadata(InnerClass');
+        });
+
+        it('should render ng-modules using the inner class name if different from outer', () => {
+          const {renderer, decorationAnalyses, switchMarkerAnalyses, privateDeclarationsAnalyses,
+                 testFormatter} =
+              createTestRenderer(
+                  'test-package', [{
+                    name: _('/node_modules/test-package/src/file.js'),
+                    contents: `
+                      import { NgModule, Directive } from '@angular/core';
+                      var DirectiveClass = /** @class */ (function () {
+                        function DirectiveClass() {}
+                        return DirectiveClass;
+                      }());
+                      DirectiveClass.decorators = [{ type: Directive, args: [{selector: 'x'}] }];
+                      var OuterClass = /** @class */ (function () {
+                        function InnerClass() {}
+                        return InnerClass;
+                      }());
+                      OuterClass.decorators = [{ type: NgModule, args: [{declarations: [DirectiveClass], exports: [DirectiveClass]}]
+                      export OuterClass;`
+                  }],
+                  undefined, undefined, /* isEs5 */ true);
+
+          renderer.renderProgram(
+              decorationAnalyses, switchMarkerAnalyses, privateDeclarationsAnalyses);
+
+          const addDefinitionsSpy = testFormatter.addDefinitions as jasmine.Spy;
+          const output = addDefinitionsSpy.calls.all()[1].args[2];
+          expect(output).toContain('InnerClass.ɵmod');
+          expect(output).toContain('type: InnerClass');
+          expect(output).toContain('ɵɵsetNgModuleScope(InnerClass');
+          expect(output).toContain('ɵsetClassMetadata(InnerClass');
+        });
+
+        it('should render pipes using the inner class name if different from outer', () => {
+          const {renderer, decorationAnalyses, switchMarkerAnalyses, privateDeclarationsAnalyses,
+                 testFormatter} =
+              createTestRenderer(
+                  'test-package', [{
+                    name: _('/node_modules/test-package/src/file.js'),
+                    contents: `
+                      import { Pipe } from '@angular/core';
+                      var OuterClass = /** @class */ (function () {
+                        function InnerClass() {}
+                        return InnerClass;
+                      }());
+                      OuterClass.decorators = [{ type: Pipe, args: [{name: 'pipe'}]
+                      export OuterClass;`
+                  }],
+                  undefined, undefined, /* isEs5 */ true);
+
+          renderer.renderProgram(
+              decorationAnalyses, switchMarkerAnalyses, privateDeclarationsAnalyses);
+
+          const addDefinitionsSpy = testFormatter.addDefinitions as jasmine.Spy;
+          const output = addDefinitionsSpy.calls.first().args[2];
+          expect(output).toContain('InnerClass.ɵfac');
+          expect(output).toContain('new (t || InnerClass)()');
+          expect(output).toContain('InnerClass.ɵpipe');
+          expect(output).toContain('ɵsetClassMetadata(InnerClass');
+        });
+
         it('should render classes without decorators if class fields are decorated', () => {
           const {renderer, decorationAnalyses, switchMarkerAnalyses, privateDeclarationsAnalyses,
                  testFormatter} =