From b72c7a89a915503c67c04b777b55c3008bc68602 Mon Sep 17 00:00:00 2001 From: JoostK Date: Tue, 3 Dec 2019 21:25:27 +0100 Subject: [PATCH] refactor(ivy): include generic type for `ModuleWithProviders` in .d.ts files (#34235) The `ModuleWithProviders` type has an optional type parameter that should be specified to indicate what NgModule class will be provided. This enables the Ivy compiler to statically determine the NgModule type from the declaration files. This type parameter will become required in the future, however to aid in the migration the compiler will detect code patterns where using `ModuleWithProviders` as return type is appropriate, in which case it transforms the emitted .d.ts files to include the generic type argument. This should reduce the number of occurrences where `ModuleWithProviders` is referenced without its generic type argument. Resolves FW-389 PR Close #34235 --- packages/compiler-cli/BUILD.bazel | 1 + .../src/ngtsc/modulewithproviders/BUILD.bazel | 17 + .../src/ngtsc/modulewithproviders/index.ts | 9 + .../ngtsc/modulewithproviders/src/scanner.ts | 164 ++++++++++ packages/compiler-cli/src/ngtsc/program.ts | 9 +- .../src/ngtsc/transform/BUILD.bazel | 1 + .../compiler-cli/src/ngtsc/transform/index.ts | 2 +- .../src/ngtsc/transform/src/compilation.ts | 14 +- .../src/ngtsc/transform/src/declaration.ts | 83 ++++- .../test/ngtsc/modulewithproviders_spec.ts | 298 ++++++++++++++++++ .../compiler/src/render3/r3_identifiers.ts | 5 + .../core/test/render3/jit_environment_spec.ts | 1 + 12 files changed, 592 insertions(+), 12 deletions(-) create mode 100644 packages/compiler-cli/src/ngtsc/modulewithproviders/BUILD.bazel create mode 100644 packages/compiler-cli/src/ngtsc/modulewithproviders/index.ts create mode 100644 packages/compiler-cli/src/ngtsc/modulewithproviders/src/scanner.ts create mode 100644 packages/compiler-cli/test/ngtsc/modulewithproviders_spec.ts diff --git a/packages/compiler-cli/BUILD.bazel b/packages/compiler-cli/BUILD.bazel index 6f8aade3bf..16b88b6f9c 100644 --- a/packages/compiler-cli/BUILD.bazel +++ b/packages/compiler-cli/BUILD.bazel @@ -32,6 +32,7 @@ ts_library( "//packages/compiler-cli/src/ngtsc/incremental", "//packages/compiler-cli/src/ngtsc/indexer", "//packages/compiler-cli/src/ngtsc/metadata", + "//packages/compiler-cli/src/ngtsc/modulewithproviders", "//packages/compiler-cli/src/ngtsc/partial_evaluator", "//packages/compiler-cli/src/ngtsc/perf", "//packages/compiler-cli/src/ngtsc/reflection", diff --git a/packages/compiler-cli/src/ngtsc/modulewithproviders/BUILD.bazel b/packages/compiler-cli/src/ngtsc/modulewithproviders/BUILD.bazel new file mode 100644 index 0000000000..2dfe50b6ba --- /dev/null +++ b/packages/compiler-cli/src/ngtsc/modulewithproviders/BUILD.bazel @@ -0,0 +1,17 @@ +load("//tools:defaults.bzl", "ts_library") + +package(default_visibility = ["//visibility:public"]) + +ts_library( + name = "modulewithproviders", + srcs = ["index.ts"] + glob([ + "src/**/*.ts", + ]), + deps = [ + "//packages/compiler", + "//packages/compiler-cli/src/ngtsc/imports", + "//packages/compiler-cli/src/ngtsc/partial_evaluator", + "//packages/compiler-cli/src/ngtsc/reflection", + "@npm//typescript", + ], +) diff --git a/packages/compiler-cli/src/ngtsc/modulewithproviders/index.ts b/packages/compiler-cli/src/ngtsc/modulewithproviders/index.ts new file mode 100644 index 0000000000..d5fe5d16d4 --- /dev/null +++ b/packages/compiler-cli/src/ngtsc/modulewithproviders/index.ts @@ -0,0 +1,9 @@ +/** + * @license + * Copyright Google Inc. All Rights Reserved. + * + * Use of this source code is governed by an MIT-style license that can be + * found in the LICENSE file at https://angular.io/license + */ + +export * from './src/scanner'; diff --git a/packages/compiler-cli/src/ngtsc/modulewithproviders/src/scanner.ts b/packages/compiler-cli/src/ngtsc/modulewithproviders/src/scanner.ts new file mode 100644 index 0000000000..bb85d24aa4 --- /dev/null +++ b/packages/compiler-cli/src/ngtsc/modulewithproviders/src/scanner.ts @@ -0,0 +1,164 @@ +/** + * @license + * Copyright Google Inc. All Rights Reserved. + * + * Use of this source code is governed by an MIT-style license that can be + * found in the LICENSE file at https://angular.io/license + */ + +import {ExpressionType, ExternalExpr, R3Identifiers as Identifiers, Type} from '@angular/compiler'; +import * as ts from 'typescript'; + +import {ImportMode, Reference, ReferenceEmitter} from '../../imports'; +import {PartialEvaluator, ResolvedValueMap} from '../../partial_evaluator'; +import {ReflectionHost} from '../../reflection'; + +export interface DtsHandler { addTypeReplacement(node: ts.Declaration, type: Type): void; } + +export class ModuleWithProvidersScanner { + constructor( + private host: ReflectionHost, private evaluator: PartialEvaluator, + private emitter: ReferenceEmitter) {} + + scan(sf: ts.SourceFile, dts: DtsHandler): void { + for (const stmt of sf.statements) { + this.visitStatement(dts, stmt); + } + } + + private visitStatement(dts: DtsHandler, stmt: ts.Statement): void { + // Detect whether a statement is exported, which is used as one of the hints whether to look + // more closely at possible MWP functions within. This is a syntactic check, not a semantic + // check, so it won't detect cases like: + // + // var X = ...; + // export {X} + // + // This is intentional, because the alternative is slow and this will catch 99% of the cases we + // need to handle. + const isExported = stmt.modifiers !== undefined && + stmt.modifiers.some(mod => mod.kind === ts.SyntaxKind.ExportKeyword); + + if (!isExported) { + return; + } + + if (ts.isClassDeclaration(stmt)) { + for (const member of stmt.members) { + if (!ts.isMethodDeclaration(member) || !isStatic(member)) { + continue; + } + + this.visitFunctionOrMethodDeclaration(dts, member); + } + } else if (ts.isFunctionDeclaration(stmt)) { + this.visitFunctionOrMethodDeclaration(dts, stmt); + } + } + + private visitFunctionOrMethodDeclaration( + dts: DtsHandler, decl: ts.MethodDeclaration|ts.FunctionDeclaration): void { + // First, some sanity. This should have a method body with a single return statement. + if (decl.body === undefined || decl.body.statements.length !== 1) { + return; + } + const retStmt = decl.body.statements[0]; + if (!ts.isReturnStatement(retStmt) || retStmt.expression === undefined) { + return; + } + const retValue = retStmt.expression; + + // Now, look at the return type of the method. Maybe bail if the type is already marked, or if + // it's incompatible with a MWP function. + const returnType = this.returnTypeOf(decl); + if (returnType === ReturnType.OTHER || returnType === ReturnType.MWP_WITH_TYPE) { + // Don't process this declaration, it either already declares the right return type, or an + // incompatible one. + return; + } + + const value = this.evaluator.evaluate(retValue); + if (!(value instanceof Map) || !value.has('ngModule')) { + // The return value does not provide sufficient information to be able to add a generic type. + return; + } + + if (returnType === ReturnType.INFERRED && !isModuleWithProvidersType(value)) { + // The return type is inferred but the returned object is not of the correct shape, so we + // shouldn's modify the return type to become `ModuleWithProviders`. + return; + } + + // The return type has been verified to represent the `ModuleWithProviders` type, but either the + // return type is inferred or the generic type argument is missing. In both cases, a new return + // type is created where the `ngModule` type is included as generic type argument. + const ngModule = value.get('ngModule'); + if (!(ngModule instanceof Reference) || !ts.isClassDeclaration(ngModule.node)) { + return; + } + + const ngModuleExpr = + this.emitter.emit(ngModule, decl.getSourceFile(), ImportMode.ForceNewImport); + const ngModuleType = new ExpressionType(ngModuleExpr); + const mwpNgType = new ExpressionType( + new ExternalExpr(Identifiers.ModuleWithProviders), /* modifiers */ null, [ngModuleType]); + + dts.addTypeReplacement(decl, mwpNgType); + } + + private returnTypeOf(decl: ts.FunctionDeclaration|ts.MethodDeclaration| + ts.VariableDeclaration): ReturnType { + if (decl.type === undefined) { + return ReturnType.INFERRED; + } else if (!ts.isTypeReferenceNode(decl.type)) { + return ReturnType.OTHER; + } + + // Try to figure out if the type is of a familiar form, something that looks like it was + // imported. + let typeId: ts.Identifier; + if (ts.isIdentifier(decl.type.typeName)) { + // def: ModuleWithProviders + typeId = decl.type.typeName; + } else if (ts.isQualifiedName(decl.type.typeName) && ts.isIdentifier(decl.type.typeName.left)) { + // def: i0.ModuleWithProviders + typeId = decl.type.typeName.right; + } else { + return ReturnType.OTHER; + } + + const importDecl = this.host.getImportOfIdentifier(typeId); + if (importDecl === null || importDecl.from !== '@angular/core' || + importDecl.name !== 'ModuleWithProviders') { + return ReturnType.OTHER; + } + + if (decl.type.typeArguments === undefined || decl.type.typeArguments.length === 0) { + // The return type is indeed ModuleWithProviders, but no generic type parameter was found. + return ReturnType.MWP_NO_TYPE; + } else { + // The return type is ModuleWithProviders, and the user has already specified a generic type. + return ReturnType.MWP_WITH_TYPE; + } + } +} + +enum ReturnType { + INFERRED, + MWP_NO_TYPE, + MWP_WITH_TYPE, + OTHER, +} + +/** Whether the resolved value map represents a ModuleWithProviders object */ +function isModuleWithProvidersType(value: ResolvedValueMap): boolean { + const ngModule = value.has('ngModule'); + const providers = value.has('providers'); + + return ngModule && (value.size === 1 || (providers && value.size === 2)); +} + +function isStatic(node: ts.Node): boolean { + return node.modifiers !== undefined && + node.modifiers.some(mod => mod.kind === ts.SyntaxKind.StaticKeyword); +} diff --git a/packages/compiler-cli/src/ngtsc/program.ts b/packages/compiler-cli/src/ngtsc/program.ts index 00b4211e99..de99bc56c0 100644 --- a/packages/compiler-cli/src/ngtsc/program.ts +++ b/packages/compiler-cli/src/ngtsc/program.ts @@ -23,6 +23,7 @@ import {IncrementalDriver} from './incremental'; import {IndexedComponent, IndexingContext} from './indexer'; import {generateAnalysis} from './indexer/src/transform'; import {CompoundMetadataReader, CompoundMetadataRegistry, DtsMetadataReader, LocalMetadataRegistry, MetadataReader} from './metadata'; +import {ModuleWithProvidersScanner} from './modulewithproviders'; import {PartialEvaluator} from './partial_evaluator'; import {NOOP_PERF_RECORDER, PerfRecorder, PerfTracker} from './perf'; import {TypeScriptReflectionHost} from './reflection'; @@ -31,8 +32,7 @@ import {NgModuleRouteAnalyzer, entryPointKeyFor} from './routing'; import {ComponentScopeReader, CompoundComponentScopeReader, LocalModuleScopeRegistry, MetadataDtsModuleScopeResolver} from './scope'; import {FactoryGenerator, FactoryInfo, GeneratedShimsHostWrapper, ShimGenerator, SummaryGenerator, TypeCheckShimGenerator, generatedFactoryTransform} from './shims'; import {ivySwitchTransform} from './switch'; -import {IvyCompilation, declarationTransformFactory, ivyTransformFactory} from './transform'; -import {DtsTransformRegistry} from './transform'; +import {DtsTransformRegistry, IvyCompilation, declarationTransformFactory, ivyTransformFactory} from './transform'; import {aliasTransformFactory} from './transform/src/alias'; import {TypeCheckContext, TypeCheckingConfig, typeCheckFilePath} from './typecheck'; import {normalizeSeparators} from './util/src/path'; @@ -71,6 +71,7 @@ export class NgtscProgram implements api.Program { private typeCheckFilePath: AbsoluteFsPath; private modifiedResourceFiles: Set|null; private dtsTransforms: DtsTransformRegistry|null = null; + private mwpScanner: ModuleWithProvidersScanner|null = null; constructor( rootNames: ReadonlyArray, private options: api.CompilerOptions, @@ -623,6 +624,8 @@ export class NgtscProgram implements api.Program { this.dtsTransforms = new DtsTransformRegistry(); + this.mwpScanner = new ModuleWithProvidersScanner(this.reflector, evaluator, this.refEmitter); + // Set up the IvyCompilation, which manages state for the Ivy transformer. const handlers = [ new ComponentDecoratorHandler( @@ -651,7 +654,7 @@ export class NgtscProgram implements api.Program { return new IvyCompilation( handlers, this.reflector, this.importRewriter, this.incrementalDriver, this.perfRecorder, this.sourceToFactorySymbols, scopeRegistry, - this.options.compileNonExportedClasses !== false, this.dtsTransforms); + this.options.compileNonExportedClasses !== false, this.dtsTransforms, this.mwpScanner); } private get reflector(): TypeScriptReflectionHost { diff --git a/packages/compiler-cli/src/ngtsc/transform/BUILD.bazel b/packages/compiler-cli/src/ngtsc/transform/BUILD.bazel index 4a280a658a..4a528c2b6f 100644 --- a/packages/compiler-cli/src/ngtsc/transform/BUILD.bazel +++ b/packages/compiler-cli/src/ngtsc/transform/BUILD.bazel @@ -13,6 +13,7 @@ ts_library( "//packages/compiler-cli/src/ngtsc/imports", "//packages/compiler-cli/src/ngtsc/incremental", "//packages/compiler-cli/src/ngtsc/indexer", + "//packages/compiler-cli/src/ngtsc/modulewithproviders", "//packages/compiler-cli/src/ngtsc/perf", "//packages/compiler-cli/src/ngtsc/reflection", "//packages/compiler-cli/src/ngtsc/scope", diff --git a/packages/compiler-cli/src/ngtsc/transform/index.ts b/packages/compiler-cli/src/ngtsc/transform/index.ts index 7cbceebaf0..d02cef8ea9 100644 --- a/packages/compiler-cli/src/ngtsc/transform/index.ts +++ b/packages/compiler-cli/src/ngtsc/transform/index.ts @@ -8,5 +8,5 @@ export * from './src/api'; export {IvyCompilation} from './src/compilation'; -export {declarationTransformFactory, DtsTransformRegistry, IvyDeclarationDtsTransform} from './src/declaration'; +export {declarationTransformFactory, DtsTransformRegistry, IvyDeclarationDtsTransform, ReturnTypeTransform} from './src/declaration'; export {ivyTransformFactory} from './src/transform'; diff --git a/packages/compiler-cli/src/ngtsc/transform/src/compilation.ts b/packages/compiler-cli/src/ngtsc/transform/src/compilation.ts index 52f26cf6d1..fc243d50eb 100644 --- a/packages/compiler-cli/src/ngtsc/transform/src/compilation.ts +++ b/packages/compiler-cli/src/ngtsc/transform/src/compilation.ts @@ -6,13 +6,14 @@ * found in the LICENSE file at https://angular.io/license */ -import {ConstantPool} from '@angular/compiler'; +import {ConstantPool, Type} from '@angular/compiler'; import * as ts from 'typescript'; import {ErrorCode, FatalDiagnosticError} from '../../diagnostics'; import {ImportRewriter} from '../../imports'; import {IncrementalDriver} from '../../incremental'; import {IndexingContext} from '../../indexer'; +import {ModuleWithProvidersScanner} from '../../modulewithproviders'; import {PerfRecorder} from '../../perf'; import {ClassDeclaration, ReflectionHost, isNamedClassDeclaration} from '../../reflection'; import {LocalModuleScopeRegistry} from '../../scope'; @@ -71,7 +72,8 @@ export class IvyCompilation { private importRewriter: ImportRewriter, private incrementalDriver: IncrementalDriver, private perf: PerfRecorder, private sourceToFactorySymbols: Map>|null, private scopeRegistry: LocalModuleScopeRegistry, private compileNonExportedClasses: boolean, - private dtsTransforms: DtsTransformRegistry) {} + private dtsTransforms: DtsTransformRegistry, private mwpScanner: ModuleWithProvidersScanner) { + } get exportStatements(): Map> { return this.reexportMap; } @@ -235,6 +237,14 @@ export class IvyCompilation { visit(sf); + this.mwpScanner.scan(sf, { + addTypeReplacement: (node: ts.Declaration, type: Type): void => { + // Only obtain the return type transform for the source file once there's a type to replace, + // so that no transform is allocated when there's nothing to do. + this.dtsTransforms.getReturnTypeTransform(sf).addTypeReplacement(node, type); + } + }); + if (preanalyze && promises.length > 0) { return Promise.all(promises).then(() => undefined); } else { diff --git a/packages/compiler-cli/src/ngtsc/transform/src/declaration.ts b/packages/compiler-cli/src/ngtsc/transform/src/declaration.ts index d33f094b6d..4547430c18 100644 --- a/packages/compiler-cli/src/ngtsc/transform/src/declaration.ts +++ b/packages/compiler-cli/src/ngtsc/transform/src/declaration.ts @@ -21,13 +21,21 @@ import {addImports} from './utils'; * have their declaration file transformed. */ export class DtsTransformRegistry { - private ivyDeclarationTransforms = new Map(); + private ivyDeclarationTransforms = new Map(); + private returnTypeTransforms = new Map(); getIvyDeclarationTransform(sf: ts.SourceFile): IvyDeclarationDtsTransform { - if (!this.ivyDeclarationTransforms.has(sf.fileName)) { - this.ivyDeclarationTransforms.set(sf.fileName, new IvyDeclarationDtsTransform()); + if (!this.ivyDeclarationTransforms.has(sf)) { + this.ivyDeclarationTransforms.set(sf, new IvyDeclarationDtsTransform()); } - return this.ivyDeclarationTransforms.get(sf.fileName) !; + return this.ivyDeclarationTransforms.get(sf) !; + } + + getReturnTypeTransform(sf: ts.SourceFile): ReturnTypeTransform { + if (!this.returnTypeTransforms.has(sf)) { + this.returnTypeTransforms.set(sf, new ReturnTypeTransform()); + } + return this.returnTypeTransforms.get(sf) !; } /** @@ -42,11 +50,16 @@ export class DtsTransformRegistry { if (!sf.isDeclarationFile) { return null; } + const originalSf = ts.getOriginalNode(sf) as ts.SourceFile; let transforms: DtsTransform[]|null = null; - if (this.ivyDeclarationTransforms.has(sf.fileName)) { + if (this.ivyDeclarationTransforms.has(originalSf)) { transforms = []; - transforms.push(this.ivyDeclarationTransforms.get(sf.fileName) !); + transforms.push(this.ivyDeclarationTransforms.get(originalSf) !); + } + if (this.returnTypeTransforms.has(originalSf)) { + transforms = transforms || []; + transforms.push(this.returnTypeTransforms.get(originalSf) !); } return transforms; } @@ -211,3 +224,61 @@ export class IvyDeclarationDtsTransform implements DtsTransform { /* members */[...members, ...newMembers]); } } + +export class ReturnTypeTransform implements DtsTransform { + private typeReplacements = new Map(); + + addTypeReplacement(declaration: ts.Declaration, type: Type): void { + this.typeReplacements.set(declaration, type); + } + + transformClassElement(element: ts.ClassElement, imports: ImportManager): ts.ClassElement { + if (!ts.isMethodSignature(element)) { + return element; + } + + const original = ts.getOriginalNode(element) as ts.MethodDeclaration; + if (!this.typeReplacements.has(original)) { + return element; + } + const returnType = this.typeReplacements.get(original) !; + const tsReturnType = translateType(returnType, imports); + + const methodSignature = ts.updateMethodSignature( + /* node */ element, + /* typeParameters */ element.typeParameters, + /* parameters */ element.parameters, + /* type */ tsReturnType, + /* name */ element.name, + /* questionToken */ element.questionToken); + + // Copy over any modifiers, these cannot be set during the `ts.updateMethodSignature` call. + methodSignature.modifiers = element.modifiers; + + // A bug in the TypeScript declaration causes `ts.MethodSignature` not to be assignable to + // `ts.ClassElement`. Since `element` was a `ts.MethodSignature` already, transforming it into + // this type is actually correct. + return methodSignature as unknown as ts.ClassElement; + } + + transformFunctionDeclaration(element: ts.FunctionDeclaration, imports: ImportManager): + ts.FunctionDeclaration { + const original = ts.getOriginalNode(element) as ts.FunctionDeclaration; + if (!this.typeReplacements.has(original)) { + return element; + } + const returnType = this.typeReplacements.get(original) !; + const tsReturnType = translateType(returnType, imports); + + return ts.updateFunctionDeclaration( + /* node */ element, + /* decorators */ element.decorators, + /* modifiers */ element.modifiers, + /* asteriskToken */ element.asteriskToken, + /* name */ element.name, + /* typeParameters */ element.typeParameters, + /* parameters */ element.parameters, + /* type */ tsReturnType, + /* body */ element.body); + } +} diff --git a/packages/compiler-cli/test/ngtsc/modulewithproviders_spec.ts b/packages/compiler-cli/test/ngtsc/modulewithproviders_spec.ts new file mode 100644 index 0000000000..3035661e28 --- /dev/null +++ b/packages/compiler-cli/test/ngtsc/modulewithproviders_spec.ts @@ -0,0 +1,298 @@ +/** + * @license + * Copyright Google Inc. All Rights Reserved. + * + * Use of this source code is governed by an MIT-style license that can be + * found in the LICENSE file at https://angular.io/license + */ + +import {runInEachFileSystem} from '../../src/ngtsc/file_system/testing'; +import {loadStandardTestFiles} from '../helpers/src/mock_file_loading'; + +import {NgtscTestEnvironment} from './env'; + +const trim = (input: string): string => input.replace(/\s+/g, ' ').trim(); + +const testFiles = loadStandardTestFiles(); + +runInEachFileSystem(() => { + describe('ModuleWithProviders generic type transform', () => { + let env !: NgtscTestEnvironment; + + beforeEach(() => { + env = NgtscTestEnvironment.setup(testFiles); + env.tsconfig(); + }); + + it('should add a generic type for static methods on exported classes', () => { + env.write('test.ts', ` + import {NgModule} from '@angular/core'; + + @NgModule() + export class TestModule { + static forRoot() { + return { + ngModule: TestModule, + }; + } + } + `); + + env.driveMain(); + + const dtsContents = trim(env.getContents('test.d.ts')); + expect(dtsContents).toContain('import * as i0 from "@angular/core";'); + expect(dtsContents).toContain('static forRoot(): i0.ModuleWithProviders;'); + }); + + it('should not add a generic type for non-static methods', () => { + env.write('test.ts', ` + import {NgModule} from '@angular/core'; + + @NgModule() + export class TestModule { + forRoot() { + return { + ngModule: TestModule, + }; + } + } + `); + + env.driveMain(); + + const dtsContents = trim(env.getContents('test.d.ts')); + expect(dtsContents).toContain('import * as i0 from "@angular/core";'); + expect(dtsContents).toContain('forRoot(): { ngModule: typeof TestModule; };'); + expect(dtsContents).not.toContain('static forRoot()'); + }); + + it('should add a generic type for exported functions', () => { + env.write('test.ts', ` + import {NgModule} from '@angular/core'; + + export function forRoot() { + return { + ngModule: TestModule, + }; + } + + @NgModule() + export class TestModule {} + `); + + env.driveMain(); + + const dtsContents = trim(env.getContents('test.d.ts')); + expect(dtsContents).toContain('import * as i0 from "@angular/core";'); + expect(dtsContents) + .toContain('export declare function forRoot(): i0.ModuleWithProviders;'); + }); + + it('should not add a generic type when already present', () => { + env.write('test.ts', ` + import {NgModule, ModuleWithProviders} from '@angular/core'; + + export class TestModule { + forRoot(): ModuleWithProviders { + return { + ngModule: TestModule, + }; + } + } + + @NgModule() + export class InternalTestModule {} + `); + + env.driveMain(); + + const dtsContents = trim(env.getContents('test.d.ts')); + expect(dtsContents).toContain('forRoot(): ModuleWithProviders;'); + }); + + it('should add a generic type when missing the generic type parameter', () => { + env.write('test.ts', ` + import {NgModule, ModuleWithProviders} from '@angular/core'; + + @NgModule() + export class TestModule { + static forRoot(): ModuleWithProviders { + return { + ngModule: TestModule, + }; + } + } + `); + + env.driveMain(); + + const dtsContents = trim(env.getContents('test.d.ts')); + expect(dtsContents).toContain('static forRoot(): i0.ModuleWithProviders;'); + }); + + it('should add a generic type when missing the generic type parameter (qualified name)', () => { + env.write('test.ts', ` + import * as ng from '@angular/core'; + + @ng.NgModule() + export class TestModule { + static forRoot(): ng.ModuleWithProviders { + return { + ngModule: TestModule, + }; + } + } + `); + + env.driveMain(); + + const dtsContents = trim(env.getContents('test.d.ts')); + expect(dtsContents).toContain('static forRoot(): i0.ModuleWithProviders;'); + }); + + it('should add a generic type and add an import for external references', () => { + env.write('test.ts', ` + import {ModuleWithProviders} from '@angular/core'; + import {InternalTestModule} from './internal'; + + export class TestModule { + static forRoot(): ModuleWithProviders { + return { + ngModule: InternalTestModule, + }; + } + } + `); + env.write('internal.ts', ` + import {NgModule} from '@angular/core'; + + @NgModule() + export class InternalTestModule {} + `); + + env.driveMain(); + + const dtsContents = trim(env.getContents('test.d.ts')); + expect(dtsContents).toContain('import * as i1 from "./internal";'); + expect(dtsContents) + .toContain('static forRoot(): i0.ModuleWithProviders;'); + }); + + it('should not add a generic type if the return type is not ModuleWithProviders', () => { + env.write('test.ts', ` + import {NgModule} from '@angular/core'; + + @NgModule() + export class TestModule { + static forRoot(): { ngModule: typeof TestModule } { + return { + ngModule: TestModule, + }; + } + } + `); + + env.driveMain(); + + const dtsContents = trim(env.getContents('test.d.ts')); + expect(dtsContents).toContain('static forRoot(): { ngModule: typeof TestModule; };'); + }); + + it('should not add a generic type if the return type is not ModuleWithProviders from @angular/core', + () => { + env.write('test.ts', ` + import {NgModule} from '@angular/core'; + import {ModuleWithProviders} from './mwp'; + + @NgModule() + export class TestModule { + static forRoot(): ModuleWithProviders { + return { + ngModule: TestModule, + }; + } + } + `); + env.write('mwp.ts', ` + export type ModuleWithProviders = { ngModule: any }; + `); + + env.driveMain(); + + const dtsContents = trim(env.getContents('test.d.ts')); + expect(dtsContents).toContain('static forRoot(): ModuleWithProviders;'); + }); + + it('should not add a generic type when the "ngModule" property is not a reference', () => { + env.write('test.ts', ` + import {NgModule} from '@angular/core'; + + @NgModule() + export class TestModule { + static forRoot() { + return { + ngModule: 'test', + }; + } + } + `); + + env.driveMain(); + + const dtsContents = trim(env.getContents('test.d.ts')); + expect(dtsContents).toContain('static forRoot(): { ngModule: string; };'); + }); + + it('should not add a generic type when the class is not exported', () => { + env.write('test.ts', ` + import {NgModule} from '@angular/core'; + + @NgModule() + class TestModule { + static forRoot() { + return { + ngModule: TestModule, + }; + } + } + `); + + env.driveMain(); + + // The TestModule class is not exported so doesn't even show up in the declaration file + const dtsContents = trim(env.getContents('test.d.ts')); + expect(dtsContents).not.toContain('static forRoot()'); + }); + + it('should add a generic type only when ngModule/providers are present', () => { + env.write('test.ts', ` + import {NgModule, ModuleWithProviders} from '@angular/core'; + + @NgModule() + export class TestModule { + static hasNgModuleAndProviders() { + return { + ngModule: TestModule, + providers: [], + }; + } + static hasNgModuleAndFoo() { + return { + ngModule: TestModule, + foo: 'test', + }; + } + } + `); + + env.driveMain(); + + const dtsContents = trim(env.getContents('test.d.ts')); + expect(dtsContents) + .toContain('static hasNgModuleAndProviders(): i0.ModuleWithProviders;'); + expect(dtsContents) + .toContain('static hasNgModuleAndFoo(): { ngModule: typeof TestModule; foo: string; };'); + }); + }); +}); diff --git a/packages/compiler/src/render3/r3_identifiers.ts b/packages/compiler/src/render3/r3_identifiers.ts index 66d0b0a5b9..dc43f681a2 100644 --- a/packages/compiler/src/render3/r3_identifiers.ts +++ b/packages/compiler/src/render3/r3_identifiers.ts @@ -260,6 +260,11 @@ export class Identifiers { moduleName: CORE, }; + static ModuleWithProviders: o.ExternalReference = { + name: 'ModuleWithProviders', + moduleName: CORE, + }; + static defineNgModule: o.ExternalReference = {name: 'ɵɵdefineNgModule', moduleName: CORE}; static setNgModuleScope: o.ExternalReference = {name: 'ɵɵsetNgModuleScope', moduleName: CORE}; diff --git a/packages/core/test/render3/jit_environment_spec.ts b/packages/core/test/render3/jit_environment_spec.ts index fbc1f984ab..c7e5b4640e 100644 --- a/packages/core/test/render3/jit_environment_spec.ts +++ b/packages/core/test/render3/jit_environment_spec.ts @@ -18,6 +18,7 @@ const INTERFACE_EXCEPTIONS = new Set([ 'ɵɵNgModuleDefWithMeta', 'ɵɵPipeDefWithMeta', 'ɵɵFactoryDef', + 'ModuleWithProviders', ]); describe('r3 jit environment', () => {