fix(ngcc): render adjacent statements after static properties (#33630)

See #33337 (comment)

Fixes FW-1664

PR Close #33630

integration/ngcc/test.sh

@@ -78,6 +78,10 @@ assertSucceeded "Expected 'ngcc' to log 'Compiling'."
   grep "ApplicationModule.ɵmod = ɵngcc0.ɵɵdefineNgModule" node_modules/@angular/core/esm5/src/application_module.js
   assertSucceeded "Expected 'ngcc' to correctly compile 'ApplicationModule' in '@angular/core' (esm5)."
 
+# Did it place the `setClassMetadata` call correctly?
+  cat node_modules/@angular/core/fesm2015/core.js | awk 'ORS=" "' | grep "ApplicationRef.ctorParameters.*setClassMetadata(ApplicationRef"
+  assertSucceeded "Expected 'ngcc' to place 'setClassMetadata' after static properties like 'ctorParameters' in '@angular/core' (fesm2015)."
+
 
 # Did it transform @angular/core typing files correctly?
   grep "import [*] as ɵngcc0 from './src/r3_symbols';" node_modules/@angular/core/core.d.ts

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

@@ -35,4 +35,25 @@ export class Esm5RenderingFormatter extends EsmRenderingFormatter {
     const insertionPoint = returnStatement.getFullStart();
     output.appendLeft(insertionPoint, '\n' + definitions);
   }
+
+  /**
+   * Add the adjacent statements inside the IIFE of each decorated class
+   */
+  addAdjacentStatements(output: MagicString, compiledClass: CompiledClass, statements: string):
+      void {
+    const iifeBody = getIifeBody(compiledClass.declaration);
+    if (!iifeBody) {
+      throw new Error(
+          `Compiled class declaration is not inside an IIFE: ${compiledClass.name} in ${compiledClass.declaration.getSourceFile().fileName}`);
+    }
+
+    const returnStatement = iifeBody.statements.find(ts.isReturnStatement);
+    if (!returnStatement) {
+      throw new Error(
+          `Compiled class wrapper IIFE does not have a return statement: ${compiledClass.name} in ${compiledClass.declaration.getSourceFile().fileName}`);
+    }
+
+    const insertionPoint = returnStatement.getFullStart();
+    output.appendLeft(insertionPoint, '\n' + statements);
+  }
 }

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

@@ -100,6 +100,30 @@ export class EsmRenderingFormatter implements RenderingFormatter {
     output.appendLeft(insertionPoint, '\n' + definitions);
   }
 
+  /**
+   * Add the adjacent statements after all static properties of the class.
+   */
+  addAdjacentStatements(output: MagicString, compiledClass: CompiledClass, statements: string):
+      void {
+    const classSymbol = this.host.getClassSymbol(compiledClass.declaration);
+    if (!classSymbol) {
+      throw new Error(`Compiled class does not have a valid symbol: ${compiledClass.name}`);
+    }
+
+    let insertionPoint = classSymbol.declaration.valueDeclaration.getEnd();
+
+    // If there are static members on this class then insert after the last one
+    if (classSymbol.declaration.exports !== undefined) {
+      classSymbol.declaration.exports.forEach(exportSymbol => {
+        const exportStatement = getContainingStatement(exportSymbol);
+        if (exportStatement !== null) {
+          insertionPoint = Math.max(insertionPoint, exportStatement.getEnd());
+        }
+      });
+    }
+    output.appendLeft(insertionPoint, '\n' + statements);
+  }
+
   /**
    * Remove static decorator properties from classes.
    */
@@ -244,3 +268,21 @@ function getNextSiblingInArray<T extends ts.Node>(node: T, array: ts.NodeArray<T
   const index = array.indexOf(node);
   return index !== -1 && array.length > index + 1 ? array[index + 1] : null;
 }
+
+/**
+ * Find the statement that contains the given class member
+ * @param symbol the symbol of a static member of a class
+ */
+function getContainingStatement(symbol: ts.Symbol): ts.ExpressionStatement|null {
+  if (symbol.valueDeclaration === undefined) {
+    return null;
+  }
+  let node: ts.Node|null = symbol.valueDeclaration;
+  while (node) {
+    if (ts.isExpressionStatement(node)) {
+      break;
+    }
+    node = node.parent;
+  }
+  return node || null;
+}

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

@@ -86,6 +86,10 @@ export class Renderer {
             this.renderDefinitions(compiledFile.sourceFile, clazz, importManager);
         this.srcFormatter.addDefinitions(outputText, clazz, renderedDefinition);
 
+        const renderedStatements =
+            this.renderAdjacentStatements(compiledFile.sourceFile, clazz, importManager);
+        this.srcFormatter.addAdjacentStatements(outputText, clazz, renderedStatements);
+
         if (!isEntryPoint && clazz.reexports.length > 0) {
           this.srcFormatter.addDirectExports(
               outputText, clazz.reexports, importManager, compiledFile.sourceFile);
@@ -147,26 +151,45 @@ export class Renderer {
   }
 
   /**
- * 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.
- */
+   * 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); });
+    return this.renderStatements(sourceFile, statements, imports);
+  }
+
+  /**
+   * Render the adjacent statements as source code for the given class.
+   * @param sourceFile The file containing the class to process.
+   * @param clazz The class whose statements 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 renderAdjacentStatements(
+      sourceFile: ts.SourceFile, compiledClass: CompiledClass, imports: ImportManager): string {
+    const statements: Statement[] = [];
     for (const c of compiledClass.compilation) {
       statements.push(...c.statements);
     }
+    return this.renderStatements(sourceFile, statements, imports);
+  }
+
+  private renderStatements(
+      sourceFile: ts.SourceFile, statements: Statement[], imports: ImportManager): string {
+    const printer = createPrinter();
+    const translate = (stmt: Statement) =>
+        translateStatement(stmt, imports, NOOP_DEFAULT_IMPORT_RECORDER);
+    const print = (stmt: Statement) =>
+        printer.printNode(ts.EmitHint.Unspecified, translate(stmt), sourceFile);
     return statements.map(print).join('\n');
   }
 }

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

@@ -36,6 +36,8 @@ export interface RenderingFormatter {
       output: MagicString, exports: Reexport[], importManager: ImportManager,
       file: ts.SourceFile): void;
   addDefinitions(output: MagicString, compiledClass: CompiledClass, definitions: string): void;
+  addAdjacentStatements(output: MagicString, compiledClass: CompiledClass, statements: string):
+      void;
   removeDecorators(output: MagicString, decoratorsToRemove: RedundantDecoratorMap): void;
   rewriteSwitchableDeclarations(
       outputText: MagicString, sourceFile: ts.SourceFile,

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

@@ -345,6 +345,55 @@ SOME DEFINITION TEXT
       });
     });
 
+    describe('addAdjacentStatements', () => {
+      const contents = `var core = require('@angular/core');\n` +
+          `var SomeDirective = /** @class **/ (function () {\n` +
+          `  function SomeDirective(zone, cons) {}\n` +
+          `  SomeDirective.prototype.method = function() {}\n` +
+          `  SomeDirective.decorators = [\n` +
+          `    { type: core.Directive, args: [{ selector: '[a]' }] },\n` +
+          `    { type: OtherA }\n` +
+          `  ];\n` +
+          `  SomeDirective.ctorParameters = () => [\n` +
+          `    { type: core.NgZone },\n` +
+          `    { type: core.Console }\n` +
+          `  ];\n` +
+          `  return SomeDirective;\n` +
+          `}());\n` +
+          `export {SomeDirective};`;
+
+      it('should insert the statements after all the static methods of the class', () => {
+        const program = {name: _('/node_modules/test-package/some/file.js'), contents};
+        const {renderer, decorationAnalyses, sourceFile} = setup(program);
+        const output = new MagicString(contents);
+        const compiledClass = decorationAnalyses.get(sourceFile) !.compiledClasses.find(
+            c => c.name === 'SomeDirective') !;
+        renderer.addAdjacentStatements(output, compiledClass, 'SOME STATEMENTS');
+        expect(output.toString())
+            .toContain(
+                `  SomeDirective.ctorParameters = () => [\n` +
+                `    { type: core.NgZone },\n` +
+                `    { type: core.Console }\n` +
+                `  ];\n` +
+                `SOME STATEMENTS\n` +
+                `  return SomeDirective;\n`);
+      });
+
+      it('should insert the statements after any definitions', () => {
+        const program = {name: _('/node_modules/test-package/some/file.js'), contents};
+        const {renderer, decorationAnalyses, sourceFile} = setup(program);
+        const output = new MagicString(contents);
+        const compiledClass = decorationAnalyses.get(sourceFile) !.compiledClasses.find(
+            c => c.name === 'SomeDirective') !;
+        renderer.addDefinitions(output, compiledClass, 'SOME DEFINITIONS');
+        renderer.addAdjacentStatements(output, compiledClass, 'SOME STATEMENTS');
+        const definitionsPosition = output.toString().indexOf('SOME DEFINITIONS');
+        const statementsPosition = output.toString().indexOf('SOME STATEMENTS');
+        expect(definitionsPosition).not.toEqual(-1, 'definitions should exist');
+        expect(statementsPosition).not.toEqual(-1, 'statements should exist');
+        expect(statementsPosition).toBeGreaterThan(definitionsPosition);
+      });
+    });
 
     describe('removeDecorators', () => {
 

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

@@ -39,6 +39,9 @@ class TestRenderingFormatter implements RenderingFormatter {
   addDefinitions(output: MagicString, compiledClass: CompiledClass, definitions: string) {
     output.prepend('\n// ADD DEFINITIONS\n');
   }
+  addAdjacentStatements(output: MagicString, compiledClass: CompiledClass, statements: string) {
+    output.prepend('\n// ADD ADJACENT STATEMENTS\n');
+  }
   removeDecorators(output: MagicString, decoratorsToRemove: RedundantDecoratorMap) {
     output.prepend('\n// REMOVE DECORATORS\n');
   }
@@ -79,6 +82,7 @@ function createTestRenderer(
   spyOn(testFormatter, 'addExports').and.callThrough();
   spyOn(testFormatter, 'addImports').and.callThrough();
   spyOn(testFormatter, 'addDefinitions').and.callThrough();
+  spyOn(testFormatter, 'addAdjacentStatements').and.callThrough();
   spyOn(testFormatter, 'addConstants').and.callThrough();
   spyOn(testFormatter, 'removeDecorators').and.callThrough();
   spyOn(testFormatter, 'rewriteSwitchableDeclarations').and.callThrough();

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

@@ -334,6 +334,56 @@ SOME DEFINITION TEXT
       });
     });
 
+    describe('addAdjacentStatements', () => {
+      const contents = `import {Directive, NgZone, Console} from '@angular/core';\n` +
+          `var SomeDirective = /** @class **/ (function () {\n` +
+          `  function SomeDirective(zone, cons) {}\n` +
+          `  SomeDirective.prototype.method = function() {}\n` +
+          `  SomeDirective.decorators = [\n` +
+          `    { type: Directive, args: [{ selector: '[a]' }] },\n` +
+          `    { type: OtherA }\n` +
+          `  ];\n` +
+          `  SomeDirective.ctorParameters = () => [\n` +
+          `    { type: NgZone },\n` +
+          `    { type: Console }\n` +
+          `  ];\n` +
+          `  return SomeDirective;\n` +
+          `}());\n` +
+          `export {SomeDirective};`;
+
+      it('should insert the statements after all the static methods of the class', () => {
+        const program = {name: _('/node_modules/test-package/some/file.js'), contents};
+        const {renderer, decorationAnalyses, sourceFile} = setup(program);
+        const output = new MagicString(contents);
+        const compiledClass = decorationAnalyses.get(sourceFile) !.compiledClasses.find(
+            c => c.name === 'SomeDirective') !;
+        renderer.addAdjacentStatements(output, compiledClass, 'SOME STATEMENTS');
+        expect(output.toString())
+            .toContain(
+                `  SomeDirective.ctorParameters = () => [\n` +
+                `    { type: NgZone },\n` +
+                `    { type: Console }\n` +
+                `  ];\n` +
+                `SOME STATEMENTS\n` +
+                `  return SomeDirective;\n`);
+      });
+
+      it('should insert the statements after any definitions', () => {
+        const program = {name: _('/node_modules/test-package/some/file.js'), contents};
+        const {renderer, decorationAnalyses, sourceFile} = setup(program);
+        const output = new MagicString(contents);
+        const compiledClass = decorationAnalyses.get(sourceFile) !.compiledClasses.find(
+            c => c.name === 'SomeDirective') !;
+        renderer.addDefinitions(output, compiledClass, 'SOME DEFINITIONS');
+        renderer.addAdjacentStatements(output, compiledClass, 'SOME STATEMENTS');
+        const definitionsPosition = output.toString().indexOf('SOME DEFINITIONS');
+        const statementsPosition = output.toString().indexOf('SOME STATEMENTS');
+        expect(definitionsPosition).not.toEqual(-1, 'definitions should exist');
+        expect(statementsPosition).not.toEqual(-1, 'statements should exist');
+        expect(statementsPosition).toBeGreaterThan(definitionsPosition);
+      });
+    });
+
 
     describe('removeDecorators', () => {
 

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

@@ -241,9 +241,54 @@ SOME DEFINITION TEXT
 A.decorators = [
 `);
       });
-
     });
 
+    describe('addAdjacentStatements', () => {
+      const contents = `import {Directive, NgZone, Console} from '@angular/core';\n` +
+          `export class SomeDirective {\n` +
+          `  constructor(zone, cons) {}\n` +
+          `  method() {}\n` +
+          `}\n` +
+          `SomeDirective.decorators = [\n` +
+          `  { type: Directive, args: [{ selector: '[a]' }] },\n` +
+          `  { type: OtherA }\n` +
+          `];\n` +
+          `SomeDirective.ctorParameters = () => [\n` +
+          `  { type: NgZone },\n` +
+          `  { type: Console }\n` +
+          `];`;
+
+      it('should insert the statements after all the static methods of the class', () => {
+        const program = {name: _('/node_modules/test-package/some/file.js'), contents};
+        const {renderer, decorationAnalyses, sourceFile} = setup([program]);
+        const output = new MagicString(contents);
+        const compiledClass = decorationAnalyses.get(sourceFile) !.compiledClasses.find(
+            c => c.name === 'SomeDirective') !;
+        renderer.addAdjacentStatements(output, compiledClass, 'SOME STATEMENTS');
+        expect(output.toString())
+            .toContain(
+                `SomeDirective.ctorParameters = () => [\n` +
+                `  { type: NgZone },\n` +
+                `  { type: Console }\n` +
+                `];\n` +
+                `SOME STATEMENTS`);
+      });
+
+      it('should insert the statements after any definitions', () => {
+        const program = {name: _('/node_modules/test-package/some/file.js'), contents};
+        const {renderer, decorationAnalyses, sourceFile} = setup([program]);
+        const output = new MagicString(contents);
+        const compiledClass = decorationAnalyses.get(sourceFile) !.compiledClasses.find(
+            c => c.name === 'SomeDirective') !;
+        renderer.addDefinitions(output, compiledClass, 'SOME DEFINITIONS');
+        renderer.addAdjacentStatements(output, compiledClass, 'SOME STATEMENTS');
+        const definitionsPosition = output.toString().indexOf('SOME DEFINITIONS');
+        const statementsPosition = output.toString().indexOf('SOME STATEMENTS');
+        expect(definitionsPosition).not.toEqual(-1, 'definitions should exist');
+        expect(statementsPosition).not.toEqual(-1, 'statements should exist');
+        expect(statementsPosition).toBeGreaterThan(definitionsPosition);
+      });
+    });
 
     describe('removeDecorators', () => {
       describe('[static property declaration]', () => {