diff --git a/packages/compiler-cli/ngcc/src/host/delegating_host.ts b/packages/compiler-cli/ngcc/src/host/delegating_host.ts index e72a40a94d01a..8352520f59c76 100644 --- a/packages/compiler-cli/ngcc/src/host/delegating_host.ts +++ b/packages/compiler-cli/ngcc/src/host/delegating_host.ts @@ -161,4 +161,8 @@ export class DelegatingReflectionHost implements NgccReflectionHost { detectKnownDeclaration(decl: T): T { return this.ngccHost.detectKnownDeclaration(decl); } + + isStaticallyExported(clazz: ClassDeclaration): boolean { + return this.ngccHost.isStaticallyExported(clazz); + } } diff --git a/packages/compiler-cli/src/ngtsc/reflection/src/host.ts b/packages/compiler-cli/src/ngtsc/reflection/src/host.ts index 7b6adeadbb65d..851880f5ef3aa 100644 --- a/packages/compiler-cli/src/ngtsc/reflection/src/host.ts +++ b/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; } diff --git a/packages/compiler-cli/src/ngtsc/reflection/src/typescript.ts b/packages/compiler-cli/src/ngtsc/reflection/src/typescript.ts index 451d0c9ae3fd8..3fa2636790333 100644 --- a/packages/compiler-cli/src/ngtsc/reflection/src/typescript.ts +++ b/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 { + 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(); + 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; +} diff --git a/packages/compiler-cli/src/ngtsc/transform/src/compilation.ts b/packages/compiler-cli/src/ngtsc/transform/src/compilation.ts index aabc0cf5a78ce..6c8ba0da9ccfc 100644 --- a/packages/compiler-cli/src/ngtsc/transform/src/compilation.ts +++ b/packages/compiler-cli/src/ngtsc/transform/src/compilation.ts @@ -211,7 +211,7 @@ export class TraitCompiler implements ProgramTypeCheckAdapter { private scanClassForTraits(clazz: ClassDeclaration): PendingTrait[]|null { - if (!this.compileNonExportedClasses && !isExported(clazz)) { + if (!this.compileNonExportedClasses && !this.reflector.isStaticallyExported(clazz)) { return null; } diff --git a/packages/compiler-cli/test/ngtsc/ngtsc_spec.ts b/packages/compiler-cli/test/ngtsc/ngtsc_spec.ts index f98377ea0f8e6..494b6045de76c 100644 --- a/packages/compiler-cli/test/ngtsc/ngtsc_spec.ts +++ b/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', () => {