From cfb8c17511a2b07fe3e275936f8e29c835f66a92 Mon Sep 17 00:00:00 2001 From: Pete Bacon Darwin Date: Thu, 29 Nov 2018 08:26:00 +0000 Subject: [PATCH] feat(ivy): ngcc - map functions as well as classes from source to typings (#27326) To support updating `ModuleWithProviders` calls, we need to be able to map exported functions between source and typings files, as well as classes. PR Close #27326 --- .../analysis/private_declarations_analyzer.ts | 2 +- .../src/ngcc/src/host/esm2015_host.ts | 69 +++++++++---------- .../src/ngcc/src/rendering/renderer.ts | 2 +- .../src/ngcc/test/host/esm2015_host_spec.ts | 28 +++++--- .../src/ngtsc/annotations/src/ng_module.ts | 6 +- .../src/ngtsc/host/src/reflection.ts | 6 +- .../src/ngtsc/metadata/src/reflector.ts | 2 +- 7 files changed, 62 insertions(+), 53 deletions(-) diff --git a/packages/compiler-cli/src/ngcc/src/analysis/private_declarations_analyzer.ts b/packages/compiler-cli/src/ngcc/src/analysis/private_declarations_analyzer.ts index 26d3f839a0..fb2505dca5 100644 --- a/packages/compiler-cli/src/ngcc/src/analysis/private_declarations_analyzer.ts +++ b/packages/compiler-cli/src/ngcc/src/analysis/private_declarations_analyzer.ts @@ -52,7 +52,7 @@ export class PrivateDeclarationsAnalyzer { return Array.from(privateDeclarations.keys()).map(id => { const from = id.getSourceFile().fileName; const declaration = privateDeclarations.get(id) !; - const dtsDeclaration = this.host.getDtsDeclarationOfClass(declaration.node); + const dtsDeclaration = this.host.getDtsDeclaration(declaration.node); const dtsFrom = dtsDeclaration && dtsDeclaration.getSourceFile().fileName; return {identifier: id.text, from, dtsFrom}; }); diff --git a/packages/compiler-cli/src/ngcc/src/host/esm2015_host.ts b/packages/compiler-cli/src/ngcc/src/host/esm2015_host.ts index f8125716c8..6d9f09af89 100644 --- a/packages/compiler-cli/src/ngcc/src/host/esm2015_host.ts +++ b/packages/compiler-cli/src/ngcc/src/host/esm2015_host.ts @@ -49,10 +49,10 @@ export const CONSTRUCTOR_PARAMS = 'ctorParameters' as ts.__String; * a static method called `ctorParameters`. */ export class Esm2015ReflectionHost extends TypeScriptReflectionHost implements NgccReflectionHost { - protected dtsClassMap: Map|null; + protected dtsDeclarationMap: Map|null; constructor(protected isCore: boolean, checker: ts.TypeChecker, dts?: BundleProgram|null) { super(checker); - this.dtsClassMap = dts && this.computeDtsClassMap(dts.path, dts.program) || null; + this.dtsDeclarationMap = dts && this.computeDtsDeclarationMap(dts.path, dts.program) || null; } /** @@ -327,15 +327,15 @@ export class Esm2015ReflectionHost extends TypeScriptReflectionHost implements N * is not a class or has an unknown number of type parameters. */ getGenericArityOfClass(clazz: ts.Declaration): number|null { - const dtsClass = this.getDtsDeclarationOfClass(clazz); - if (dtsClass) { - return dtsClass.typeParameters ? dtsClass.typeParameters.length : 0; + const dtsDeclaration = this.getDtsDeclaration(clazz); + if (dtsDeclaration && ts.isClassDeclaration(dtsDeclaration)) { + return dtsDeclaration.typeParameters ? dtsDeclaration.typeParameters.length : 0; } return null; } /** - * Take an exported declaration of a class (maybe downleveled to a variable) and look up the + * Take an exported declaration of a class (maybe down-leveled to a variable) and look up the * declaration of its type in a separate .d.ts tree. * * This function is allowed to return `null` if the current compilation unit does not have a @@ -346,20 +346,17 @@ export class Esm2015ReflectionHost extends TypeScriptReflectionHost implements N * Note that the `ts.ClassDeclaration` returned from this function may not be from the same * `ts.Program` as the input declaration. */ - getDtsDeclarationOfClass(declaration: ts.Declaration): ts.ClassDeclaration|null { - if (this.dtsClassMap) { - if (ts.isClassDeclaration(declaration)) { - if (!declaration.name || !ts.isIdentifier(declaration.name)) { - throw new Error( - `Cannot get the dts file for a class declaration that has no indetifier: ${declaration.getText()} in ${declaration.getSourceFile().fileName}`); - } - return this.dtsClassMap.get(declaration.name.text) || null; - } + getDtsDeclaration(declaration: ts.Declaration): ts.Declaration|null { + if (!this.dtsDeclarationMap) { + return null; } - return null; + if (!isNamedDeclaration(declaration)) { + throw new Error( + `Cannot get the dts file for a declaration that has no name: ${declaration.getText()} in ${declaration.getSourceFile().fileName}`); + } + return this.dtsDeclarationMap.get(declaration.name.text) || null; } - ///////////// Protected Helpers ///////////// protected getDecoratorsOfSymbol(symbol: ts.Symbol): Decorator[]|null { @@ -738,7 +735,7 @@ export class Esm2015ReflectionHost extends TypeScriptReflectionHost implements N } if (!name) { - if (isNamedDeclaration(node) && node.name && ts.isIdentifier(node.name)) { + if (isNamedDeclaration(node)) { name = node.name.text; nameNode = node.name; } else { @@ -846,8 +843,8 @@ export class Esm2015ReflectionHost extends TypeScriptReflectionHost implements N } /** - * Get the parameter type and decorators for a class where the information is stored on - * in calls to `__decorate` helpers. + * Get the parameter type and decorators for a class where the information is stored via + * calls to `__decorate` helpers. * * Reflect over the helpers to find the decorators and types about each of * the class's constructor parameters. @@ -1002,9 +999,9 @@ export class Esm2015ReflectionHost extends TypeScriptReflectionHost implements N * @param dtsProgram The program containing all the typings files. * @returns a map of class names to class declarations. */ - protected computeDtsClassMap(dtsRootFileName: string, dtsProgram: ts.Program): - Map { - const dtsClassMap = new Map(); + protected computeDtsDeclarationMap(dtsRootFileName: string, dtsProgram: ts.Program): + Map { + const dtsDeclarationMap = new Map(); const checker = dtsProgram.getTypeChecker(); // First add all the classes that are publicly exported from the entry-point @@ -1012,13 +1009,13 @@ export class Esm2015ReflectionHost extends TypeScriptReflectionHost implements N if (!rootFile) { throw new Error(`The given file ${dtsRootFileName} is not part of the typings program.`); } - collectExportedClasses(checker, dtsClassMap, rootFile); + collectExportedDeclarations(checker, dtsDeclarationMap, rootFile); // Now add any additional classes that are exported from individual dts files, // but are not publicly exported from the entry-point. dtsProgram.getSourceFiles().forEach( - sourceFile => { collectExportedClasses(checker, dtsClassMap, sourceFile); }); - return dtsClassMap; + sourceFile => { collectExportedDeclarations(checker, dtsDeclarationMap, sourceFile); }); + return dtsDeclarationMap; } } @@ -1129,8 +1126,10 @@ function isThisAssignment(node: ts.Declaration): node is ts.BinaryExpression& node.left.expression.kind === ts.SyntaxKind.ThisKeyword; } -function isNamedDeclaration(node: ts.Declaration): node is ts.NamedDeclaration { - return !!(node as any).name; +function isNamedDeclaration(node: ts.Declaration): node is ts.NamedDeclaration& + {name: ts.Identifier} { + const anyNode: any = node; + return !!anyNode.name && ts.isIdentifier(anyNode.name); } @@ -1153,13 +1152,11 @@ function getFarLeftIdentifier(propertyAccess: ts.PropertyAccessExpression): ts.I } /** - * Search a source file for exported classes, storing them in the provided `dtsClassMap`. - * @param checker The typechecker for the source program. - * @param dtsClassMap The map in which to store the collected exported classes. - * @param srcFile The source file to search for exported classes. + * Collect mappings between exported declarations in a source file and its associated + * declaration in the typings program. */ -function collectExportedClasses( - checker: ts.TypeChecker, dtsClassMap: Map, +function collectExportedDeclarations( + checker: ts.TypeChecker, dtsDeclarationMap: Map, srcFile: ts.SourceFile): void { const srcModule = srcFile && checker.getSymbolAtLocation(srcFile); const moduleExports = srcModule && checker.getExportsOfModule(srcModule); @@ -1170,8 +1167,8 @@ function collectExportedClasses( } const declaration = exportedSymbol.valueDeclaration; const name = exportedSymbol.name; - if (declaration && ts.isClassDeclaration(declaration) && !dtsClassMap.has(name)) { - dtsClassMap.set(name, declaration); + if (declaration && !dtsDeclarationMap.has(name)) { + dtsDeclarationMap.set(name, declaration); } }); } diff --git a/packages/compiler-cli/src/ngcc/src/rendering/renderer.ts b/packages/compiler-cli/src/ngcc/src/rendering/renderer.ts index 26bb1ebfa8..0c1c39fd97 100644 --- a/packages/compiler-cli/src/ngcc/src/rendering/renderer.ts +++ b/packages/compiler-cli/src/ngcc/src/rendering/renderer.ts @@ -307,7 +307,7 @@ export abstract class Renderer { const dtsMap = new Map(); analyses.forEach(compiledFile => { compiledFile.compiledClasses.forEach(compiledClass => { - const dtsDeclaration = this.host.getDtsDeclarationOfClass(compiledClass.declaration); + const dtsDeclaration = this.host.getDtsDeclaration(compiledClass.declaration); if (dtsDeclaration) { const dtsFile = dtsDeclaration.getSourceFile(); const classes = dtsMap.get(dtsFile) || []; diff --git a/packages/compiler-cli/src/ngcc/test/host/esm2015_host_spec.ts b/packages/compiler-cli/src/ngcc/test/host/esm2015_host_spec.ts index 1b2d2ede9e..0b8def3fb8 100644 --- a/packages/compiler-cli/src/ngcc/test/host/esm2015_host_spec.ts +++ b/packages/compiler-cli/src/ngcc/test/host/esm2015_host_spec.ts @@ -453,6 +453,7 @@ const TYPINGS_SRC_FILES = [ }, {name: '/src/class1.js', contents: 'export class Class1 {}\nexport class MissingClass1 {}'}, {name: '/src/class2.js', contents: 'export class Class2 {}'}, + {name: '/src/func1.js', contents: 'export function mooFn() {}'}, {name: '/src/internal.js', contents: 'export class InternalClass {}\nexport class Class2 {}'}, {name: '/src/missing-class.js', contents: 'export class MissingClass2 {}'}, { name: '/src/flat-file.js', @@ -476,6 +477,7 @@ const TYPINGS_DTS_FILES = [ contents: `export declare class Class2 {}\nexport declare interface SomeInterface {}\nexport {Class3 as xClass3} from './class3';` }, + {name: '/typings/func1.d.ts', contents: 'export declare function mooFn(): void;'}, { name: '/typings/internal.d.ts', contents: `export declare class InternalClass {}\nexport declare class Class2 {}` @@ -1286,10 +1288,20 @@ describe('Fesm2015ReflectionHost', () => { const class1 = getDeclaration(srcProgram, '/src/class1.js', 'Class1', ts.isClassDeclaration); const host = new Esm2015ReflectionHost(false, srcProgram.getTypeChecker(), dts); - const dtsDeclaration = host.getDtsDeclarationOfClass(class1); + const dtsDeclaration = host.getDtsDeclaration(class1); expect(dtsDeclaration !.getSourceFile().fileName).toEqual('/typings/class1.d.ts'); }); + it('should find the dts declaration for exported functions', () => { + const srcProgram = makeTestProgram(...TYPINGS_SRC_FILES); + const dtsProgram = makeTestBundleProgram(TYPINGS_DTS_FILES); + const mooFn = getDeclaration(srcProgram, '/src/func1.js', 'mooFn', ts.isFunctionDeclaration); + const host = new Esm2015ReflectionHost(false, srcProgram.getTypeChecker(), dtsProgram); + + const dtsDeclaration = host.getDtsDeclaration(mooFn); + expect(dtsDeclaration !.getSourceFile().fileName).toEqual('/typings/func1.d.ts'); + }); + it('should return null if there is no matching class in the matching dts file', () => { const srcProgram = makeTestProgram(...TYPINGS_SRC_FILES); const dts = makeTestBundleProgram(TYPINGS_DTS_FILES); @@ -1297,7 +1309,7 @@ describe('Fesm2015ReflectionHost', () => { getDeclaration(srcProgram, '/src/class1.js', 'MissingClass1', ts.isClassDeclaration); const host = new Esm2015ReflectionHost(false, srcProgram.getTypeChecker(), dts); - expect(host.getDtsDeclarationOfClass(missingClass)).toBe(null); + expect(host.getDtsDeclaration(missingClass)).toBe(null); }); it('should return null if there is no matching dts file', () => { @@ -1307,7 +1319,7 @@ describe('Fesm2015ReflectionHost', () => { srcProgram, '/src/missing-class.js', 'MissingClass2', ts.isClassDeclaration); const host = new Esm2015ReflectionHost(false, srcProgram.getTypeChecker(), dts); - expect(host.getDtsDeclarationOfClass(missingClass)).toBe(null); + expect(host.getDtsDeclaration(missingClass)).toBe(null); }); it('should find the dts file that contains a matching class declaration, even if the source files do not match', @@ -1318,7 +1330,7 @@ describe('Fesm2015ReflectionHost', () => { getDeclaration(srcProgram, '/src/flat-file.js', 'Class1', ts.isClassDeclaration); const host = new Esm2015ReflectionHost(false, srcProgram.getTypeChecker(), dts); - const dtsDeclaration = host.getDtsDeclarationOfClass(class1); + const dtsDeclaration = host.getDtsDeclaration(class1); expect(dtsDeclaration !.getSourceFile().fileName).toEqual('/typings/class1.d.ts'); }); @@ -1329,7 +1341,7 @@ describe('Fesm2015ReflectionHost', () => { getDeclaration(srcProgram, '/src/flat-file.js', 'Class3', ts.isClassDeclaration); const host = new Esm2015ReflectionHost(false, srcProgram.getTypeChecker(), dts); - const dtsDeclaration = host.getDtsDeclarationOfClass(class3); + const dtsDeclaration = host.getDtsDeclaration(class3); expect(dtsDeclaration !.getSourceFile().fileName).toEqual('/typings/class3.d.ts'); }); @@ -1341,7 +1353,7 @@ describe('Fesm2015ReflectionHost', () => { getDeclaration(srcProgram, '/src/internal.js', 'InternalClass', ts.isClassDeclaration); const host = new Esm2015ReflectionHost(false, srcProgram.getTypeChecker(), dts); - const dtsDeclaration = host.getDtsDeclarationOfClass(internalClass); + const dtsDeclaration = host.getDtsDeclaration(internalClass); expect(dtsDeclaration !.getSourceFile().fileName).toEqual('/typings/internal.d.ts'); }); @@ -1355,10 +1367,10 @@ describe('Fesm2015ReflectionHost', () => { getDeclaration(srcProgram, '/src/internal.js', 'Class2', ts.isClassDeclaration); const host = new Esm2015ReflectionHost(false, srcProgram.getTypeChecker(), dts); - const class2DtsDeclaration = host.getDtsDeclarationOfClass(class2); + const class2DtsDeclaration = host.getDtsDeclaration(class2); expect(class2DtsDeclaration !.getSourceFile().fileName).toEqual('/typings/class2.d.ts'); - const internalClass2DtsDeclaration = host.getDtsDeclarationOfClass(internalClass2); + const internalClass2DtsDeclaration = host.getDtsDeclaration(internalClass2); expect(internalClass2DtsDeclaration !.getSourceFile().fileName) .toEqual('/typings/class2.d.ts'); }); diff --git a/packages/compiler-cli/src/ngtsc/annotations/src/ng_module.ts b/packages/compiler-cli/src/ngtsc/annotations/src/ng_module.ts index 0b9e025873..a19261665d 100644 --- a/packages/compiler-cli/src/ngtsc/annotations/src/ng_module.ts +++ b/packages/compiler-cli/src/ngtsc/annotations/src/ng_module.ts @@ -109,7 +109,7 @@ export class NgModuleDecoratorHandler implements DecoratorHandler