diff --git a/packages/compiler-cli/ngcc/src/analysis/module_with_providers_analyzer.ts b/packages/compiler-cli/ngcc/src/analysis/module_with_providers_analyzer.ts index 119a4fe5cd..8be4952c74 100644 --- a/packages/compiler-cli/ngcc/src/analysis/module_with_providers_analyzer.ts +++ b/packages/compiler-cli/ngcc/src/analysis/module_with_providers_analyzer.ts @@ -38,30 +38,17 @@ export class ModuleWithProvidersAnalyzer { rootFiles.forEach(f => { const fns = this.host.getModuleWithProvidersFunctions(f); fns && fns.forEach(fn => { + if (fn.ngModule.viaModule === null) { + // Record the usage of an internal module as it needs to become an exported symbol + this.referencesRegistry.add(fn.ngModule.node, new Reference(fn.ngModule.node)); + } + const dtsFn = this.getDtsDeclarationForFunction(fn); const typeParam = dtsFn.type && ts.isTypeReferenceNode(dtsFn.type) && dtsFn.type.typeArguments && dtsFn.type.typeArguments[0] || null; if (!typeParam || isAnyKeyword(typeParam)) { - // Either we do not have a parameterized type or the type is `any`. - let ngModule = fn.ngModule; - // For internal (non-library) module references, redirect the module's value declaration - // to its type declaration. - if (ngModule.viaModule === null) { - const dtsNgModule = this.host.getDtsDeclaration(ngModule.node); - if (!dtsNgModule) { - throw new Error( - `No typings declaration can be found for the referenced NgModule class in ${fn.declaration.getText()}.`); - } - if (!ts.isClassDeclaration(dtsNgModule) || !hasNameIdentifier(dtsNgModule)) { - throw new Error( - `The referenced NgModule in ${fn.declaration.getText()} is not a named class declaration in the typings program; instead we get ${dtsNgModule.getText()}`); - } - // Record the usage of the internal module as it needs to become an exported symbol - this.referencesRegistry.add(ngModule.node, new Reference(ngModule.node)); - - ngModule = {node: dtsNgModule, viaModule: null}; - } + const ngModule = this.resolveNgModuleReference(fn); const dtsFile = dtsFn.getSourceFile(); const analysis = analyses.has(dtsFile) ? analyses.get(dtsFile) : []; analysis.push({declaration: dtsFn, ngModule}); @@ -100,6 +87,30 @@ export class ModuleWithProvidersAnalyzer { } return dtsFn; } + + private resolveNgModuleReference(fn: ModuleWithProvidersFunction): + ConcreteDeclaration { + const ngModule = fn.ngModule; + + // For external module references, use the declaration as is. + if (ngModule.viaModule !== null) { + return ngModule; + } + + // For internal (non-library) module references, redirect the module's value declaration + // to its type declaration. + const dtsNgModule = this.host.getDtsDeclaration(ngModule.node); + if (!dtsNgModule) { + throw new Error( + `No typings declaration can be found for the referenced NgModule class in ${fn.declaration.getText()}.`); + } + if (!ts.isClassDeclaration(dtsNgModule) || !hasNameIdentifier(dtsNgModule)) { + throw new Error( + `The referenced NgModule in ${fn.declaration.getText()} is not a named class declaration in the typings program; instead we get ${dtsNgModule.getText()}`); + } + + return {node: dtsNgModule, viaModule: null}; + } } diff --git a/packages/compiler-cli/ngcc/test/analysis/module_with_providers_analyzer_spec.ts b/packages/compiler-cli/ngcc/test/analysis/module_with_providers_analyzer_spec.ts index 9deaccfba8..fe51e4ecc3 100644 --- a/packages/compiler-cli/ngcc/test/analysis/module_with_providers_analyzer_spec.ts +++ b/packages/compiler-cli/ngcc/test/analysis/module_with_providers_analyzer_spec.ts @@ -24,7 +24,7 @@ runInEachFileSystem(() => { let _: typeof absoluteFrom; let analyses: ModuleWithProvidersAnalyses; let program: ts.Program; - let dtsProgram: BundleProgram|null; + let dtsProgram: BundleProgram; let referencesRegistry: NgccReferencesRegistry; beforeEach(() => { @@ -333,7 +333,7 @@ runInEachFileSystem(() => { 'test-package', 'esm2015', false, getRootFiles(TEST_PROGRAM), getRootFiles(TEST_DTS_PROGRAM)); program = bundle.src.program; - dtsProgram = bundle.dts; + dtsProgram = bundle.dts !; const host = new Esm2015ReflectionHost( new MockLogger(), false, program.getTypeChecker(), dtsProgram); referencesRegistry = new NgccReferencesRegistry(host); @@ -412,7 +412,7 @@ runInEachFileSystem(() => { function getAnalysisDescription( analyses: ModuleWithProvidersAnalyses, fileName: AbsoluteFsPath) { - const file = getSourceFileOrError(dtsProgram !.program, fileName); + const file = getSourceFileOrError(dtsProgram.program, fileName); const analysis = analyses.get(file); return analysis ? analysis.map( @@ -424,4 +424,144 @@ runInEachFileSystem(() => { } }); }); + + describe('tracking references when generic types already present', () => { + let _: typeof absoluteFrom; + let analyses: ModuleWithProvidersAnalyses; + let program: ts.Program; + let dtsProgram: BundleProgram; + let referencesRegistry: NgccReferencesRegistry; + + beforeEach(() => { + _ = absoluteFrom; + + const TEST_PROGRAM: TestFile[] = [ + { + name: _('/node_modules/test-package/src/entry-point.js'), + contents: ` + export * from './explicit'; + export * from './module'; + ` + }, + { + name: _('/node_modules/test-package/src/explicit.js'), + contents: ` + import {ExternalModule} from './module'; + import {LibraryModule} from 'some-library'; + export class ExplicitInternalModule {} + export class ExplicitClass { + static explicitInternalMethod() { + return { + ngModule: ExplicitInternalModule, + providers: [] + }; + } + static explicitExternalMethod() { + return { + ngModule: ExternalModule, + providers: [] + }; + } + static explicitLibraryMethod() { + return { + ngModule: LibraryModule, + providers: [] + }; + } + } + ` + }, + { + name: _('/node_modules/test-package/src/module.js'), + contents: ` + export class ExternalModule {} + ` + }, + { + name: _('/node_modules/some-library/index.d.ts'), + contents: 'export declare class LibraryModule {}' + }, + ]; + const TEST_DTS_PROGRAM: TestFile[] = [ + { + name: _('/node_modules/test-package/typings/entry-point.d.ts'), + contents: ` + export * from './explicit'; + export * from './module'; + ` + }, + { + name: _('/node_modules/test-package/typings/explicit.d.ts'), + contents: ` + import {ModuleWithProviders} from './core'; + import {ExternalModule} from './module'; + import {LibraryModule} from 'some-library'; + export declare class ExplicitInternalModule {} + export declare class ExplicitClass { + static explicitInternalMethod(): ModuleWithProviders; + static explicitExternalMethod(): ModuleWithProviders; + static explicitLibraryMethod(): ModuleWithProviders; + } + ` + }, + { + name: _('/node_modules/test-package/typings/module.d.ts'), + contents: ` + export declare class ExternalModule {} + ` + }, + { + name: _('/node_modules/test-package/typings/core.d.ts'), + contents: ` + + export declare interface Type { + new (...args: any[]): T + } + export declare type Provider = any; + export declare interface ModuleWithProviders { + ngModule: Type + providers?: Provider[] + } + ` + }, + { + name: _('/node_modules/some-library/index.d.ts'), + contents: 'export declare class LibraryModule {}' + }, + ]; + loadTestFiles(TEST_PROGRAM); + loadTestFiles(TEST_DTS_PROGRAM); + const bundle = makeTestEntryPointBundle( + 'test-package', 'esm2015', false, getRootFiles(TEST_PROGRAM), + getRootFiles(TEST_DTS_PROGRAM)); + program = bundle.src.program; + dtsProgram = bundle.dts !; + const host = + new Esm2015ReflectionHost(new MockLogger(), false, program.getTypeChecker(), dtsProgram); + referencesRegistry = new NgccReferencesRegistry(host); + + const analyzer = new ModuleWithProvidersAnalyzer(host, referencesRegistry); + analyses = analyzer.analyzeProgram(program); + }); + + it('should track references even when nothing needs to be updated', () => { + const file = getSourceFileOrError( + dtsProgram.program, _('/node_modules/test-package/typings/explicit.d.ts')); + expect(analyses.has(file)).toBe(false); + + const declarations = referencesRegistry.getDeclarationMap(); + const explicitInternalModuleDeclaration = getDeclaration( + program, absoluteFrom('/node_modules/test-package/src/explicit.js'), + 'ExplicitInternalModule', ts.isClassDeclaration); + const externalModuleDeclaration = getDeclaration( + program, absoluteFrom('/node_modules/test-package/src/module.js'), 'ExternalModule', + ts.isClassDeclaration); + const libraryModuleDeclaration = getDeclaration( + program, absoluteFrom('/node_modules/some-library/index.d.ts'), 'LibraryModule', + ts.isClassDeclaration); + expect(declarations.has(explicitInternalModuleDeclaration.name !)).toBe(true); + expect(declarations.has(externalModuleDeclaration.name !)).toBe(true); + expect(declarations.has(libraryModuleDeclaration.name !)).toBe(false); + }); + }); });