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

…#42207)

The compiler flag `compileNonExportedClasses` allows the Angular compiler to
process classes which are not exported at the top level of a source file.
This is often used to allow for AOT compilation of test classes inside
`it()` test blocks, for example.

Previously, the compiler would identify exported classes by looking for an
`export` modifier on the class declaration itself. This works for the
trivial case, but fails for indirectly exported classes:

```typescript
// Component is declared unexported.
@component({...})
class FooCmp {...}

// Indirect export of FooCmp
export {FooCmp};
```

This is not an immediate problem for most application builds, since the
default value for `compileNonExportedClasses` is `true` and therefore such
classes get compiled regardless.

However, in the Angular Language Service now, `compileNonExportedClasses` is
forcibly overridden to `false`. That's because the tsconfig used by the IDE
and Language Service is often far broader than the application build's
configuration, and pulls in test files that can contain unexported classes
not designed with AOT compilation in mind.

Therefore, the Language Service has trouble working with such structures.

In this commit, the `ReflectionHost` gains a new API for detecting whether a
class is exported. The implementation of this method now not only considers
the `export` modifier, but also scans the `ts.SourceFile` for indirect
exports like the example above. This ensures the above case will be
processed directly in the Language Service.

This new operation is cached using an expando symbol on the `ts.SourceFile`,
ensuring good performance even when scanning large source files with lots of
exports (e.g. a FESM file under `ngcc`).

Fixes #42184.

PR Close #42207

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

@@ -7395,6 +7395,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', () => {