fix(ngcc): ensure that adjacent statements go after helper calls (#33689

)

Previously the renderers were fixed so that they inserted extra
"adjacent" statements after the last static property of classes.

In order to help the build-optimizer (in Angular CLI) to be able to
tree-shake classes effectively, these statements should also appear
after any helper calls, such as `__decorate()`.

This commit moves the computation of this positioning into the
`NgccReflectionHost` via the `getEndOfClass()` method, which
returns the last statement that is related to the class.

FW-1668

PR Close #33689

packages/compiler-cli/ngcc/src/host/esm2015_host.ts

@@ -534,6 +534,35 @@ export class Esm2015ReflectionHost extends TypeScriptReflectionHost implements N
     return infos;
   }
 
+  getEndOfClass(classSymbol: NgccClassSymbol): ts.Node {
+    let last: ts.Node = classSymbol.declaration.valueDeclaration;
+
+    // If there are static members on this class then find the last one
+    if (classSymbol.declaration.exports !== undefined) {
+      classSymbol.declaration.exports.forEach(exportSymbol => {
+        if (exportSymbol.valueDeclaration === undefined) {
+          return;
+        }
+        const exportStatement = getContainingStatement(exportSymbol.valueDeclaration);
+        if (exportStatement !== null && last.getEnd() < exportStatement.getEnd()) {
+          last = exportStatement;
+        }
+      });
+    }
+
+    // If there are helper calls for this class then find the last one
+    const helpers = this.getHelperCallsForClass(
+        classSymbol, ['__decorate', '__extends', '__param', '__metadata']);
+    helpers.forEach(helper => {
+      const helperStatement = getContainingStatement(helper);
+      if (helperStatement !== null && last.getEnd() < helperStatement.getEnd()) {
+        last = helperStatement;
+      }
+    });
+
+    return last;
+  }
+
   ///////////// Protected Helpers /////////////
 
   /**
@@ -1891,3 +1920,17 @@ function isSynthesizedSuperCall(expression: ts.Expression): boolean {
   return ts.isSpreadElement(argument) && ts.isIdentifier(argument.expression) &&
       argument.expression.text === 'arguments';
 }
+
+/**
+ * Find the statement that contains the given node
+ * @param node a node whose containing statement we wish to find
+ */
+function getContainingStatement(node: ts.Node): ts.ExpressionStatement|null {
+  while (node) {
+    if (ts.isExpressionStatement(node)) {
+      break;
+    }
+    node = node.parent;
+  }
+  return node || null;
+}

packages/compiler-cli/ngcc/src/host/esm5_host.ts

@@ -103,6 +103,22 @@ export class Esm5ReflectionHost extends Esm2015ReflectionHost {
     return this.getInternalNameOfClass(clazz);
   }
 
+  getEndOfClass(classSymbol: NgccClassSymbol): ts.Node {
+    const iifeBody = getIifeBody(classSymbol.declaration.valueDeclaration);
+    if (!iifeBody) {
+      throw new Error(
+          `Compiled class declaration is not inside an IIFE: ${classSymbol.name} in ${classSymbol.declaration.valueDeclaration.getSourceFile().fileName}`);
+    }
+
+    const returnStatementIndex = iifeBody.statements.findIndex(ts.isReturnStatement);
+    if (returnStatementIndex === -1) {
+      throw new Error(
+          `Compiled class wrapper IIFE does not have a return statement: ${classSymbol.name} in ${classSymbol.declaration.valueDeclaration.getSourceFile().fileName}`);
+    }
+
+    // Return the statement before the IIFE return statement
+    return iifeBody.statements[returnStatementIndex - 1];
+  }
   /**
    * In ES5, the implementation of a class is a function expression that is hidden inside an IIFE,
    * whose value is assigned to a variable (which represents the class to the rest of the program).

packages/compiler-cli/ngcc/src/host/ngcc_host.ts

@@ -116,4 +116,16 @@ export interface NgccReflectionHost extends ReflectionHost {
    * objects.
    */
   getModuleWithProvidersFunctions(f: ts.SourceFile): ModuleWithProvidersFunction[];
+
+  /**
+   * Find the last node that is relevant to the specified class.
+   *
+   * As well as the main declaration, classes can have additional statements such as static
+   * properties (`SomeClass.staticProp = ...;`) and decorators (`__decorate(SomeClass, ...);`).
+   * It is useful to know exactly where the class "ends" so that we can inject additional
+   * statements after that point.
+   *
+   * @param classSymbol The class whose statements we want.
+   */
+  getEndOfClass(classSymbol: NgccClassSymbol): ts.Node;
 }

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

@@ -35,25 +35,4 @@ 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

@@ -109,19 +109,8 @@ export class EsmRenderingFormatter implements RenderingFormatter {
     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);
+    const endOfClass = this.host.getEndOfClass(classSymbol);
+    output.appendLeft(endOfClass.getEnd(), '\n' + statements);
   }
 
   /**
@@ -268,21 +257,3 @@ 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/test/host/esm2015_host_import_helper_spec.ts

@@ -486,6 +486,20 @@ runInEachFileSystem(() => {
             expect(value).toBe(null);
           });
         });
+
+        describe('getEndOfClass()', () => {
+          it('should return the last statement related to the class', () => {
+            const {program} = makeTestBundleProgram(_('/ngmodule.js'));
+            const host =
+                new Esm2015ReflectionHost(new MockLogger(), false, program.getTypeChecker());
+            const classSymbol =
+                host.findClassSymbols(program.getSourceFile(_('/ngmodule.js')) !)[0];
+            const endOfClass = host.getEndOfClass(classSymbol);
+            expect(endOfClass.getText())
+                .toMatch(
+                    /HttpClientXsrfModule = HttpClientXsrfModule_1 = .*__decorate.*\(\[\n\s*NgModule\(\{\n\s*providers: \[],\n\s*}\)\n\s*], HttpClientXsrfModule\);/);
+          });
+        });
       });
     });
 

packages/compiler-cli/ngcc/test/host/esm2015_host_spec.ts

@@ -2160,5 +2160,62 @@ runInEachFileSystem(() => {
             ]);
       });
     });
+
+    describe('getEndOfClass()', () => {
+      it('should return the last static property of the class', () => {
+        const testFile: TestFile = {
+          name: _('/node_modules/test-package/some/file.js'),
+          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` +
+              `];\n` +
+              `callSomeFunction();\n` +
+              `var value = 100;\n`
+        };
+        loadTestFiles([testFile]);
+        const {program} = makeTestBundleProgram(testFile.name);
+        const host = new Esm2015ReflectionHost(new MockLogger(), false, program.getTypeChecker());
+        const classSymbol = host.findClassSymbols(program.getSourceFile(testFile.name) !)[0];
+        const endOfClass = host.getEndOfClass(classSymbol);
+        expect(endOfClass.getText())
+            .toEqual(
+                `SomeDirective.ctorParameters = () => [\n` +
+                `  { type: NgZone },\n` +
+                `  { type: Console }\n` +
+                `];`);
+      });
+
+      it('should return the class declaration if there are no extra statements', () => {
+        const testFile: TestFile = {
+          name: _('/node_modules/test-package/some/file.js'),
+          contents: `export class SomeDirective {\n` +
+              `  constructor(zone, cons) {}\n` +
+              `  method() {}\n` +
+              `}\n` +
+              `callSomeFunction();\n` +
+              `var value = 100;\n`
+        };
+        loadTestFiles([testFile]);
+        const {program} = makeTestBundleProgram(testFile.name);
+        const host = new Esm2015ReflectionHost(new MockLogger(), false, program.getTypeChecker());
+        const classSymbol = host.findClassSymbols(program.getSourceFile(testFile.name) !)[0];
+        const endOfClass = host.getEndOfClass(classSymbol);
+        expect(endOfClass.getText())
+            .toEqual(
+                `export class SomeDirective {\n` +
+                `  constructor(zone, cons) {}\n` +
+                `  method() {}\n` +
+                `}`);
+      });
+    });
   });
 });

packages/compiler-cli/ngcc/test/host/esm5_host_import_helper_spec.ts

@@ -532,6 +532,19 @@ export { SomeDirective };
             expect(value).toBe(null);
           });
         });
+
+        describe('getEndOfClass()', () => {
+          it('should return the last statement related to the class', () => {
+            const {program} = makeTestBundleProgram(_('/ngmodule.js'));
+            const host = new Esm5ReflectionHost(new MockLogger(), false, program.getTypeChecker());
+            const classSymbol =
+                host.findClassSymbols(program.getSourceFile(_('/ngmodule.js')) !)[0];
+            const endOfClass = host.getEndOfClass(classSymbol);
+            expect(endOfClass.getText())
+                .toMatch(
+                    /HttpClientXsrfModule = HttpClientXsrfModule_1 = .*__decorate.*\(\[\n\s*NgModule\(\{\n\s*providers: \[],\n\s*}\)\n\s*], HttpClientXsrfModule\);/);
+          });
+        });
       });
 
       function findVariableDeclaration(

packages/compiler-cli/ngcc/test/host/esm5_host_spec.ts

@@ -2530,5 +2530,20 @@ runInEachFileSystem(() => {
         ]);
       });
     });
+
+    describe('getEndOfClass()', () => {
+      it('should return the last static property of the class', () => {
+        loadTestFiles([SOME_DIRECTIVE_FILE]);
+        const {program} = makeTestBundleProgram(SOME_DIRECTIVE_FILE.name);
+        const host = new Esm5ReflectionHost(new MockLogger(), false, program.getTypeChecker());
+        const classSymbol =
+            host.findClassSymbols(program.getSourceFile(SOME_DIRECTIVE_FILE.name) !)[0];
+        const endOfClass = host.getEndOfClass(classSymbol);
+        expect(endOfClass.getText()).toEqual(`SomeDirective.propDecorators = {
+        "input1": [{ type: Input },],
+        "input2": [{ type: Input },],
+      };`);
+      });
+    });
   });
 });