refactor(ivy): check metadata presence before compiling Type in R3TestBed (#34204)

Prior to this commit, there was no check in R3TestBed to verify that metadata is resolved using a given Type. That leads to some cryptic error messages (when TestBed tries to compile a Type without having metadata) in case TestBed override functions receive unexpected Types (for example a Directive is used in `TestBed.overrideComponent` call). This commit adds the necessary checks to verify metadata presence before TestBed tries to (re)compile a Type.

PR Close #34204
This commit is contained in:
Andrew Kushnir 2019-12-02 21:51:19 -08:00 committed by Miško Hevery
parent c50faa97ca
commit 9b6a1b85b1
2 changed files with 49 additions and 10 deletions

View File

@ -6,7 +6,7 @@
* found in the LICENSE file at https://angular.io/license * found in the LICENSE file at https://angular.io/license
*/ */
import {Compiler, Component, Directive, ErrorHandler, Inject, Injectable, InjectionToken, Injector, Input, ModuleWithProviders, NgModule, Optional, Pipe, ViewChild, ɵsetClassMetadata as setClassMetadata, ɵɵdefineComponent as defineComponent, ɵɵdefineNgModule as defineNgModule, ɵɵtext as text} from '@angular/core'; import {Compiler, Component, Directive, ErrorHandler, Inject, Injectable, InjectionToken, Injector, Input, ModuleWithProviders, NgModule, Optional, Pipe, Type, ViewChild, ɵ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 {TestBed, getTestBed} from '@angular/core/testing/src/test_bed';
import {By} from '@angular/platform-browser'; import {By} from '@angular/platform-browser';
import {expect} from '@angular/platform-browser/testing/src/matchers'; import {expect} from '@angular/platform-browser/testing/src/matchers';
@ -715,6 +715,36 @@ describe('TestBed', () => {
expect(fixture.nativeElement.innerHTML).toEqual('<outer><inner>Inner</inner></outer>'); expect(fixture.nativeElement.innerHTML).toEqual('<outer><inner>Inner</inner></outer>');
}); });
onlyInIvy('Ivy-specific errors').describe('checking types before compiling them', () => {
@Directive({
selector: 'my-dir',
})
class MyDir {
}
@NgModule()
class MyModule {
}
// [decorator, type, overrideFn]
const cases: [string, Type<any>, string][] = [
['Component', MyDir, 'overrideComponent'],
['NgModule', MyDir, 'overrideModule'],
['Pipe', MyModule, 'overridePipe'],
['Directive', MyModule, 'overrideDirective'],
];
cases.forEach(([decorator, type, overrideFn]) => {
it(`should throw an error in case invalid type is used in ${overrideFn} function`, () => {
TestBed.configureTestingModule({declarations: [MyDir]});
expect(() => {
(TestBed as any)[overrideFn](type, {});
TestBed.createComponent(type);
}).toThrowError(new RegExp(`class doesn't have @${decorator} decorator`, 'g'));
});
});
});
onlyInIvy('TestBed should handle AOT pre-compiled Components') onlyInIvy('TestBed should handle AOT pre-compiled Components')
.describe('AOT pre-compiled components', () => { .describe('AOT pre-compiled components', () => {
/** /**

View File

@ -129,10 +129,10 @@ export class R3TestBedCompiler {
this.resolvers.module.addOverride(ngModule, override); this.resolvers.module.addOverride(ngModule, override);
const metadata = this.resolvers.module.resolve(ngModule); const metadata = this.resolvers.module.resolve(ngModule);
if (metadata === null) { if (metadata === null) {
throw new Error(`${ngModule.name} is not an @NgModule or is missing metadata`); throw invalidTypeError(ngModule.name, 'NgModule');
} }
this.recompileNgModule(ngModule); this.recompileNgModule(ngModule, metadata);
// At this point, the module has a valid module def (ɵmod), but the override may have introduced // At this point, the module has a valid module def (ɵmod), but the override may have introduced
// new declarations or imported modules. Ingest any possible new types and add them to the // new declarations or imported modules. Ingest any possible new types and add them to the
@ -306,7 +306,10 @@ export class R3TestBedCompiler {
let needsAsyncResources = false; let needsAsyncResources = false;
this.pendingComponents.forEach(declaration => { this.pendingComponents.forEach(declaration => {
needsAsyncResources = needsAsyncResources || isComponentDefPendingResolution(declaration); needsAsyncResources = needsAsyncResources || isComponentDefPendingResolution(declaration);
const metadata = this.resolvers.component.resolve(declaration) !; const metadata = this.resolvers.component.resolve(declaration);
if (metadata === null) {
throw invalidTypeError(declaration.name, 'Component');
}
this.maybeStoreNgDef(NG_COMP_DEF, declaration); this.maybeStoreNgDef(NG_COMP_DEF, declaration);
compileComponent(declaration, metadata); compileComponent(declaration, metadata);
}); });
@ -314,13 +317,19 @@ export class R3TestBedCompiler {
this.pendingDirectives.forEach(declaration => { this.pendingDirectives.forEach(declaration => {
const metadata = this.resolvers.directive.resolve(declaration); const metadata = this.resolvers.directive.resolve(declaration);
if (metadata === null) {
throw invalidTypeError(declaration.name, 'Directive');
}
this.maybeStoreNgDef(NG_DIR_DEF, declaration); this.maybeStoreNgDef(NG_DIR_DEF, declaration);
compileDirective(declaration, metadata); compileDirective(declaration, metadata);
}); });
this.pendingDirectives.clear(); this.pendingDirectives.clear();
this.pendingPipes.forEach(declaration => { this.pendingPipes.forEach(declaration => {
const metadata = this.resolvers.pipe.resolve(declaration) !; const metadata = this.resolvers.pipe.resolve(declaration);
if (metadata === null) {
throw invalidTypeError(declaration.name, 'Pipe');
}
this.maybeStoreNgDef(NG_PIPE_DEF, declaration); this.maybeStoreNgDef(NG_PIPE_DEF, declaration);
compilePipe(declaration, metadata); compilePipe(declaration, metadata);
}); });
@ -434,11 +443,7 @@ export class R3TestBedCompiler {
} }
} }
private recompileNgModule(ngModule: Type<any>): void { private recompileNgModule(ngModule: Type<any>, metadata: NgModule): void {
const metadata = this.resolvers.module.resolve(ngModule);
if (metadata === null) {
throw new Error(`Unable to resolve metadata for NgModule: ${ngModule.name}`);
}
// Cache the initial ngModuleDef as it will be overwritten. // Cache the initial ngModuleDef as it will be overwritten.
this.maybeStoreNgDef(NG_MOD_DEF, ngModule); this.maybeStoreNgDef(NG_MOD_DEF, ngModule);
this.maybeStoreNgDef(NG_INJ_DEF, ngModule); this.maybeStoreNgDef(NG_INJ_DEF, ngModule);
@ -751,6 +756,10 @@ function forEachRight<T>(values: T[], fn: (value: T, idx: number) => void): void
} }
} }
function invalidTypeError(name: string, expectedType: string): Error {
return new Error(`${name} class doesn't have @${expectedType} decorator or is missing metadata.`);
}
class R3TestCompiler implements Compiler { class R3TestCompiler implements Compiler {
constructor(private testBed: R3TestBedCompiler) {} constructor(private testBed: R3TestBedCompiler) {}