cargdev/angular
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

fix(core): Add an error state for ChangeDetectors that is set when bindings or lifecycle events throw exceptions and prevents further detection.

Browse Source 
- Changes the `alreadyChecked` flag of AbstractChangeDetector to a new `state` flag.
- Changes all checks of alreadyChecked to check that the state is NeverChecked.
- Set state to Errored if an error is thrown during detection.
- Skip change detection for a detector and its children when the state is Errored.
- Add a test to validate this fixes issue #4323.

Closes #4953
This commit is contained in:
Alex Rickabaugh
2015-10-27 10:55:01 -07:00
parent c930a533d1
commit d1b54d6807
12 changed files with 110 additions and 27 deletions
Download Patch File Download Diff File Expand all files Collapse all files

14
modules/angular2/src/core/change_detection/abstract_change_detector.ts
View File

@ -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<T> 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<T> 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<T> 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<T> 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<T> 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.

2
modules/angular2/src/core/change_detection/change_detection_jit_generator.dart
View File

@ -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";

15
modules/angular2/src/core/change_detection/change_detection_jit_generator.ts
View File

@ -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 */

8
modules/angular2/src/core/change_detection/codegen_logic_util.ts
View File

@ -6,12 +6,14 @@ import {BindingTarget} from './binding_record';
import {DirectiveRecord} from './directive_record';
import {ChangeDetectionStrategy} from './constants';
import {BaseException} from 'angular2/src/core/facade/exceptions';
import {IS_DART} from 'angular2/src/core/compiler/util';
/**
* Class responsible for providing change detection logic for change detector classes.
*/
export class CodegenLogicUtil {
constructor(private _names: CodegenNameUtil, private _utilName: string,
private _changeDetectorStateName: string,
private _changeDetection: ChangeDetectionStrategy) {}
/**
@ -182,12 +184,13 @@ export class CodegenLogicUtil {
genContentLifecycleCallbacks(directiveRecords: DirectiveRecord[]): string[] {
var res = [];
var eq = IS_DART ? '==' : '===';
// NOTE(kegluneq): Order is important!
for (var i = directiveRecords.length - 1; i >= 0; --i) {
var dir = directiveRecords[i];
if (dir.callAfterContentInit) {
res.push(
`if(! ${this._names.getAlreadyCheckedName()}) ${this._names.getDirectiveName(dir.directiveIndex)}.afterContentInit();`);
`if(${this._names.getStateName()} ${eq} ${this._changeDetectorStateName}.NeverChecked) ${this._names.getDirectiveName(dir.directiveIndex)}.afterContentInit();`);
}
if (dir.callAfterContentChecked) {
@ -199,12 +202,13 @@ export class CodegenLogicUtil {
genViewLifecycleCallbacks(directiveRecords: DirectiveRecord[]): string[] {
var res = [];
var eq = IS_DART ? '==' : '===';
// NOTE(kegluneq): Order is important!
for (var i = directiveRecords.length - 1; i >= 0; --i) {
var dir = directiveRecords[i];
if (dir.callAfterViewInit) {
res.push(
`if(! ${this._names.getAlreadyCheckedName()}) ${this._names.getDirectiveName(dir.directiveIndex)}.afterViewInit();`);
`if(${this._names.getStateName()} ${eq} ${this._changeDetectorStateName}.NeverChecked) ${this._names.getDirectiveName(dir.directiveIndex)}.afterViewInit();`);
}
if (dir.callAfterViewChecked) {

5
modules/angular2/src/core/change_detection/codegen_name_util.ts
View File

@ -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); }

27
modules/angular2/src/core/change_detection/constants.ts
View File

@ -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) ||

9
modules/angular2/src/core/change_detection/dynamic_change_detector.ts
View File

@ -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<any> {
@ -134,7 +134,8 @@ export class DynamicChangeDetector extends AbstractChangeDetector<any> {
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<any> {
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<any> {
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) {

4
modules/angular2/src/core/change_detection/jit_proto_change_detector.ts
View File

@ -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();
}
}

2
modules/angular2/src/core/change_detection/pregen_proto_change_detector.dart
View File

@ -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'

10
modules/angular2/src/core/compiler/change_detector_compiler.ts
View File

@ -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();
}