From c2efa23e94bf9ef3594bfa37aa30a0fc1dc2620a Mon Sep 17 00:00:00 2001 From: vsavkin Date: Sun, 5 Jul 2015 14:46:35 -0700 Subject: [PATCH] fix(change_detection): throw ChangeDetectionError in JIT mode --- .../abstract_change_detector.ts | 6 +++++ .../change_detection_jit_generator.ts | 22 ++++++++++++++----- .../dynamic_change_detector.ts | 4 +--- .../src/change_detection/exceptions.ts | 19 +++++----------- .../change_detector_codegen.dart | 13 +++++++++-- .../change_detector_config.ts | 3 ++- .../change_detection/change_detector_spec.ts | 7 +++--- .../expected/bar.ng_deps.dart | 11 +++++++++- 8 files changed, 54 insertions(+), 31 deletions(-) diff --git a/modules/angular2/src/change_detection/abstract_change_detector.ts b/modules/angular2/src/change_detection/abstract_change_detector.ts index 3b51309d58..34a77d6093 100644 --- a/modules/angular2/src/change_detection/abstract_change_detector.ts +++ b/modules/angular2/src/change_detection/abstract_change_detector.ts @@ -2,6 +2,8 @@ import {isPresent} from 'angular2/src/facade/lang'; import {List, ListWrapper} from 'angular2/src/facade/collection'; import {ChangeDetectorRef} from './change_detector_ref'; import {ChangeDetector} from './interfaces'; +import {ChangeDetectionError} from './exceptions'; +import {ProtoRecord} from './proto_record'; import {Locals} from './parser/locals'; import {CHECK_ALWAYS, CHECK_ONCE, CHECKED, DETACHED, ON_PUSH} from './constants'; @@ -79,4 +81,8 @@ export class AbstractChangeDetector implements ChangeDetector { c = c.parent; } } + + throwError(proto: ProtoRecord, exception: any, stack: any): void { + throw new ChangeDetectionError(proto, exception, stack); + } } diff --git a/modules/angular2/src/change_detection/change_detection_jit_generator.ts b/modules/angular2/src/change_detection/change_detection_jit_generator.ts index 0731a742a7..e17d74ecaa 100644 --- a/modules/angular2/src/change_detection/change_detection_jit_generator.ts +++ b/modules/angular2/src/change_detection/change_detection_jit_generator.ts @@ -28,7 +28,7 @@ var IS_CHANGED_LOCAL = "isChanged"; var CHANGES_LOCAL = "changes"; var LOCALS_ACCESSOR = "this.locals"; var MODE_ACCESSOR = "this.mode"; -var CURRENT_PROTO = "currentProto"; +var CURRENT_PROTO = "this.currentProto"; var ALREADY_CHECKED_ACCESSOR = "this.alreadyChecked"; @@ -74,6 +74,7 @@ export class ChangeDetectorJITGenerator { ${PROTOS_ACCESSOR} = protos; ${DIRECTIVES_ACCESSOR} = directiveRecords; ${LOCALS_ACCESSOR} = null; + ${CURRENT_PROTO} = null; ${ALREADY_CHECKED_ACCESSOR} = false; ${this._genFieldDefinitions()} } @@ -84,19 +85,28 @@ export class ChangeDetectorJITGenerator { if (!this.hydrated()) { ${UTIL}.throwDehydrated(); } + try { + this.__detectChangesInRecords(throwOnChange); + } catch (e) { + this.throwError(${CURRENT_PROTO}, e, e.stack); + } + } + + ${typeName}.prototype.__detectChangesInRecords = function(throwOnChange) { + ${CURRENT_PROTO} = null; + ${this._genLocalDefinitions()} ${this._genChangeDefinitions()} var ${IS_CHANGED_LOCAL} = false; - var ${CURRENT_PROTO}; var ${CHANGES_LOCAL} = null; - + context = ${CONTEXT_ACCESSOR}; - + ${this.records.map((r) => this._genRecord(r)).join("\n")} - + ${ALREADY_CHECKED_ACCESSOR} = true; } - + ${typeName}.prototype.callOnAllChangesDone = function() { ${this._genCallOnAllChangesDoneBody()} } diff --git a/modules/angular2/src/change_detection/dynamic_change_detector.ts b/modules/angular2/src/change_detection/dynamic_change_detector.ts index 70918213aa..b44c5a96ba 100644 --- a/modules/angular2/src/change_detection/dynamic_change_detector.ts +++ b/modules/angular2/src/change_detection/dynamic_change_detector.ts @@ -10,8 +10,6 @@ import {ChangeDetectionUtil, SimpleChange, uninitialized} from './change_detecti import {ProtoRecord, RecordType} from './proto_record'; -import {ExpressionChangedAfterItHasBeenChecked, ChangeDetectionError} from './exceptions'; - export class DynamicChangeDetector extends AbstractChangeDetector { locals: Locals = null; values: List; @@ -149,7 +147,7 @@ export class DynamicChangeDetector extends AbstractChangeDetector { return this._referenceCheck(proto, throwOnChange); } } catch (e) { - throw new ChangeDetectionError(proto, e); + this.throwError(proto, e, e.stack); } } diff --git a/modules/angular2/src/change_detection/exceptions.ts b/modules/angular2/src/change_detection/exceptions.ts index 38aa43202f..f5863f0573 100644 --- a/modules/angular2/src/change_detection/exceptions.ts +++ b/modules/angular2/src/change_detection/exceptions.ts @@ -2,29 +2,20 @@ import {ProtoRecord} from './proto_record'; import {BaseException} from "angular2/src/facade/lang"; export class ExpressionChangedAfterItHasBeenChecked extends BaseException { - message: string; - constructor(proto: ProtoRecord, change: any) { - super(); - this.message = - `Expression '${proto.expressionAsString}' has changed after it was checked. ` + - `Previous value: '${change.previousValue}'. Current value: '${change.currentValue}'`; + super(`Expression '${proto.expressionAsString}' has changed after it was checked. ` + + `Previous value: '${change.previousValue}'. Current value: '${change.currentValue}'`); } - - toString(): string { return this.message; } } export class ChangeDetectionError extends BaseException { - message: string; location: string; - constructor(proto: ProtoRecord, public originalException: any) { - super(); + constructor(proto: ProtoRecord, originalException: any, originalStack: any) { + super(`${originalException} in [${proto.expressionAsString}]`, originalException, + originalStack); this.location = proto.expressionAsString; - this.message = `${this.originalException} in [${this.location}]`; } - - toString(): string { return this.message; } } export class DehydratedException extends BaseException { diff --git a/modules/angular2/src/transform/template_compiler/change_detector_codegen.dart b/modules/angular2/src/transform/template_compiler/change_detector_codegen.dart index cefcdb4237..7116af55f8 100644 --- a/modules/angular2/src/transform/template_compiler/change_detector_codegen.dart +++ b/modules/angular2/src/transform/template_compiler/change_detector_codegen.dart @@ -131,6 +131,7 @@ class _CodegenState { $_DIRECTIVES_ACCESSOR; dynamic $_LOCALS_ACCESSOR = null; dynamic $_ALREADY_CHECKED_ACCESSOR = false; + dynamic $_CURRENT_PROTO = null; ${_allFields().map((f) { if (f == _CONTEXT_ACCESSOR) { return '$_contextTypeName $f = null;'; @@ -148,10 +149,18 @@ class _CodegenState { if (!hydrated()) { $_UTIL.throwDehydrated(); } + try { + this.__detectChangesInRecords(throwOnChange); + } catch (e, s) { + this.throwError($_CURRENT_PROTO, e, s); + } + } + + void __detectChangesInRecords(throwOnChange) { ${_genLocalDefinitions()} ${_genChangeDefinitons()} var $_IS_CHANGED_LOCAL = false; - var $_CURRENT_PROTO; + $_CURRENT_PROTO = null; var $_CHANGES_LOCAL = null; context = $_CONTEXT_ACCESSOR; @@ -159,7 +168,7 @@ class _CodegenState { $_ALREADY_CHECKED_ACCESSOR = true; } - + void callOnAllChangesDone() { ${_getCallOnAllChangesDoneBody()} } diff --git a/modules/angular2/test/change_detection/change_detector_config.ts b/modules/angular2/test/change_detection/change_detector_config.ts index 44bcb83af6..ac223956b9 100644 --- a/modules/angular2/test/change_detection/change_detector_config.ts +++ b/modules/angular2/test/change_detection/change_detector_config.ts @@ -309,5 +309,6 @@ var _availableDefinitions = [ 'sayHi("Jim")', 'a()(99)', 'a.sayHi("Jim")', - 'passThrough([12])' + 'passThrough([12])', + 'invalidFn(1)' ]; diff --git a/modules/angular2/test/change_detection/change_detector_spec.ts b/modules/angular2/test/change_detection/change_detector_spec.ts index 20ca3a550d..eb319f68ce 100644 --- a/modules/angular2/test/change_detection/change_detector_spec.ts +++ b/modules/angular2/test/change_detection/change_detector_spec.ts @@ -542,16 +542,15 @@ export function main() { }); }); - // TODO vsavkin: implement it describe('error handling', () => { - xit('should wrap exceptions into ChangeDetectionError', () => { - var val = _createChangeDetector('invalidProp'); + it('should wrap exceptions into ChangeDetectionError', () => { + var val = _createChangeDetector('invalidFn(1)'); try { val.changeDetector.detectChanges(); throw new BaseException('fail'); } catch (e) { expect(e).toBeAnInstanceOf(ChangeDetectionError); - expect(e.location).toEqual('invalidProp in someComponent'); + expect(e.location).toEqual('invalidFn(1) in location'); } }); }); diff --git a/modules/angular2/test/transform/integration/two_annotations_files/expected/bar.ng_deps.dart b/modules/angular2/test/transform/integration/two_annotations_files/expected/bar.ng_deps.dart index ea430ff3bf..5279e94385 100644 --- a/modules/angular2/test/transform/integration/two_annotations_files/expected/bar.ng_deps.dart +++ b/modules/angular2/test/transform/integration/two_annotations_files/expected/bar.ng_deps.dart @@ -32,6 +32,7 @@ class _MyComponent_ChangeDetector0 extends _gen.AbstractChangeDetector { final _gen.List<_gen.DirectiveRecord> _directiveRecords; dynamic _locals = null; dynamic _alreadyChecked = false; + dynamic currentProto = null; MyComponent _context = null; dynamic _myNum0 = _gen.ChangeDetectionUtil.uninitialized(); dynamic _interpolate1 = _gen.ChangeDetectionUtil.uninitialized(); @@ -44,6 +45,14 @@ class _MyComponent_ChangeDetector0 extends _gen.AbstractChangeDetector { if (!hydrated()) { _gen.ChangeDetectionUtil.throwDehydrated(); } + try { + this.__detectChangesInRecords(throwOnChange); + } catch (e, s) { + this.throwError(currentProto, e, s); + } + } + + void __detectChangesInRecords(throwOnChange) { var context = null; var myNum0 = null; var interpolate1 = null; @@ -51,7 +60,7 @@ class _MyComponent_ChangeDetector0 extends _gen.AbstractChangeDetector { var change_myNum0 = false; var change_interpolate1 = false; var isChanged = false; - var currentProto; + currentProto = null; var changes = null; context = _context;