diff --git a/packages/core/src/render3/jit/module.ts b/packages/core/src/render3/jit/module.ts index fe41493869..e639c9b25b 100644 --- a/packages/core/src/render3/jit/module.ts +++ b/packages/core/src/render3/jit/module.ts @@ -94,8 +94,13 @@ export function compileNgModule(moduleType: Type, ngModule: NgModule = {}): /** * Compiles and adds the `ngModuleDef` and `ngInjectorDef` properties to the module class. + * + * It's possible to compile a module via this API which will allow duplicate declarations in its + * root. */ -export function compileNgModuleDefs(moduleType: NgModuleType, ngModule: NgModule): void { +export function compileNgModuleDefs( + moduleType: NgModuleType, ngModule: NgModule, + allowDuplicateDeclarationsInRoot: boolean = false): void { ngDevMode && assertDefined(moduleType, 'Required value moduleType'); ngDevMode && assertDefined(ngModule, 'Required value ngModule'); const declarations: Type[] = flatten(ngModule.declarations || EMPTY_ARRAY); @@ -129,7 +134,8 @@ export function compileNgModuleDefs(moduleType: NgModuleType, ngModule: NgModule Object.defineProperty(moduleType, NG_INJECTOR_DEF, { get: () => { if (ngInjectorDef === null) { - ngDevMode && verifySemanticsOfNgModuleDef(moduleType as any as NgModuleType); + ngDevMode && verifySemanticsOfNgModuleDef( + moduleType as any as NgModuleType, allowDuplicateDeclarationsInRoot); const meta: R3InjectorMetadataFacade = { name: moduleType.name, type: moduleType, @@ -150,7 +156,8 @@ export function compileNgModuleDefs(moduleType: NgModuleType, ngModule: NgModule }); } -function verifySemanticsOfNgModuleDef(moduleType: NgModuleType): void { +function verifySemanticsOfNgModuleDef( + moduleType: NgModuleType, allowDuplicateDeclarationsInRoot: boolean): void { if (verifiedNgModule.get(moduleType)) return; verifiedNgModule.set(moduleType, true); moduleType = resolveForwardRef(moduleType); @@ -158,7 +165,9 @@ function verifySemanticsOfNgModuleDef(moduleType: NgModuleType): void { const errors: string[] = []; const declarations = maybeUnwrapFn(ngModuleDef.declarations); const imports = maybeUnwrapFn(ngModuleDef.imports); - flatten(imports).map(unwrapModuleWithProvidersImports).forEach(verifySemanticsOfNgModuleDef); + flatten(imports) + .map(unwrapModuleWithProvidersImports) + .forEach(mod => verifySemanticsOfNgModuleDef(mod, false)); const exports = maybeUnwrapFn(ngModuleDef.exports); declarations.forEach(verifyDeclarationsHaveDefinitions); const combinedDeclarations: Type[] = [ @@ -166,7 +175,7 @@ function verifySemanticsOfNgModuleDef(moduleType: NgModuleType): void { ...flatten(imports.map(computeCombinedExports)).map(resolveForwardRef), ]; exports.forEach(verifyExportsAreDeclaredOrReExported); - declarations.forEach(verifyDeclarationIsUnique); + declarations.forEach(decl => verifyDeclarationIsUnique(decl, allowDuplicateDeclarationsInRoot)); declarations.forEach(verifyComponentEntryComponentsIsPartOfNgModule); const ngModule = getAnnotation(moduleType, 'NgModule'); @@ -174,7 +183,7 @@ function verifySemanticsOfNgModuleDef(moduleType: NgModuleType): void { ngModule.imports && flatten(ngModule.imports) .map(unwrapModuleWithProvidersImports) - .forEach(verifySemanticsOfNgModuleDef); + .forEach(mod => verifySemanticsOfNgModuleDef(mod, false)); ngModule.bootstrap && ngModule.bootstrap.forEach(verifyCorrectBootstrapType); ngModule.bootstrap && ngModule.bootstrap.forEach(verifyComponentIsPartOfNgModule); ngModule.entryComponents && ngModule.entryComponents.forEach(verifyComponentIsPartOfNgModule); @@ -209,15 +218,17 @@ function verifySemanticsOfNgModuleDef(moduleType: NgModuleType): void { } } - function verifyDeclarationIsUnique(type: Type) { + function verifyDeclarationIsUnique(type: Type, suppressErrors: boolean) { type = resolveForwardRef(type); const existingModule = ownerNgModule.get(type); if (existingModule && existingModule !== moduleType) { - const modules = [existingModule, moduleType].map(stringifyForError).sort(); - errors.push( - `Type ${stringifyForError(type)} is part of the declarations of 2 modules: ${modules[0]} and ${modules[1]}! ` + - `Please consider moving ${stringifyForError(type)} to a higher module that imports ${modules[0]} and ${modules[1]}. ` + - `You can also create a new NgModule that exports and includes ${stringifyForError(type)} then import that NgModule in ${modules[0]} and ${modules[1]}.`); + if (!suppressErrors) { + const modules = [existingModule, moduleType].map(stringifyForError).sort(); + errors.push( + `Type ${stringifyForError(type)} is part of the declarations of 2 modules: ${modules[0]} and ${modules[1]}! ` + + `Please consider moving ${stringifyForError(type)} to a higher module that imports ${modules[0]} and ${modules[1]}. ` + + `You can also create a new NgModule that exports and includes ${stringifyForError(type)} then import that NgModule in ${modules[0]} and ${modules[1]}.`); + } } else { // Mark type as having owner. ownerNgModule.set(type, moduleType); @@ -312,7 +323,7 @@ function computeCombinedExports(type: Type): Type[] { return [...flatten(maybeUnwrapFn(ngModuleDef.exports).map((type) => { const ngModuleDef = getNgModuleDef(type); if (ngModuleDef) { - verifySemanticsOfNgModuleDef(type as any as NgModuleType); + verifySemanticsOfNgModuleDef(type as any as NgModuleType, false); return computeCombinedExports(type); } else { return type; diff --git a/packages/core/test/test_bed_spec.ts b/packages/core/test/test_bed_spec.ts index 56dfc65ea0..61b809e2e3 100644 --- a/packages/core/test/test_bed_spec.ts +++ b/packages/core/test/test_bed_spec.ts @@ -6,7 +6,7 @@ * found in the LICENSE file at https://angular.io/license */ -import {Component, Directive, ErrorHandler, Inject, Injectable, InjectionToken, Input, NgModule, Optional, Pipe, ɵsetClassMetadata as setClassMetadata, ɵɵdefineComponent as defineComponent, ɵɵtext as text} from '@angular/core'; +import {Component, Directive, ErrorHandler, Inject, Injectable, InjectionToken, Input, NgModule, Optional, Pipe, ɵsetClassMetadata as setClassMetadata, ɵɵdefineComponent as defineComponent, ɵɵdefineNgModule as defineNgModule, ɵɵtext as text} from '@angular/core'; import {TestBed, getTestBed} from '@angular/core/testing/src/test_bed'; import {By} from '@angular/platform-browser'; import {expect} from '@angular/platform-browser/testing/src/matchers'; @@ -371,6 +371,62 @@ describe('TestBed', () => { }).toThrowError(); }); + onlyInIvy('TestBed new feature to allow declaration and import of component') + .it('should allow both the declaration and import of a component into the testing module', + () => { + // This test validates that a component (Outer) which is both declared and imported + // (via its module) in the testing module behaves correctly. That is: + // + // 1) the component should be compiled in the scope of its original module. + // + // This condition is tested by having the component (Outer) use another component + // (Inner) within its template. Thus, if it's compiled in the correct scope then the + // text 'Inner' from the template of (Inner) should appear in the result. + // + // 2) the component should be available in the TestingModule scope. + // + // This condition is tested by attempting to use the component (Outer) inside a test + // fixture component (Fixture) which is declared in the testing module only. + + @Component({ + selector: 'inner', + template: 'Inner', + }) + class Inner { + } + + @Component({ + selector: 'outer', + template: '', + }) + class Outer { + } + + @NgModule({ + declarations: [Inner, Outer], + }) + class Module { + } + + @Component({ + template: '', + selector: 'fixture', + }) + class Fixture { + } + + TestBed.configureTestingModule({ + declarations: [Outer, Fixture], + imports: [Module], + }); + + const fixture = TestBed.createComponent(Fixture); + // The Outer component should have its template stamped out, and that template should + // include a correct instance of the Inner component with the 'Inner' text from its + // template. + expect(fixture.nativeElement.innerHTML).toEqual('Inner'); + }); + onlyInIvy('TestBed should handle AOT pre-compiled Components') .describe('AOT pre-compiled components', () => { /** @@ -431,6 +487,42 @@ describe('TestBed', () => { const fixture = TestBed.createComponent(SomeComponent); expect(fixture.nativeElement.innerHTML).toBe(''); }); + + it('should allow component in both in declarations and imports', () => { + const SomeComponent = getAOTCompiledComponent(); + + // This is an AOT compiled module which declares (but does not export) SomeComponent. + class ModuleClass { + static ngModuleDef = defineNgModule({ + type: ModuleClass, + declarations: [SomeComponent], + }); + } + + @Component({ + template: '', + + selector: 'fixture', + }) + class TestFixture { + } + + TestBed.configureTestingModule({ + // Here, SomeComponent is both declared, and then the module which declares it is + // also imported. This used to be a duplicate declaration error, but is now interpreted + // to mean: + // 1) Compile (or reuse) SomeComponent in the context of its original NgModule + // 2) Make SomeComponent available in the scope of the testing module, even if it wasn't + // originally exported from its NgModule. + // + // This allows TestFixture to use SomeComponent, which is asserted below. + declarations: [SomeComponent, TestFixture], + imports: [ModuleClass], + }); + const fixture = TestBed.createComponent(TestFixture); + // The regex avoids any issues with styling attributes. + expect(fixture.nativeElement.innerHTML).toMatch(/]*>Some template<\/comp>/); + }); }); onlyInIvy('patched ng defs should be removed after resetting TestingModule') diff --git a/packages/core/testing/src/r3_test_bed_compiler.ts b/packages/core/testing/src/r3_test_bed_compiler.ts index e18e7f81fa..3c53f11ad5 100644 --- a/packages/core/testing/src/r3_test_bed_compiler.ts +++ b/packages/core/testing/src/r3_test_bed_compiler.ts @@ -15,8 +15,15 @@ import {MetadataOverride} from './metadata_override'; import {ComponentResolver, DirectiveResolver, NgModuleResolver, PipeResolver, Resolver} from './resolvers'; import {TestModuleMetadata} from './test_bed_common'; -const TESTING_MODULE = 'TestingModule'; -type TESTING_MODULE = typeof TESTING_MODULE; +enum TestingModuleOverride { + DECLARATION, + OVERRIDE_TEMPLATE, +} + +function isTestingModuleOverride(value: unknown): value is TestingModuleOverride { + return value === TestingModuleOverride.DECLARATION || + value === TestingModuleOverride.OVERRIDE_TEMPLATE; +} // Resolvers for Angular decorators type Resolvers = { @@ -56,7 +63,7 @@ export class R3TestBedCompiler { private resolvers: Resolvers = initResolvers(); - private componentToModuleScope = new Map, Type|TESTING_MODULE>(); + private componentToModuleScope = new Map, Type|TestingModuleOverride>(); // Map that keeps initial version of component/directive/pipe defs in case // we compile a Type again, thus overriding respective static fields. This is @@ -92,7 +99,7 @@ export class R3TestBedCompiler { configureTestingModule(moduleDef: TestModuleMetadata): void { // Enqueue any compilation tasks for the directly declared component. if (moduleDef.declarations !== undefined) { - this.queueTypeArray(moduleDef.declarations, TESTING_MODULE); + this.queueTypeArray(moduleDef.declarations, TestingModuleOverride.DECLARATION); this.declarations.push(...moduleDef.declarations); } @@ -188,7 +195,7 @@ export class R3TestBedCompiler { } // Set the component's scope to be the testing module. - this.componentToModuleScope.set(type, TESTING_MODULE); + this.componentToModuleScope.set(type, TestingModuleOverride.OVERRIDE_TEMPLATE); } async compileComponents(): Promise { @@ -306,14 +313,15 @@ export class R3TestBedCompiler { } private applyTransitiveScopes(): void { - const moduleToScope = new Map|TESTING_MODULE, NgModuleTransitiveScopes>(); - const getScopeOfModule = (moduleType: Type| TESTING_MODULE): NgModuleTransitiveScopes => { - if (!moduleToScope.has(moduleType)) { - const realType = moduleType === TESTING_MODULE ? this.testModuleType : moduleType; - moduleToScope.set(moduleType, transitiveScopesFor(realType)); - } - return moduleToScope.get(moduleType) !; - }; + const moduleToScope = new Map|TestingModuleOverride, NgModuleTransitiveScopes>(); + const getScopeOfModule = + (moduleType: Type| TestingModuleOverride): NgModuleTransitiveScopes => { + if (!moduleToScope.has(moduleType)) { + const realType = isTestingModuleOverride(moduleType) ? this.testModuleType : moduleType; + moduleToScope.set(moduleType, transitiveScopesFor(realType)); + } + return moduleToScope.get(moduleType) !; + }; this.componentToModuleScope.forEach((moduleType, componentType) => { const moduleScope = getScopeOfModule(moduleType); @@ -370,7 +378,7 @@ export class R3TestBedCompiler { this.existingComponentStyles.clear(); } - private queueTypeArray(arr: any[], moduleType: Type|TESTING_MODULE): void { + private queueTypeArray(arr: any[], moduleType: Type|TestingModuleOverride): void { for (const value of arr) { if (Array.isArray(value)) { this.queueTypeArray(value, moduleType); @@ -392,7 +400,7 @@ export class R3TestBedCompiler { compileNgModuleDefs(ngModule as NgModuleType, metadata); } - private queueType(type: Type, moduleType: Type|TESTING_MODULE): void { + private queueType(type: Type, moduleType: Type|TestingModuleOverride): void { const component = this.resolvers.component.resolve(type); if (component) { // Check whether a give Type has respective NG def (ngComponentDef) and compile if def is @@ -404,9 +412,22 @@ export class R3TestBedCompiler { this.seenComponents.add(type); // Keep track of the module which declares this component, so later the component's scope - // can be set correctly. Only record this the first time, because it might be overridden by - // overrideTemplateUsingTestingModule. - if (!this.componentToModuleScope.has(type)) { + // can be set correctly. If the component has already been recorded here, then one of several + // cases is true: + // * the module containing the component was imported multiple times (common). + // * the component is declared in multiple modules (which is an error). + // * the component was in 'declarations' of the testing module, and also in an imported module + // in which case the module scope will be TestingModuleOverride.DECLARATION. + // * overrideTemplateUsingTestingModule was called for the component in which case the module + // scope will be TestingModuleOverride.OVERRIDE_TEMPLATE. + // + // If the component was previously in the testing module's 'declarations' (meaning the + // current value is TestingModuleOverride.DECLARATION), then `moduleType` is the component's + // real module, which was imported. This pattern is understood to mean that the component + // should use its original scope, but that the testing module should also contain the + // component in its scope. + if (!this.componentToModuleScope.has(type) || + this.componentToModuleScope.get(type) === TestingModuleOverride.DECLARATION) { this.componentToModuleScope.set(type, moduleType); } return; @@ -525,7 +546,7 @@ export class R3TestBedCompiler { imports, schemas: this.schemas, providers, - }); + }, /* allowDuplicateDeclarationsInRoot */ true); // clang-format on this.applyProviderOverridesToModule(this.testModuleType);