fix(compiler-cli): better detect classes that are indirectly exported

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

@@ -161,4 +161,8 @@ export class DelegatingReflectionHost implements NgccReflectionHost {
   detectKnownDeclaration<T extends Declaration>(decl: T): T {
     return this.ngccHost.detectKnownDeclaration(decl);
   }
+
+  isStaticallyExported(clazz: ClassDeclaration): boolean {
+    return this.ngccHost.isStaticallyExported(clazz);
+  }
 }

packages/compiler-cli/src/ngtsc/reflection/src/host.ts

@@ -856,4 +856,14 @@ export interface ReflectionHost {
    * have a different name than it does externally.
    */
   getAdjacentNameOfClass(clazz: ClassDeclaration): ts.Identifier;
+
+  /**
+   * Returns `true` if a class is exported from the module in which it's defined.
+   *
+   * Not all mechanisms by which a class is exported can be statically detected, especially when
+   * processing already compiled JavaScript. A `false` result does not indicate that the class is
+   * never visible outside its module, only that it was not exported via one of the export
+   * mechanisms that the `ReflectionHost` is capable of statically checking.
+   */
+  isStaticallyExported(clazz: ClassDeclaration): boolean;
 }

packages/compiler-cli/src/ngtsc/reflection/src/typescript.ts

@@ -199,6 +199,35 @@ export class TypeScriptReflectionHost implements ReflectionHost {
     return clazz.name;
   }
 
+  isStaticallyExported(clazz: ClassDeclaration): boolean {
+    // First check if there's an `export` modifier directly on the class declaration.
+    let topLevel: ts.Node = clazz;
+    if (ts.isVariableDeclaration(clazz) && ts.isVariableDeclarationList(clazz.parent)) {
+      topLevel = clazz.parent.parent;
+    }
+    if (topLevel.modifiers !== undefined &&
+        topLevel.modifiers.some(modifier => modifier.kind === ts.SyntaxKind.ExportKeyword)) {
+      // The node is part of a declaration that's directly exported.
+      return true;
+    }
+
+    // If `topLevel` is not directly exported via a modifier, then it might be indirectly exported,
+    // e.g.:
+    //
+    // class Foo {}
+    // export {Foo};
+    //
+    // The only way to check this is to look at the module level for exports of the class. As a
+    // performance optimization, this check is only performed if the class is actually declared at
+    // the top level of the file and thus eligible for exporting in the first place.
+    if (topLevel.parent === undefined || !ts.isSourceFile(topLevel.parent)) {
+      return false;
+    }
+
+    const localExports = this.getLocalExportedClassesOfSourceFile(clazz.getSourceFile());
+    return localExports.has(clazz);
+  }
+
   protected getDirectImportOfIdentifier(id: ts.Identifier): Import|null {
     const symbol = this.checker.getSymbolAtLocation(id);
 
@@ -413,6 +442,58 @@ export class TypeScriptReflectionHost implements ReflectionHost {
       isStatic,
     };
   }
+
+  /**
+   * Get the set of classes declared in `file` which are exported.
+   */
+  private getLocalExportedClassesOfSourceFile(file: ts.SourceFile): Set<ClassDeclaration> {
+    const cacheSf: SourceFileWithCachedExports = file as SourceFileWithCachedExports;
+    if (cacheSf[LocalExportedClasses] !== undefined) {
+      // TS does not currently narrow symbol-keyed fields, hence the non-null assert is needed.
+      return cacheSf[LocalExportedClasses]!;
+    }
+
+    const exportSet = new Set<ClassDeclaration>();
+    cacheSf[LocalExportedClasses] = exportSet;
+
+    const sfSymbol = this.checker.getSymbolAtLocation(cacheSf);
+
+    if (sfSymbol === undefined || sfSymbol.exports === undefined) {
+      return exportSet;
+    }
+
+    // Scan the exported symbol of the `ts.SourceFile` for the original `symbol` of the class
+    // declaration.
+    //
+    // Note: when checking multiple classes declared in the same file, this repeats some operations.
+    // In theory, this could be expensive if run in the context of a massive input file (like a
+    // large FESM in ngcc). If performance does become an issue here, it should be possible to
+    // create a `Set<>`
+
+    // Unfortunately, `ts.Iterator` doesn't implement the iterator protocol, so iteration here is
+    // done manually.
+    const iter = sfSymbol.exports.values();
+    let item = iter.next();
+    while (item.done !== true) {
+      let exportedSymbol = item.value;
+
+      // If this exported symbol comes from an `export {Foo}` statement, then the symbol is actually
+      // for the export declaration, not the original declaration. Such a symbol will be an alias,
+      // so unwrap aliasing if necessary.
+      if (exportedSymbol.flags & ts.SymbolFlags.Alias) {
+        exportedSymbol = this.checker.getAliasedSymbol(exportedSymbol);
+      }
+
+      if (exportedSymbol.valueDeclaration !== undefined &&
+          exportedSymbol.valueDeclaration.getSourceFile() === file &&
+          this.isClass(exportedSymbol.valueDeclaration)) {
+        exportSet.add(exportedSymbol.valueDeclaration);
+      }
+      item = iter.next();
+    }
+
+    return exportSet;
+  }
 }
 
 export function reflectNameOfDeclaration(decl: ts.Declaration): string|null {
@@ -593,3 +674,26 @@ function getExportedName(decl: ts.Declaration, originalId: ts.Identifier): strin
       (decl.propertyName !== undefined ? decl.propertyName : decl.name).text :
       originalId.text;
 }
+
+const LocalExportedClasses = Symbol('LocalExportedClasses');
+
+/**
+ * A `ts.SourceFile` expando which includes a cached `Set` of local `ClassDeclarations` that are
+ * exported either directly (`export class ...`) or indirectly (via `export {...}`).
+ *
+ * This cache does not cause memory leaks as:
+ *
+ *  1. The only references cached here are local to the `ts.SourceFile`, and thus also available in
+ *     `this.statements`.
+ *
+ *  2. The only way this `Set` could change is if the source file itself was changed, which would
+ *     invalidate the entire `ts.SourceFile` object in favor of a new version. Thus, changing the
+ *     source file also invalidates this cache.
+ */
+interface SourceFileWithCachedExports extends ts.SourceFile {
+  /**
+   * Cached `Set` of `ClassDeclaration`s which are locally declared in this file and are exported
+   * either directly or indirectly.
+   */
+  [LocalExportedClasses]?: Set<ClassDeclaration>;
+}

packages/compiler-cli/src/ngtsc/transform/src/compilation.ts

@@ -211,7 +211,7 @@ export class TraitCompiler implements ProgramTypeCheckAdapter {
 
   private scanClassForTraits(clazz: ClassDeclaration):
       PendingTrait<unknown, unknown, SemanticSymbol|null, unknown>[]|null {
-    if (!this.compileNonExportedClasses && !isExported(clazz)) {
+    if (!this.compileNonExportedClasses && !this.reflector.isStaticallyExported(clazz)) {
       return null;
     }
 

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

@@ -7405,6 +7405,24 @@ export const Foo = Foo__PRE_R3__;
         expect(jsContents).not.toContain('defineNgModule(');
         expect(jsContents).toContain('NgModule({');
       });
+
+      it('should still compile a class that is indirectly exported', () => {
+        env.write('test.ts', `
+          import {Component} from '@angular/core';
+
+          @Component({
+            selector: 'test-cmp',
+            template: 'Test Cmp',
+          })
+          class TestCmp {}
+
+          export {TestCmp};
+        `);
+        env.driveMain();
+        const jsContents = env.getContents('test.js');
+
+        expect(jsContents).toContain('defineComponent');
+      });
     });
 
     describe('undecorated providers', () => {