From 0aa0f11a2bd1f480c7182efc2d3b65e40cbade03 Mon Sep 17 00:00:00 2001 From: Kara Erickson Date: Thu, 11 Apr 2019 16:11:45 -0700 Subject: [PATCH] fix(ivy): log errors instead of re-throwing them (#29853) In order to be backwards compatible with View Engine, Ivy should log errors by default in the TestBed error handler rather than re-throwing them. Re-throwing the errors is a breaking change that causes issues with libraries like ngrx that have async behavior and custom error handling. This logging approach has issues (for both VE and Ivy) because it can allow tests to pass inappropriately if errors are thrown inside listeners. However, since re-throwing would be breaking and requires a larger redesign, we should wait until post-Ivy. PR Close #29853 --- packages/core/test/test_bed_spec.ts | 45 +++++++++++++++++++ .../core/testing/src/r3_test_bed_compiler.ts | 14 +----- 2 files changed, 47 insertions(+), 12 deletions(-) diff --git a/packages/core/test/test_bed_spec.ts b/packages/core/test/test_bed_spec.ts index 0070e06797..d9da302e95 100644 --- a/packages/core/test/test_bed_spec.ts +++ b/packages/core/test/test_bed_spec.ts @@ -267,6 +267,51 @@ describe('TestBed', () => { TestBed.configureTestingModule({imports: [ProvidesErrorHandler, HelloWorldModule]}); expect(TestBed.get(ErrorHandler)).toEqual(jasmine.any(CustomErrorHandler)); + + }); + + it('should throw errors in CD', () => { + @Component({selector: 'my-comp', template: ''}) + class MyComp { + name !: {hello: string}; + + ngOnInit() { + // this should throw because this.name is undefined + this.name.hello = 'hello'; + } + } + + TestBed.configureTestingModule({declarations: [MyComp]}); + + expect(() => { + const fixture = TestBed.createComponent(MyComp); + fixture.detectChanges(); + }).toThrowError(); + }); + + // TODO(FW-1245): properly fix issue where errors in listeners aren't thrown and don't cause + // tests to fail. This is an issue in both View Engine and Ivy, and may require a breaking + // change to completely fix (since simple re-throwing breaks handlers in ngrx, etc). + xit('should throw errors in listeners', () => { + + @Component({selector: 'my-comp', template: ''}) + class MyComp { + name !: {hello: string}; + + onClick() { + // this should throw because this.name is undefined + this.name.hello = 'hello'; + } + } + + TestBed.configureTestingModule({declarations: [MyComp]}); + const fixture = TestBed.createComponent(MyComp); + fixture.detectChanges(); + + expect(() => { + const button = fixture.nativeElement.querySelector('button'); + button.click(); + }).toThrowError(); }); onlyInIvy('TestBed should handle AOT pre-compiled Components') diff --git a/packages/core/testing/src/r3_test_bed_compiler.ts b/packages/core/testing/src/r3_test_bed_compiler.ts index b5222215ac..aac9429371 100644 --- a/packages/core/testing/src/r3_test_bed_compiler.ts +++ b/packages/core/testing/src/r3_test_bed_compiler.ts @@ -6,7 +6,7 @@ * found in the LICENSE file at https://angular.io/license */ -import {ApplicationInitStatus, COMPILER_OPTIONS, Compiler, Component, Directive, ErrorHandler, ModuleWithComponentFactories, NgModule, NgModuleFactory, NgZone, Injector, Pipe, PlatformRef, Provider, Type, ɵcompileComponent as compileComponent, ɵcompileDirective as compileDirective, ɵcompileNgModuleDefs as compileNgModuleDefs, ɵcompilePipe as compilePipe, ɵgetInjectableDef as getInjectableDef, ɵNG_COMPONENT_DEF as NG_COMPONENT_DEF, ɵNG_DIRECTIVE_DEF as NG_DIRECTIVE_DEF, ɵNG_INJECTOR_DEF as NG_INJECTOR_DEF, ɵNG_MODULE_DEF as NG_MODULE_DEF, ɵNG_PIPE_DEF as NG_PIPE_DEF, ɵRender3ComponentFactory as ComponentFactory, ɵRender3NgModuleRef as NgModuleRef, ɵɵInjectableDef as InjectableDef, ɵNgModuleFactory as R3NgModuleFactory, ɵNgModuleTransitiveScopes as NgModuleTransitiveScopes, ɵNgModuleType as NgModuleType, ɵDirectiveDef as DirectiveDef, ɵpatchComponentDefWithScope as patchComponentDefWithScope, ɵtransitiveScopesFor as transitiveScopesFor,} from '@angular/core'; +import {ApplicationInitStatus, COMPILER_OPTIONS, Compiler, Component, Directive, ModuleWithComponentFactories, NgModule, NgModuleFactory, NgZone, Injector, Pipe, PlatformRef, Provider, Type, ɵcompileComponent as compileComponent, ɵcompileDirective as compileDirective, ɵcompileNgModuleDefs as compileNgModuleDefs, ɵcompilePipe as compilePipe, ɵgetInjectableDef as getInjectableDef, ɵNG_COMPONENT_DEF as NG_COMPONENT_DEF, ɵNG_DIRECTIVE_DEF as NG_DIRECTIVE_DEF, ɵNG_INJECTOR_DEF as NG_INJECTOR_DEF, ɵNG_MODULE_DEF as NG_MODULE_DEF, ɵNG_PIPE_DEF as NG_PIPE_DEF, ɵRender3ComponentFactory as ComponentFactory, ɵRender3NgModuleRef as NgModuleRef, ɵɵInjectableDef as InjectableDef, ɵNgModuleFactory as R3NgModuleFactory, ɵNgModuleTransitiveScopes as NgModuleTransitiveScopes, ɵNgModuleType as NgModuleType, ɵDirectiveDef as DirectiveDef, ɵpatchComponentDefWithScope as patchComponentDefWithScope, ɵtransitiveScopesFor as transitiveScopesFor,} from '@angular/core'; import {ResourceLoader} from '@angular/compiler'; import {clearResolutionOfComponentResourcesQueue, restoreComponentResolutionQueue, resolveComponentResources, isComponentDefPendingResolution} from '../../src/metadata/resource_loading'; @@ -513,10 +513,6 @@ export class R3TestBedCompiler { class RootScopeModule { } - @NgModule({providers: [{provide: ErrorHandler, useClass: R3TestErrorHandler}]}) - class R3ErrorHandlerModule { - } - const ngZone = new NgZone({enableLongStackTrace: true}); const providers: Provider[] = [ {provide: NgZone, useValue: ngZone}, @@ -524,8 +520,7 @@ export class R3TestBedCompiler { ...this.providers, ...this.providerOverrides, ]; - const imports = - [RootScopeModule, this.additionalModuleTypes, R3ErrorHandlerModule, this.imports || []]; + const imports = [RootScopeModule, this.additionalModuleTypes, this.imports || []]; // clang-format off compileNgModuleDefs(this.testModuleType, { @@ -633,11 +628,6 @@ function flatten(values: any[], mapFn?: (value: T) => any): T[] { return out; } -/** Error handler used for tests. Rethrows errors rather than logging them out. */ -class R3TestErrorHandler extends ErrorHandler { - handleError(error: any) { throw error; } -} - class R3TestCompiler implements Compiler { constructor(private testBed: R3TestBedCompiler) {}