diff --git a/modules/angular2/src/core/change_detection/abstract_change_detector.ts b/modules/angular2/src/core/change_detection/abstract_change_detector.ts index 886d4ea0db30e..45141238bb71a 100644 --- a/modules/angular2/src/core/change_detection/abstract_change_detector.ts +++ b/modules/angular2/src/core/change_detection/abstract_change_detector.ts @@ -13,7 +13,7 @@ import { } from './exceptions'; import {BindingTarget} from './binding_record'; import {Locals} from './parser/locals'; -import {ChangeDetectionStrategy} from './constants'; +import {ChangeDetectionStrategy, ChangeDetectorState} from './constants'; import {wtfCreateScope, wtfLeave, WtfScopeFn} from '../profile/profile'; import {isObservable} from './observable_facade'; @@ -33,7 +33,7 @@ export class AbstractChangeDetector implements ChangeDetector { // The names of the below fields must be kept in sync with codegen_name_util.ts or // change detection will fail. - alreadyChecked: any = false; + state: ChangeDetectorState = ChangeDetectorState.NeverChecked; context: T; locals: Locals = null; mode: ChangeDetectionStrategy = null; @@ -80,7 +80,7 @@ export class AbstractChangeDetector implements ChangeDetector { runDetectChanges(throwOnChange: boolean): void { if (this.mode === ChangeDetectionStrategy.Detached || - this.mode === ChangeDetectionStrategy.Checked) + this.mode === ChangeDetectionStrategy.Checked || this.state === ChangeDetectorState.Errored) return; var s = _scope_check(this.id, throwOnChange); @@ -95,7 +95,7 @@ export class AbstractChangeDetector implements ChangeDetector { if (this.mode === ChangeDetectionStrategy.CheckOnce) this.mode = ChangeDetectionStrategy.Checked; - this.alreadyChecked = true; + this.state = ChangeDetectorState.CheckedBefore; wtfLeave(s); } @@ -112,6 +112,10 @@ export class AbstractChangeDetector implements ChangeDetector { try { this.detectChangesInRecordsInternal(throwOnChange); } catch (e) { + // throwOnChange errors aren't counted as fatal errors. + if (!(e instanceof ExpressionChangedAfterItHasBeenCheckedException)) { + this.state = ChangeDetectorState.Errored; + } this._throwError(e, e.stack); } } @@ -137,7 +141,7 @@ export class AbstractChangeDetector implements ChangeDetector { this.locals = locals; this.pipes = pipes; this.hydrateDirectives(directives); - this.alreadyChecked = false; + this.state = ChangeDetectorState.NeverChecked; } // Subclasses should override this method to hydrate any directives. diff --git a/modules/angular2/src/core/change_detection/change_detection_jit_generator.dart b/modules/angular2/src/core/change_detection/change_detection_jit_generator.dart index 7811a13b06ae5..d2ec55ce8c373 100644 --- a/modules/angular2/src/core/change_detection/change_detection_jit_generator.dart +++ b/modules/angular2/src/core/change_detection/change_detection_jit_generator.dart @@ -9,7 +9,7 @@ library change_detection.change_detection_jit_generator; class ChangeDetectorJITGenerator { String typeName; ChangeDetectorJITGenerator( - definition, changeDetectionUtilVarName, abstractChangeDetectorVarName) {} + definition, changeDetectionUtilVarName, abstractChangeDetectorVarName, changeDetectorStateVarName) {} generate() { throw "Jit Change Detection is not supported in Dart"; diff --git a/modules/angular2/src/core/change_detection/change_detection_jit_generator.ts b/modules/angular2/src/core/change_detection/change_detection_jit_generator.ts index c10bbac2335b9..79473de4e6df3 100644 --- a/modules/angular2/src/core/change_detection/change_detection_jit_generator.ts +++ b/modules/angular2/src/core/change_detection/change_detection_jit_generator.ts @@ -13,7 +13,7 @@ import {codify} from './codegen_facade'; import {EventBinding} from './event_binding'; import {BindingTarget} from './binding_record'; import {ChangeDetectorGenConfig, ChangeDetectorDefinition} from './interfaces'; -import {ChangeDetectionStrategy} from './constants'; +import {ChangeDetectionStrategy, ChangeDetectorState} from './constants'; import {createPropertyRecords, createEventRecords} from './proto_change_detector'; /** @@ -41,7 +41,8 @@ export class ChangeDetectorJITGenerator { typeName: string; constructor(definition: ChangeDetectorDefinition, private changeDetectionUtilVarName: string, - private abstractChangeDetectorVarName: string) { + private abstractChangeDetectorVarName: string, + private changeDetectorStateVarName: string) { var propertyBindingRecords = createPropertyRecords(definition); var eventBindingRecords = createEventRecords(definition); var propertyBindingTargets = definition.bindingRecords.map(b => b.target); @@ -55,8 +56,9 @@ export class ChangeDetectorJITGenerator { this.directiveRecords = definition.directiveRecords; this._names = new CodegenNameUtil(this.records, this.eventBindings, this.directiveRecords, this.changeDetectionUtilVarName); - this._logic = new CodegenLogicUtil(this._names, this.changeDetectionUtilVarName, - this.changeDetectionStrategy); + this._logic = + new CodegenLogicUtil(this._names, this.changeDetectionUtilVarName, + this.changeDetectorStateVarName, this.changeDetectionStrategy); this.typeName = sanitizeName(`ChangeDetector_${this.id}`); } @@ -68,7 +70,8 @@ export class ChangeDetectorJITGenerator { } `; return new Function(this.abstractChangeDetectorVarName, this.changeDetectionUtilVarName, - factorySource)(AbstractChangeDetector, ChangeDetectionUtil); + this.changeDetectorStateVarName, factorySource)( + AbstractChangeDetector, ChangeDetectionUtil, ChangeDetectorState); } generateSource(): string { @@ -423,7 +426,7 @@ export class ChangeDetectorJITGenerator { /** @internal */ _genOnInit(r: ProtoRecord): string { var br = r.bindingRecord; - return `if (!throwOnChange && !${this._names.getAlreadyCheckedName()}) ${this._names.getDirectiveName(br.directiveRecord.directiveIndex)}.onInit();`; + return `if (!throwOnChange && ${this._names.getStateName()} === ${this.changeDetectorStateVarName}.NeverChecked) ${this._names.getDirectiveName(br.directiveRecord.directiveIndex)}.onInit();`; } /** @internal */ diff --git a/modules/angular2/src/core/change_detection/codegen_logic_util.ts b/modules/angular2/src/core/change_detection/codegen_logic_util.ts index bb669ba4cc104..7735df5a8d45a 100644 --- a/modules/angular2/src/core/change_detection/codegen_logic_util.ts +++ b/modules/angular2/src/core/change_detection/codegen_logic_util.ts @@ -12,6 +12,7 @@ import {BaseException} from 'angular2/src/core/facade/exceptions'; */ export class CodegenLogicUtil { constructor(private _names: CodegenNameUtil, private _utilName: string, + private _changeDetectorStateName: string, private _changeDetection: ChangeDetectionStrategy) {} /** @@ -187,7 +188,7 @@ export class CodegenLogicUtil { var dir = directiveRecords[i]; if (dir.callAfterContentInit) { res.push( - `if(! ${this._names.getAlreadyCheckedName()}) ${this._names.getDirectiveName(dir.directiveIndex)}.afterContentInit();`); + `if(${this._names.getStateName()} === ${this._changeDetectorStateName}.NeverChecked) ${this._names.getDirectiveName(dir.directiveIndex)}.afterContentInit();`); } if (dir.callAfterContentChecked) { @@ -204,7 +205,7 @@ export class CodegenLogicUtil { var dir = directiveRecords[i]; if (dir.callAfterViewInit) { res.push( - `if(! ${this._names.getAlreadyCheckedName()}) ${this._names.getDirectiveName(dir.directiveIndex)}.afterViewInit();`); + `if(${this._names.getStateName()} === ${this._changeDetectorStateName}.NeverChecked) ${this._names.getDirectiveName(dir.directiveIndex)}.afterViewInit();`); } if (dir.callAfterViewChecked) { diff --git a/modules/angular2/src/core/change_detection/codegen_name_util.ts b/modules/angular2/src/core/change_detection/codegen_name_util.ts index 6660c2e57e33e..bef3f362c82cc 100644 --- a/modules/angular2/src/core/change_detection/codegen_name_util.ts +++ b/modules/angular2/src/core/change_detection/codegen_name_util.ts @@ -8,7 +8,8 @@ import {EventBinding} from './event_binding'; // The names of these fields must be kept in sync with abstract_change_detector.ts or change // detection will fail. -const _ALREADY_CHECKED_ACCESSOR = "alreadyChecked"; +const _STATE_ACCESSOR = "state"; +const _CONTEXT_ACCESSOR = "context"; const _PROP_BINDING_INDEX = "propertyBindingIndex"; const _DIRECTIVES_ACCESSOR = "directiveIndices"; const _DISPATCHER_ACCESSOR = "dispatcher"; @@ -77,7 +78,7 @@ export class CodegenNameUtil { getLocalsAccessorName(): string { return this._addFieldPrefix(_LOCALS_ACCESSOR); } - getAlreadyCheckedName(): string { return this._addFieldPrefix(_ALREADY_CHECKED_ACCESSOR); } + getStateName(): string { return this._addFieldPrefix(_STATE_ACCESSOR); } getModeName(): string { return this._addFieldPrefix(_MODE_ACCESSOR); } diff --git a/modules/angular2/src/core/change_detection/constants.ts b/modules/angular2/src/core/change_detection/constants.ts index fc3666e5331e2..5c77005bcaf56 100644 --- a/modules/angular2/src/core/change_detection/constants.ts +++ b/modules/angular2/src/core/change_detection/constants.ts @@ -1,5 +1,26 @@ import {StringWrapper, normalizeBool, isBlank} from 'angular2/src/core/facade/lang'; +export enum ChangeDetectorState { + /** + * `NeverChecked` means that the change detector has not been checked yet, and + * initialization methods should be called during detection. + */ + NeverChecked, + + /** + * `CheckedBefore` means that the change detector has successfully completed at least + * one detection previously. + */ + CheckedBefore, + + /** + * `Errored` means that the change detector encountered an error checking a binding + * or calling a directive lifecycle method and is now in an inconsistent state. Change + * detectors in this state will no longer detect changes. + */ + Errored +} + export enum ChangeDetectionStrategy { /** * `CheckedOnce` means that after calling detectChanges the mode of the change detector @@ -51,6 +72,12 @@ export var CHANGE_DETECTION_STRATEGY_VALUES = [ ChangeDetectionStrategy.OnPushObserve ]; +export var CHANGE_DETECTOR_STATE_VALUES = [ + ChangeDetectorState.NeverChecked, + ChangeDetectorState.CheckedBefore, + ChangeDetectorState.Errored +]; + export function isDefaultChangeDetectionStrategy( changeDetectionStrategy: ChangeDetectionStrategy): boolean { return isBlank(changeDetectionStrategy) || diff --git a/modules/angular2/src/core/change_detection/dynamic_change_detector.ts b/modules/angular2/src/core/change_detection/dynamic_change_detector.ts index eb389cd09d748..efbca30e3121b 100644 --- a/modules/angular2/src/core/change_detection/dynamic_change_detector.ts +++ b/modules/angular2/src/core/change_detection/dynamic_change_detector.ts @@ -9,7 +9,7 @@ import {DirectiveRecord, DirectiveIndex} from './directive_record'; import {Locals} from './parser/locals'; import {ChangeDetectorGenConfig} from './interfaces'; import {ChangeDetectionUtil, SimpleChange} from './change_detection_util'; -import {ChangeDetectionStrategy} from './constants'; +import {ChangeDetectionStrategy, ChangeDetectorState} from './constants'; import {ProtoRecord, RecordType} from './proto_record'; export class DynamicChangeDetector extends AbstractChangeDetector { @@ -134,7 +134,8 @@ export class DynamicChangeDetector extends AbstractChangeDetector { if (proto.isLifeCycleRecord()) { if (proto.name === "DoCheck" && !throwOnChange) { this._getDirectiveFor(directiveRecord.directiveIndex).doCheck(); - } else if (proto.name === "OnInit" && !throwOnChange && !this.alreadyChecked) { + } else if (proto.name === "OnInit" && !throwOnChange && + this.state == ChangeDetectorState.NeverChecked) { this._getDirectiveFor(directiveRecord.directiveIndex).onInit(); } else if (proto.name === "OnChanges" && isPresent(changes) && !throwOnChange) { this._getDirectiveFor(directiveRecord.directiveIndex).onChanges(changes); @@ -170,7 +171,7 @@ export class DynamicChangeDetector extends AbstractChangeDetector { var dirs = this._directiveRecords; for (var i = dirs.length - 1; i >= 0; --i) { var dir = dirs[i]; - if (dir.callAfterContentInit && !this.alreadyChecked) { + if (dir.callAfterContentInit && this.state == ChangeDetectorState.NeverChecked) { this._getDirectiveFor(dir.directiveIndex).afterContentInit(); } @@ -184,7 +185,7 @@ export class DynamicChangeDetector extends AbstractChangeDetector { var dirs = this._directiveRecords; for (var i = dirs.length - 1; i >= 0; --i) { var dir = dirs[i]; - if (dir.callAfterViewInit && !this.alreadyChecked) { + if (dir.callAfterViewInit && this.state == ChangeDetectorState.NeverChecked) { this._getDirectiveFor(dir.directiveIndex).afterViewInit(); } if (dir.callAfterViewChecked) { diff --git a/modules/angular2/src/core/change_detection/jit_proto_change_detector.ts b/modules/angular2/src/core/change_detection/jit_proto_change_detector.ts index f06c992894a76..add231e80dc24 100644 --- a/modules/angular2/src/core/change_detection/jit_proto_change_detector.ts +++ b/modules/angular2/src/core/change_detection/jit_proto_change_detector.ts @@ -18,6 +18,8 @@ export class JitProtoChangeDetector implements ProtoChangeDetector { /** @internal */ _createFactory(definition: ChangeDetectorDefinition) { - return new ChangeDetectorJITGenerator(definition, 'util', 'AbstractChangeDetector').generate(); + return new ChangeDetectorJITGenerator(definition, 'util', 'AbstractChangeDetector', + 'ChangeDetectorStatus') + .generate(); } } diff --git a/modules/angular2/src/core/change_detection/pregen_proto_change_detector.dart b/modules/angular2/src/core/change_detection/pregen_proto_change_detector.dart index 3fe7ce10943c3..1963abed56d07 100644 --- a/modules/angular2/src/core/change_detection/pregen_proto_change_detector.dart +++ b/modules/angular2/src/core/change_detection/pregen_proto_change_detector.dart @@ -8,6 +8,8 @@ export 'package:angular2/src/core/change_detection/abstract_change_detector.dart show AbstractChangeDetector; export 'package:angular2/src/core/change_detection/change_detection.dart' show ChangeDetectionStrategy; +export 'package:angular2/src/core/change_detection/constants.dart' + show ChangeDetectorState; export 'package:angular2/src/core/change_detection/directive_record.dart' show DirectiveIndex, DirectiveRecord; export 'package:angular2/src/core/change_detection/interfaces.dart' diff --git a/modules/angular2/src/core/compiler/change_detector_compiler.ts b/modules/angular2/src/core/compiler/change_detector_compiler.ts index 5ca5d588699cd..177105a28d864 100644 --- a/modules/angular2/src/core/compiler/change_detector_compiler.ts +++ b/modules/angular2/src/core/compiler/change_detector_compiler.ts @@ -21,6 +21,7 @@ import {Injectable} from 'angular2/src/core/di'; const ABSTRACT_CHANGE_DETECTOR = "AbstractChangeDetector"; const UTIL = "ChangeDetectionUtil"; +const CHANGE_DETECTOR_STATE = "ChangeDetectorState"; var ABSTRACT_CHANGE_DETECTOR_MODULE = moduleRef( `package:angular2/src/core/change_detection/abstract_change_detector${MODULE_SUFFIX}`); @@ -28,6 +29,8 @@ var UTIL_MODULE = moduleRef(`package:angular2/src/core/change_detection/change_detection_util${MODULE_SUFFIX}`); var PREGEN_PROTO_CHANGE_DETECTOR_MODULE = moduleRef( `package:angular2/src/core/change_detection/pregen_proto_change_detector${MODULE_SUFFIX}`); +var CONSTANTS_MODULE = + moduleRef(`package:angular2/src/core/change_detection/constants${MODULE_SUFFIX}`); @Injectable() export class ChangeDetectionCompiler { @@ -46,7 +49,9 @@ export class ChangeDetectionCompiler { var proto = new DynamicProtoChangeDetector(definition); return (dispatcher) => proto.instantiate(dispatcher); } else { - return new ChangeDetectorJITGenerator(definition, UTIL, ABSTRACT_CHANGE_DETECTOR).generate(); + return new ChangeDetectorJITGenerator(definition, UTIL, ABSTRACT_CHANGE_DETECTOR, + CHANGE_DETECTOR_STATE) + .generate(); } } @@ -74,7 +79,8 @@ export class ChangeDetectionCompiler { } else { codegen = new ChangeDetectorJITGenerator( definition, `${UTIL_MODULE}${UTIL}`, - `${ABSTRACT_CHANGE_DETECTOR_MODULE}${ABSTRACT_CHANGE_DETECTOR}`); + `${ABSTRACT_CHANGE_DETECTOR_MODULE}${ABSTRACT_CHANGE_DETECTOR}`, + `${CONSTANTS_MODULE}${CHANGE_DETECTOR_STATE}`); factories.push(`function(dispatcher) { return new ${codegen.typeName}(dispatcher); }`); sourcePart = codegen.generateSource(); } diff --git a/modules/angular2/test/core/change_detection/change_detector_spec.ts b/modules/angular2/test/core/change_detection/change_detector_spec.ts index 19bdacbe48972..c72098c79dd53 100644 --- a/modules/angular2/test/core/change_detection/change_detector_spec.ts +++ b/modules/angular2/test/core/change_detection/change_detector_spec.ts @@ -431,10 +431,12 @@ export function main() { describe('updating directives', () => { var directive1; var directive2; + var directive3; beforeEach(() => { directive1 = new TestDirective(); directive2 = new TestDirective(); + directive3 = new TestDirective(null, null, true); }); it('should happen directly, without invoking the dispatcher', () => { @@ -510,6 +512,30 @@ export function main() { expect(directive1.onInitCalled).toBe(false); }); + + it('should not call onInit again if it throws', () => { + var cd = _createWithoutHydrate('directiveOnInit').changeDetector; + + cd.hydrate(_DEFAULT_CONTEXT, null, new FakeDirectives([directive3], []), null); + var errored = false; + // First pass fails, but onInit should be called. + try { + cd.detectChanges(); + } catch (e) { + errored = true; + } + expect(errored).toBe(true); + expect(directive3.onInitCalled).toBe(true); + directive3.onInitCalled = false; + + // Second change detection also fails, but this time onInit should not be called. + try { + cd.detectChanges(); + } catch (e) { + throw new BaseException("Second detectChanges() should not have run detection."); + } + expect(directive3.onInitCalled).toBe(false); + }); }); describe('afterContentInit', () => { @@ -1336,13 +1362,19 @@ class TestDirective { afterViewCheckedCalled = false; event; - constructor(public afterContentCheckedSpy = null, public afterViewCheckedSpy = null) {} + constructor(public afterContentCheckedSpy = null, public afterViewCheckedSpy = null, + public throwOnInit = false) {} onEvent(event) { this.event = event; } doCheck() { this.doCheckCalled = true; } - onInit() { this.onInitCalled = true; } + onInit() { + this.onInitCalled = true; + if (this.throwOnInit) { + throw "simulated onInit failure"; + } + } onChanges(changes) { var r = {}; diff --git a/modules_dart/transform/lib/src/transform/template_compiler/change_detector_codegen.dart b/modules_dart/transform/lib/src/transform/template_compiler/change_detector_codegen.dart index 4b78f4f268163..90424dd079752 100644 --- a/modules_dart/transform/lib/src/transform/template_compiler/change_detector_codegen.dart +++ b/modules_dart/transform/lib/src/transform/template_compiler/change_detector_codegen.dart @@ -121,7 +121,7 @@ class _CodegenState { var names = new CodegenNameUtil( protoRecords, eventBindings, def.directiveRecords, '$genPrefix$_UTIL'); - var logic = new CodegenLogicUtil(names, '$genPrefix$_UTIL', def.strategy); + var logic = new CodegenLogicUtil(names, '$genPrefix$_UTIL', '$genPrefix$_STATE', def.strategy); return new _CodegenState._( genPrefix, def.id, @@ -503,7 +503,7 @@ class _CodegenState { String _genOnInit(ProtoRecord r) { var br = r.bindingRecord; - return 'if (!throwOnChange && !${_names.getAlreadyCheckedName()}) ' + return 'if (!throwOnChange && ${_names.getStateName()} == ${_genPrefix}$_STATE.NeverChecked) ' '${_names.getDirectiveName(br.directiveRecord.directiveIndex)}.onInit();'; } @@ -539,3 +539,4 @@ const _GEN_PROPERTY_BINDING_TARGETS_NAME = '${_GEN_PREFIX}_propertyBindingTargets'; const _GEN_DIRECTIVE_INDICES_NAME = '${_GEN_PREFIX}_directiveIndices'; const _UTIL = 'ChangeDetectionUtil'; +const _STATE = 'ChangeDetectorState';