Skip to content

Commit

Permalink
feat(change detect): Throw on attempts to use dehydrated detector
Browse files Browse the repository at this point in the history 
- Modify change detectors to `throw` when attempting to detect changes
  on a dehydrated detector.
- Modify `DynamicChagneDetector` to use `null` for the `context` of a
  dehydrated detector.
  • Loading branch information
Tim Blasi committed Jun 8, 2015
1 parent cd95e07 commit b6e95bb
Show file tree
Hide file tree
Showing 10 changed files with 78 additions and 33 deletions.
1 change: 1 addition & 0 deletions modules/angular2/change_detection.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ export {Parser} from './src/change_detection/parser/parser';
export {Locals} from './src/change_detection/parser/locals';

export {
DehydratedException,
ExpressionChangedAfterItHasBeenChecked,
ChangeDetectionError
} from './src/change_detection/exceptions';
Expand Down
6 changes: 6 additions & 0 deletions modules/angular2/src/change_detection/change_detection_jit_generator.dart
View file
Original file line number Diff line number Diff line change
@@ -1,5 +1,11 @@
library change_detectoin.change_detection_jit_generator;

/// Placeholder JIT generator for Dart.
/// Dart does not support `eval`, so JIT generation is not an option. Instead,
/// the Dart transformer pre-generates these Change Detector classes and
/// registers them with the system. See `PreGeneratedChangeDetection`,
/// `PregenProtoChangeDetector`, and
/// `src/transform/template_compiler/change_detector_codegen.dart` for details.
class ChangeDetectorJITGenerator {
ChangeDetectorJITGenerator(typeName, strategy, records, directiveMementos) {}

Expand Down
5 changes: 4 additions & 1 deletion modules/angular2/src/change_detection/change_detection_jit_generator.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -98,6 +98,9 @@ export class ChangeDetectorJITGenerator {
${this.typeName}.prototype = Object.create(${ABSTRACT_CHANGE_DETECTOR}.prototype);
${this.typeName}.prototype.detectChangesInRecords = function(throwOnChange) {
if (!this.hydrated()) {
${UTIL}.throwDehydrated();
}
${this._genLocalDefinitions()}
${this._genChangeDefinitions()}
var ${IS_CHANGED_LOCAL} = false;
Expand Down Expand Up @@ -131,7 +134,7 @@ export class ChangeDetectorJITGenerator {
}
${this.typeName}.prototype.hydrated = function() {
return Boolean(${CONTEXT_ACCESSOR});
return ${CONTEXT_ACCESSOR} !== null;
}
return function(dispatcher, pipeRegistry) {
Expand Down
4 changes: 3 additions & 1 deletion modules/angular2/src/change_detection/change_detection_util.ts
View file
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
import {isPresent, isBlank, BaseException, Type} from 'angular2/src/facade/lang';
import {List, ListWrapper, MapWrapper, StringMapWrapper} from 'angular2/src/facade/collection';
import {ProtoRecord} from './proto_record';
import {ExpressionChangedAfterItHasBeenChecked} from './exceptions';
import {DehydratedException, ExpressionChangedAfterItHasBeenChecked} from './exceptions';
import {WrappedValue} from './pipes/pipe';
import {CHECK_ALWAYS, CHECK_ONCE, CHECKED, DETACHED, ON_PUSH} from './constants';

Expand Down Expand Up @@ -127,6 +127,8 @@ export class ChangeDetectionUtil {
throw new ExpressionChangedAfterItHasBeenChecked(proto, change);
}

static throwDehydrated() { throw new DehydratedException(); }

static changeDetectionMode(strategy: string) {
return strategy == ON_PUSH ? CHECK_ONCE : CHECK_ALWAYS;
}
Expand Down
11 changes: 8 additions & 3 deletions modules/angular2/src/change_detection/dynamic_change_detector.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,8 @@ export class DynamicChangeDetector extends AbstractChangeDetector {
this.prevContexts = ListWrapper.createFixedSize(protos.length + 1);
this.changes = ListWrapper.createFixedSize(protos.length + 1);

ListWrapper.fill(this.values, uninitialized);
this.values[0] = null;
ListWrapper.fill(this.values, uninitialized, 1);
ListWrapper.fill(this.pipes, null);
ListWrapper.fill(this.prevContexts, uninitialized);
ListWrapper.fill(this.changes, false);
Expand All @@ -60,7 +61,8 @@ export class DynamicChangeDetector extends AbstractChangeDetector {

dehydrate() {
this._destroyPipes();
ListWrapper.fill(this.values, uninitialized);
this.values[0] = null;
ListWrapper.fill(this.values, uninitialized, 1);
ListWrapper.fill(this.changes, false);
ListWrapper.fill(this.pipes, null);
ListWrapper.fill(this.prevContexts, uninitialized);
Expand All @@ -75,9 +77,12 @@ export class DynamicChangeDetector extends AbstractChangeDetector {
}
}

hydrated(): boolean { return this.values[0] !== uninitialized; }
hydrated(): boolean { return this.values[0] !== null; }

detectChangesInRecords(throwOnChange: boolean) {
if (!this.hydrated()) {
ChangeDetectionUtil.throwDehydrated();
}
var protos: List<ProtoRecord> = this.protos;

var changes = null;
Expand Down
6 changes: 5 additions & 1 deletion modules/angular2/src/change_detection/exceptions.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -27,4 +27,8 @@ export class ChangeDetectionError extends BaseException {
}

toString(): string { return this.message; }
}
}

export class DehydratedException extends BaseException {
constructor() { super('Attempt to detect changes on a dehydrated detector.'); }
}
5 changes: 4 additions & 1 deletion modules/angular2/src/transform/template_compiler/change_detector_codegen.dart
View file
Original file line number Diff line number Diff line change
Expand Up @@ -113,6 +113,9 @@ class _CodegenState {
this.$_DIRECTIVES_ACCESSOR) : super();
void detectChangesInRecords(throwOnChange) {
if (!hydrated()) {
$_UTIL.throwDehydrated();
}
${_genLocalDefinitions()}
${_genChangeDefinitons()}
var $_IS_CHANGED_LOCAL = false;
Expand Down Expand Up @@ -145,7 +148,7 @@ class _CodegenState {
$_LOCALS_ACCESSOR = null;
}
hydrated() => $_CONTEXT_ACCESSOR == null;
hydrated() => $_CONTEXT_ACCESSOR != null;
static $_GEN_PREFIX.ProtoChangeDetector
$PROTO_CHANGE_DETECTOR_FACTORY_METHOD(
Expand Down
67 changes: 42 additions & 25 deletions modules/angular2/test/change_detection/change_detector_spec.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ import {
} from 'angular2/test_lib';

import {
CONST_EXPR,
isPresent,
isBlank,
isJsObject,
Expand All @@ -21,6 +22,7 @@ import {List, ListWrapper, MapWrapper, StringMapWrapper} from 'angular2/src/faca

import {
ChangeDispatcher,
DehydratedException,
DynamicChangeDetector,
ChangeDetectionError,
BindingRecord,
Expand All @@ -47,6 +49,7 @@ import {
import {getDefinition} from './simple_watch_config';
import {getFactoryById} from './generated/simple_watch_classes';

const _DEFAULT_CONTEXT = CONST_EXPR(new Object());

export function main() {
// These tests also run against pre-generated Dart Change Detectors. We will move tests up from
Expand All @@ -72,7 +75,7 @@ export function main() {
}
}

function _bindSimpleValue(expression: string, context = null) {
function _bindSimpleValue(expression: string, context = _DEFAULT_CONTEXT) {
var dispatcher = new TestDispatcher();
var testDef = getDefinition(expression);
var protoCd = _getProtoChangeDetector(testDef.cdDef);
Expand Down Expand Up @@ -297,7 +300,7 @@ export function main() {

function dirs(directives: List<any>) { return new FakeDirectives(directives, []); }

function createChangeDetector(propName: string, exp: string, context = null,
function createChangeDetector(propName: string, exp: string, context = _DEFAULT_CONTEXT,
registry = null) {
var dispatcher = new TestDispatcher();

Expand Down Expand Up @@ -444,7 +447,7 @@ export function main() {

var cd = pcd.instantiate(dispatcher);

cd.hydrate(null, null, dirs([directive1]));
cd.hydrate(_DEFAULT_CONTEXT, null, dirs([directive1]));

cd.detectChanges();

Expand All @@ -465,7 +468,7 @@ export function main() {

var cd = pcd.instantiate(dispatcher);

cd.hydrate(null, null, dirs([directive1, directive2]));
cd.hydrate(_DEFAULT_CONTEXT, null, dirs([directive1, directive2]));

cd.detectChanges();

Expand All @@ -482,7 +485,7 @@ export function main() {

var cd = pcd.instantiate(dispatcher);

cd.hydrate(null, null, dirs([directive1]));
cd.hydrate(_DEFAULT_CONTEXT, null, dirs([directive1]));

cd.detectChanges();

Expand All @@ -502,7 +505,7 @@ export function main() {

var cd = pcd.instantiate(dispatcher);

cd.hydrate(null, null, dirs([directive1]));
cd.hydrate(_DEFAULT_CONTEXT, null, dirs([directive1]));

cd.checkNoChanges();

Expand All @@ -518,7 +521,7 @@ export function main() {

var cd = pcd.instantiate(dispatcher);

cd.hydrate(null, null, dirs([directive1]));
cd.hydrate(_DEFAULT_CONTEXT, null, dirs([directive1]));

cd.detectChanges();

Expand All @@ -538,7 +541,7 @@ export function main() {

var cd = pcd.instantiate(dispatcher);

cd.hydrate(null, null, dirs([directive1]));
cd.hydrate(_DEFAULT_CONTEXT, null, dirs([directive1]));

cd.checkNoChanges();

Expand All @@ -551,7 +554,7 @@ export function main() {
var pcd = createProtoChangeDetector([], [], [dirRecord1, dirRecord2]);

var cd = pcd.instantiate(dispatcher);
cd.hydrate(null, null, dirs([directive1, directive2]));
cd.hydrate(_DEFAULT_CONTEXT, null, dirs([directive1, directive2]));

cd.detectChanges();

Expand Down Expand Up @@ -582,7 +585,7 @@ export function main() {

var cd = pcd.instantiate(dispatcher);

cd.hydrate(null, null, dirs([directive1]));
cd.hydrate(_DEFAULT_CONTEXT, null, dirs([directive1]));

cd.detectChanges();

Expand All @@ -599,7 +602,7 @@ export function main() {
td1 = new TestDirective(() => ListWrapper.push(onChangesDoneCalls, td1));
var td2;
td2 = new TestDirective(() => ListWrapper.push(onChangesDoneCalls, td2));
cd.hydrate(null, null, dirs([td1, td2]));
cd.hydrate(_DEFAULT_CONTEXT, null, dirs([td1, td2]));

cd.detectChanges();

Expand All @@ -620,8 +623,8 @@ export function main() {
var parentDirective =
new TestDirective(() => { expect(directiveInShadowDom.a).toBe(null); });

parent.hydrate(null, null, dirs([parentDirective]));
child.hydrate(null, null, dirs([directiveInShadowDom]));
parent.hydrate(_DEFAULT_CONTEXT, null, dirs([parentDirective]));
child.hydrate(_DEFAULT_CONTEXT, null, dirs([directiveInShadowDom]));

parent.detectChanges();
});
Expand All @@ -641,7 +644,7 @@ export function main() {
[BindingRecord.createForHostProperty(index, ast("a"), "prop")], null,
[dirRecord]);
var cd = pcd.instantiate(dispatcher);
cd.hydrate(null, null, dirs([directive]));
cd.hydrate(_DEFAULT_CONTEXT, null, dirs([directive]));

cd.detectChanges();

Expand Down Expand Up @@ -688,7 +691,7 @@ export function main() {
var cd = pcd.instantiate(
new TestDispatcher(),
[BindingRecord.createForElement(ast("invalidProp"), 0, "a")], null, []);
cd.hydrate(null, null);
cd.hydrate(_DEFAULT_CONTEXT, null);

try {
cd.detectChanges();
Expand Down Expand Up @@ -747,15 +750,15 @@ export function main() {

expect(cd.mode).toEqual(null);

cd.hydrate(null, null, null);
cd.hydrate(_DEFAULT_CONTEXT, null, null);

expect(cd.mode).toEqual(CHECK_ALWAYS);
});

it("should set the mode to CHECK_ONCE when the push change detection is used", () => {
var proto = createProtoChangeDetector([], [], [], null, ON_PUSH);
var cd = proto.instantiate(null);
cd.hydrate(null, null, null);
cd.hydrate(_DEFAULT_CONTEXT, null, null);

expect(cd.mode).toEqual(CHECK_ONCE);
});
Expand All @@ -765,6 +768,7 @@ export function main() {
var cd = c["changeDetector"];
var dispatcher = c["dispatcher"];

cd.hydrate(_DEFAULT_CONTEXT, null, null);
cd.mode = DETACHED;
cd.detectChanges();

Expand All @@ -776,6 +780,7 @@ export function main() {
var cd = c["changeDetector"];
var dispatcher = c["dispatcher"];

cd.hydrate(_DEFAULT_CONTEXT, null, null);
cd.mode = CHECKED;
cd.detectChanges();

Expand All @@ -784,6 +789,7 @@ export function main() {

it("should change CHECK_ONCE to CHECKED", () => {
var cd = createProtoChangeDetector([]).instantiate(null);
cd.hydrate(_DEFAULT_CONTEXT, null, null);
cd.mode = CHECK_ONCE;

cd.detectChanges();
Expand All @@ -793,6 +799,7 @@ export function main() {

it("should not change the CHECK_ALWAYS", () => {
var cd = createProtoChangeDetector([]).instantiate(null);
cd.hydrate(_DEFAULT_CONTEXT, null, null);
cd.mode = CHECK_ALWAYS;

cd.detectChanges();
Expand All @@ -809,7 +816,7 @@ export function main() {
beforeEach(() => {
var proto = createProtoChangeDetector([], [], [], null, ON_PUSH);
checkedDetector = proto.instantiate(null);
checkedDetector.hydrate(null, null, null);
checkedDetector.hydrate(_DEFAULT_CONTEXT, null, null);
checkedDetector.mode = CHECKED;

// this directive is a component with ON_PUSH change detection
Expand All @@ -829,7 +836,7 @@ export function main() {
[dirRecordWithOnPush]);

var cd = proto.instantiate(null);
cd.hydrate(null, null, directives);
cd.hydrate(_DEFAULT_CONTEXT, null, directives);

expect(checkedDetector.mode).toEqual(CHECKED);

Expand Down Expand Up @@ -898,6 +905,21 @@ export function main() {

expect(pipe.destroyCalled).toBe(true);
});

it("should throw when detectChanges is called on a dehydrated detector", () => {
var context = new Person('Bob');
var c = createChangeDetector("propName", "name", context);
var cd = c["changeDetector"];
var log = c["dispatcher"].log;

cd.detectChanges();
expect(log).toEqual(["propName=Bob"]);

cd.dehydrate();
var dehydratedException = new DehydratedException();
expect(() => {cd.detectChanges()}).toThrowError(dehydratedException.toString());
expect(log).toEqual(["propName=Bob"]);
});
});

describe("pipes", () => {
Expand Down Expand Up @@ -1098,13 +1120,8 @@ class TestDirective {
}

class Person {
name: string;
age: number;
address: Address;
constructor(name: string, address: Address = null) {
this.name = name;
this.address = address;
}
constructor(public name: string, public address: Address = null) {}

sayHi(m) { return `Hi, ${m}`; }

Expand Down
5 changes: 4 additions & 1 deletion modules/angular2/test/transform/integration/two_annotations_files/expected/bar.ng_deps.dart
View file
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,9 @@ class _MyComponent_ChangeDetector0 extends _gen.AbstractChangeDetector {
: super();

void detectChangesInRecords(throwOnChange) {
if (!hydrated()) {
_gen.ChangeDetectionUtil.throwDehydrated();
}
var context = null;
var change_context = false;
var isChanged = false;
Expand All @@ -56,7 +59,7 @@ class _MyComponent_ChangeDetector0 extends _gen.AbstractChangeDetector {
_locals = null;
}

hydrated() => _context == null;
hydrated() => _context != null;

static _gen.ProtoChangeDetector newProtoChangeDetector(
_gen.PipeRegistry registry, _gen.ChangeDetectorDefinition def) {
Expand Down
Loading

0 comments on commit b6e95bb

Please sign in to comment.