Skip to content

Commit b6e95bb

Browse files
author
Tim Blasi
committed
feat(change detect): Throw on attempts to use dehydrated detector
- 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.
1 parent cd95e07 commit b6e95bb

File tree

10 files changed

+78
-33
lines changed

10 files changed

+78
-33
lines changed

‎modules/angular2/change_detection.ts‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,7 @@ export {Parser} from './src/change_detection/parser/parser';
1919
export {Locals} from './src/change_detection/parser/locals';
2020

2121
export {
22+
DehydratedException,
2223
ExpressionChangedAfterItHasBeenChecked,
2324
ChangeDetectionError
2425
} from './src/change_detection/exceptions';

‎modules/angular2/src/change_detection/change_detection_jit_generator.dart‎

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,11 @@
11
library change_detectoin.change_detection_jit_generator;
22

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

‎modules/angular2/src/change_detection/change_detection_jit_generator.ts‎

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -98,6 +98,9 @@ export class ChangeDetectorJITGenerator {
9898
${this.typeName}.prototype = Object.create(${ABSTRACT_CHANGE_DETECTOR}.prototype);
9999
100100
${this.typeName}.prototype.detectChangesInRecords = function(throwOnChange) {
101+
if (!this.hydrated()) {
102+
${UTIL}.throwDehydrated();
103+
}
101104
${this._genLocalDefinitions()}
102105
${this._genChangeDefinitions()}
103106
var ${IS_CHANGED_LOCAL} = false;
@@ -131,7 +134,7 @@ export class ChangeDetectorJITGenerator {
131134
}
132135
133136
${this.typeName}.prototype.hydrated = function() {
134-
return Boolean(${CONTEXT_ACCESSOR});
137+
return ${CONTEXT_ACCESSOR} !== null;
135138
}
136139
137140
return function(dispatcher, pipeRegistry) {

‎modules/angular2/src/change_detection/change_detection_util.ts‎

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,7 @@
11
import {isPresent, isBlank, BaseException, Type} from 'angular2/src/facade/lang';
22
import {List, ListWrapper, MapWrapper, StringMapWrapper} from 'angular2/src/facade/collection';
33
import {ProtoRecord} from './proto_record';
4-
import {ExpressionChangedAfterItHasBeenChecked} from './exceptions';
4+
import {DehydratedException, ExpressionChangedAfterItHasBeenChecked} from './exceptions';
55
import {WrappedValue} from './pipes/pipe';
66
import {CHECK_ALWAYS, CHECK_ONCE, CHECKED, DETACHED, ON_PUSH} from './constants';
77

@@ -127,6 +127,8 @@ export class ChangeDetectionUtil {
127127
throw new ExpressionChangedAfterItHasBeenChecked(proto, change);
128128
}
129129

130+
static throwDehydrated() { throw new DehydratedException(); }
131+
130132
static changeDetectionMode(strategy: string) {
131133
return strategy == ON_PUSH ? CHECK_ONCE : CHECK_ALWAYS;
132134
}

‎modules/angular2/src/change_detection/dynamic_change_detector.ts‎

Lines changed: 8 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -44,7 +44,8 @@ export class DynamicChangeDetector extends AbstractChangeDetector {
4444
this.prevContexts = ListWrapper.createFixedSize(protos.length + 1);
4545
this.changes = ListWrapper.createFixedSize(protos.length + 1);
4646

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

6162
dehydrate() {
6263
this._destroyPipes();
63-
ListWrapper.fill(this.values, uninitialized);
64+
this.values[0] = null;
65+
ListWrapper.fill(this.values, uninitialized, 1);
6466
ListWrapper.fill(this.changes, false);
6567
ListWrapper.fill(this.pipes, null);
6668
ListWrapper.fill(this.prevContexts, uninitialized);
@@ -75,9 +77,12 @@ export class DynamicChangeDetector extends AbstractChangeDetector {
7577
}
7678
}
7779

78-
hydrated(): boolean { return this.values[0] !== uninitialized; }
80+
hydrated(): boolean { return this.values[0] !== null; }
7981

8082
detectChangesInRecords(throwOnChange: boolean) {
83+
if (!this.hydrated()) {
84+
ChangeDetectionUtil.throwDehydrated();
85+
}
8186
var protos: List<ProtoRecord> = this.protos;
8287

8388
var changes = null;

‎modules/angular2/src/change_detection/exceptions.ts‎

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -27,4 +27,8 @@ export class ChangeDetectionError extends BaseException {
2727
}
2828

2929
toString(): string { return this.message; }
30-
}
30+
}
31+
32+
export class DehydratedException extends BaseException {
33+
constructor() { super('Attempt to detect changes on a dehydrated detector.'); }
34+
}

‎modules/angular2/src/transform/template_compiler/change_detector_codegen.dart‎

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -113,6 +113,9 @@ class _CodegenState {
113113
this.$_DIRECTIVES_ACCESSOR) : super();
114114
115115
void detectChangesInRecords(throwOnChange) {
116+
if (!hydrated()) {
117+
$_UTIL.throwDehydrated();
118+
}
116119
${_genLocalDefinitions()}
117120
${_genChangeDefinitons()}
118121
var $_IS_CHANGED_LOCAL = false;
@@ -145,7 +148,7 @@ class _CodegenState {
145148
$_LOCALS_ACCESSOR = null;
146149
}
147150
148-
hydrated() => $_CONTEXT_ACCESSOR == null;
151+
hydrated() => $_CONTEXT_ACCESSOR != null;
149152
150153
static $_GEN_PREFIX.ProtoChangeDetector
151154
$PROTO_CHANGE_DETECTOR_FACTORY_METHOD(

‎modules/angular2/test/change_detection/change_detector_spec.ts‎

Lines changed: 42 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,7 @@ import {
1111
} from 'angular2/test_lib';
1212

1313
import {
14+
CONST_EXPR,
1415
isPresent,
1516
isBlank,
1617
isJsObject,
@@ -21,6 +22,7 @@ import {List, ListWrapper, MapWrapper, StringMapWrapper} from 'angular2/src/faca
2122

2223
import {
2324
ChangeDispatcher,
25+
DehydratedException,
2426
DynamicChangeDetector,
2527
ChangeDetectionError,
2628
BindingRecord,
@@ -47,6 +49,7 @@ import {
4749
import {getDefinition} from './simple_watch_config';
4850
import {getFactoryById} from './generated/simple_watch_classes';
4951

52+
const _DEFAULT_CONTEXT = CONST_EXPR(new Object());
5053

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

75-
function _bindSimpleValue(expression: string, context = null) {
78+
function _bindSimpleValue(expression: string, context = _DEFAULT_CONTEXT) {
7679
var dispatcher = new TestDispatcher();
7780
var testDef = getDefinition(expression);
7881
var protoCd = _getProtoChangeDetector(testDef.cdDef);
@@ -297,7 +300,7 @@ export function main() {
297300

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

300-
function createChangeDetector(propName: string, exp: string, context = null,
303+
function createChangeDetector(propName: string, exp: string, context = _DEFAULT_CONTEXT,
301304
registry = null) {
302305
var dispatcher = new TestDispatcher();
303306

@@ -444,7 +447,7 @@ export function main() {
444447

445448
var cd = pcd.instantiate(dispatcher);
446449

447-
cd.hydrate(null, null, dirs([directive1]));
450+
cd.hydrate(_DEFAULT_CONTEXT, null, dirs([directive1]));
448451

449452
cd.detectChanges();
450453

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

466469
var cd = pcd.instantiate(dispatcher);
467470

468-
cd.hydrate(null, null, dirs([directive1, directive2]));
471+
cd.hydrate(_DEFAULT_CONTEXT, null, dirs([directive1, directive2]));
469472

470473
cd.detectChanges();
471474

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

483486
var cd = pcd.instantiate(dispatcher);
484487

485-
cd.hydrate(null, null, dirs([directive1]));
488+
cd.hydrate(_DEFAULT_CONTEXT, null, dirs([directive1]));
486489

487490
cd.detectChanges();
488491

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

503506
var cd = pcd.instantiate(dispatcher);
504507

505-
cd.hydrate(null, null, dirs([directive1]));
508+
cd.hydrate(_DEFAULT_CONTEXT, null, dirs([directive1]));
506509

507510
cd.checkNoChanges();
508511

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

519522
var cd = pcd.instantiate(dispatcher);
520523

521-
cd.hydrate(null, null, dirs([directive1]));
524+
cd.hydrate(_DEFAULT_CONTEXT, null, dirs([directive1]));
522525

523526
cd.detectChanges();
524527

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

539542
var cd = pcd.instantiate(dispatcher);
540543

541-
cd.hydrate(null, null, dirs([directive1]));
544+
cd.hydrate(_DEFAULT_CONTEXT, null, dirs([directive1]));
542545

543546
cd.checkNoChanges();
544547

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

553556
var cd = pcd.instantiate(dispatcher);
554-
cd.hydrate(null, null, dirs([directive1, directive2]));
557+
cd.hydrate(_DEFAULT_CONTEXT, null, dirs([directive1, directive2]));
555558

556559
cd.detectChanges();
557560

@@ -582,7 +585,7 @@ export function main() {
582585

583586
var cd = pcd.instantiate(dispatcher);
584587

585-
cd.hydrate(null, null, dirs([directive1]));
588+
cd.hydrate(_DEFAULT_CONTEXT, null, dirs([directive1]));
586589

587590
cd.detectChanges();
588591

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

604607
cd.detectChanges();
605608

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

623-
parent.hydrate(null, null, dirs([parentDirective]));
624-
child.hydrate(null, null, dirs([directiveInShadowDom]));
626+
parent.hydrate(_DEFAULT_CONTEXT, null, dirs([parentDirective]));
627+
child.hydrate(_DEFAULT_CONTEXT, null, dirs([directiveInShadowDom]));
625628

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

646649
cd.detectChanges();
647650

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

693696
try {
694697
cd.detectChanges();
@@ -747,15 +750,15 @@ export function main() {
747750

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

750-
cd.hydrate(null, null, null);
753+
cd.hydrate(_DEFAULT_CONTEXT, null, null);
751754

752755
expect(cd.mode).toEqual(CHECK_ALWAYS);
753756
});
754757

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

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

771+
cd.hydrate(_DEFAULT_CONTEXT, null, null);
768772
cd.mode = DETACHED;
769773
cd.detectChanges();
770774

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

783+
cd.hydrate(_DEFAULT_CONTEXT, null, null);
779784
cd.mode = CHECKED;
780785
cd.detectChanges();
781786

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

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

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

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

798805
cd.detectChanges();
@@ -809,7 +816,7 @@ export function main() {
809816
beforeEach(() => {
810817
var proto = createProtoChangeDetector([], [], [], null, ON_PUSH);
811818
checkedDetector = proto.instantiate(null);
812-
checkedDetector.hydrate(null, null, null);
819+
checkedDetector.hydrate(_DEFAULT_CONTEXT, null, null);
813820
checkedDetector.mode = CHECKED;
814821

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

831838
var cd = proto.instantiate(null);
832-
cd.hydrate(null, null, directives);
839+
cd.hydrate(_DEFAULT_CONTEXT, null, directives);
833840

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

@@ -898,6 +905,21 @@ export function main() {
898905

899906
expect(pipe.destroyCalled).toBe(true);
900907
});
908+
909+
it("should throw when detectChanges is called on a dehydrated detector", () => {
910+
var context = new Person('Bob');
911+
var c = createChangeDetector("propName", "name", context);
912+
var cd = c["changeDetector"];
913+
var log = c["dispatcher"].log;
914+
915+
cd.detectChanges();
916+
expect(log).toEqual(["propName=Bob"]);
917+
918+
cd.dehydrate();
919+
var dehydratedException = new DehydratedException();
920+
expect(() => {cd.detectChanges()}).toThrowError(dehydratedException.toString());
921+
expect(log).toEqual(["propName=Bob"]);
922+
});
901923
});
902924

903925
describe("pipes", () => {
@@ -1098,13 +1120,8 @@ class TestDirective {
10981120
}
10991121

11001122
class Person {
1101-
name: string;
11021123
age: number;
1103-
address: Address;
1104-
constructor(name: string, address: Address = null) {
1105-
this.name = name;
1106-
this.address = address;
1107-
}
1124+
constructor(public name: string, public address: Address = null) {}
11081125

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

‎modules/angular2/test/transform/integration/two_annotations_files/expected/bar.ng_deps.dart‎

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -34,6 +34,9 @@ class _MyComponent_ChangeDetector0 extends _gen.AbstractChangeDetector {
3434
: super();
3535

3636
void detectChangesInRecords(throwOnChange) {
37+
if (!hydrated()) {
38+
_gen.ChangeDetectionUtil.throwDehydrated();
39+
}
3740
var context = null;
3841
var change_context = false;
3942
var isChanged = false;
@@ -56,7 +59,7 @@ class _MyComponent_ChangeDetector0 extends _gen.AbstractChangeDetector {
5659
_locals = null;
5760
}
5861

59-
hydrated() => _context == null;
62+
hydrated() => _context != null;
6063

6164
static _gen.ProtoChangeDetector newProtoChangeDetector(
6265
_gen.PipeRegistry registry, _gen.ChangeDetectorDefinition def) {

0 commit comments

Comments
 (0)