Skip to content

Commit 68faddb

Browse files
vsavkinvsavkin
committed
feat(change_detection): updated handling ON_PUSH detectors so they get notified when their bindings change
1 parent dc9c614 commit 68faddb

File tree

13 files changed

+249
-60
lines changed

13 files changed

+249
-60
lines changed

‎modules/angular2/src/change_detection/abstract_change_detector.js‎

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -9,13 +9,13 @@ export class AbstractChangeDetector extends ChangeDetector {
99
shadowDomChildren:List;
1010
parent:ChangeDetector;
1111
mode:string;
12-
changeDetectorRef:ChangeDetectorRef;
12+
ref:ChangeDetectorRef;
1313

1414
constructor() {
1515
super();
1616
this.lightDomChildren = [];
1717
this.shadowDomChildren = [];
18-
this.changeDetectorRef = new ChangeDetectorRef(this);
18+
this.ref = new ChangeDetectorRef(this);
1919
this.mode = null;
2020
}
2121

@@ -80,6 +80,10 @@ export class AbstractChangeDetector extends ChangeDetector {
8080
}
8181
}
8282

83+
markAsCheckOnce() {
84+
this.mode = CHECK_ONCE;
85+
}
86+
8387
markPathToRootAsCheckOnce() {
8488
var c = this;
8589
while(isPresent(c) && c.mode != DETACHED) {

‎modules/angular2/src/change_detection/binding_record.js‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -32,6 +32,10 @@ export class BindingRecord {
3232
return isPresent(this.directiveRecord) && this.directiveRecord.callOnChange;
3333
}
3434

35+
isOnPushChangeDetection() {
36+
return isPresent(this.directiveRecord) && this.directiveRecord.isOnPushChangeDetection();
37+
}
38+
3539
isDirective() {
3640
return this.mode === DIRECTIVE;
3741
}

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

Lines changed: 56 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -36,7 +36,7 @@ var PIPE_REGISTRY_ACCESSOR = "this.pipeRegistry";
3636
var PROTOS_ACCESSOR = "this.protos";
3737
var DIRECTIVES_ACCESSOR = "this.directiveRecords";
3838
var CONTEXT_ACCESSOR = "this.context";
39-
var CHANGE_LOCAL = "change";
39+
var IS_CHANGED_LOCAL = "isChanged";
4040
var CHANGES_LOCAL = "changes";
4141
var LOCALS_ACCESSOR = "this.locals";
4242
var MODE_ACCESSOR = "this.mode";
@@ -78,10 +78,15 @@ function pipeOnDestroyTemplate(pipeNames:List) {
7878
}
7979

8080
function hydrateTemplate(type:string, mode:string, fieldDefinitions:string, pipeOnDestroy:string,
81-
directiveFieldNames:List<String>):string {
81+
directiveFieldNames:List<String>, detectorFieldNames:List<String>):string {
8282
var directiveInit = "";
8383
for(var i = 0; i < directiveFieldNames.length; ++i) {
84-
directiveInit += `${directiveFieldNames[i]} = directives.directive(this.directiveRecords[${i}]);\n`;
84+
directiveInit += `${directiveFieldNames[i]} = directives.getDirectiveFor(this.directiveRecords[${i}]);\n`;
85+
}
86+
87+
var detectorInit = "";
88+
for(var i = 0; i < detectorFieldNames.length; ++i) {
89+
detectorInit += `${detectorFieldNames[i]} = directives.getDetectorFor(this.directiveRecords[${i}]);\n`;
8590
}
8691

8792
return `
@@ -90,6 +95,7 @@ ${type}.prototype.hydrate = function(context, locals, directives) {
9095
${CONTEXT_ACCESSOR} = context;
9196
${LOCALS_ACCESSOR} = locals;
9297
${directiveInit}
98+
${detectorInit}
9399
}
94100
${type}.prototype.dehydrate = function() {
95101
${pipeOnDestroy}
@@ -128,7 +134,7 @@ function detectChangesBodyTemplate(localDefinitions:string, changeDefinitions:st
128134
${localDefinitions}
129135
${changeDefinitions}
130136
var ${TEMP_LOCAL};
131-
var ${CHANGE_LOCAL};
137+
var ${IS_CHANGED_LOCAL} = false;
132138
var ${CURRENT_PROTO};
133139
var ${CHANGES_LOCAL} = null;
134140

@@ -208,6 +214,7 @@ function updateDirectiveTemplate(oldValue:string, newValue:string, directiveProp
208214
return `
209215
if(throwOnChange) ${UTIL}.throwOnChange(${CURRENT_PROTO}, ${UTIL}.simpleChange(${oldValue}, ${newValue}));
210216
${directiveProperty} = ${newValue};
217+
${IS_CHANGED_LOCAL} = true;
211218
`;
212219
}
213220
@@ -227,6 +234,22 @@ if(${CHANGES_LOCAL}) {
227234
`;
228235
}
229236
237+
function notifyOnPushDetectorsTemplate(detector:string):string{
238+
return `
239+
if(${IS_CHANGED_LOCAL}) {
240+
${detector}.markAsCheckOnce();
241+
}
242+
`;
243+
}
244+
245+
function lastInDirectiveTemplate(notifyOnChanges:string, notifyOnPush:string):string{
246+
return `
247+
${notifyOnChanges}
248+
${notifyOnPush}
249+
${IS_CHANGED_LOCAL} = false;
250+
`;
251+
}
252+
230253
231254
export class ChangeDetectorJITGenerator {
232255
typeName:string;
@@ -285,22 +308,32 @@ export class ChangeDetectorJITGenerator {
285308
genHydrate():string {
286309
var mode = ChangeDetectionUtil.changeDetectionMode(this.changeDetectionStrategy);
287310
return hydrateTemplate(this.typeName, mode, this.genFieldDefinitions(),
288-
pipeOnDestroyTemplate(this.getNonNullPipeNames()), this.getDirectiveFieldNames());
311+
pipeOnDestroyTemplate(this.getNonNullPipeNames()),
312+
this.getDirectiveFieldNames(), this.getDetectorFieldNames());
289313
}
290314
291315
getDirectiveFieldNames():List<string> {
292316
return this.directiveRecords.map((d) => this.getDirective(d));
293317
}
294318
319+
getDetectorFieldNames():List<string> {
320+
return this.directiveRecords.filter(r => r.isOnPushChangeDetection()).map((d) => this.getDetector(d));
321+
}
322+
295323
getDirective(d:DirectiveRecord) {
296324
return `this.directive_${d.name}`;
297325
}
298326
327+
getDetector(d:DirectiveRecord) {
328+
return `this.detector_${d.name}`;
329+
}
330+
299331
genFieldDefinitions() {
300332
var fields = [];
301333
fields = fields.concat(this.fieldNames);
302334
fields = fields.concat(this.getNonNullPipeNames());
303335
fields = fields.concat(this.getDirectiveFieldNames());
336+
fields = fields.concat(this.getDetectorFieldNames());
304337
return fieldDefinitionsTemplate(fields);
305338
}
306339
@@ -362,11 +395,11 @@ export class ChangeDetectorJITGenerator {
362395
var change = this.changeNames[r.selfIndex];
363396
364397
var pipe = this.pipeNames[r.selfIndex];
365-
var cdRef = r.mode === RECORD_TYPE_BINDING_PIPE ? "this.changeDetectorRef" : "null";
398+
var cdRef = r.mode === RECORD_TYPE_BINDING_PIPE ? "this.ref" : "null";
366399
367400
var update = this.genUpdateDirectiveOrElement(r);
368401
var addToChanges = this.genAddToChanges(r);
369-
var lastInDirective = this.genNotifyOnChanges(r);
402+
var lastInDirective = this.genLastInDirective(r);
370403
371404
return pipeCheckTemplate(r.selfIndex - 1, context, cdRef, pipe, r.name, oldValue, newValue, change,
372405
update, addToChanges, lastInDirective);
@@ -380,7 +413,7 @@ export class ChangeDetectorJITGenerator {
380413
381414
var update = this.genUpdateDirectiveOrElement(r);
382415
var addToChanges = this.genAddToChanges(r);
383-
var lastInDirective = this.genNotifyOnChanges(r);
416+
var lastInDirective = this.genLastInDirective(r);
384417
385418
var check = referenceCheckTemplate(r.selfIndex - 1, assignment, oldValue, newValue, change,
386419
update, addToChanges, lastInDirective);
@@ -471,6 +504,12 @@ export class ChangeDetectorJITGenerator {
471504
return r.bindingRecord.callOnChange() ? addToChangesTemplate(oldValue, newValue) : "";
472505
}
473506

507+
genLastInDirective(r:ProtoRecord):string{
508+
var onChanges = this.genNotifyOnChanges(r);
509+
var onPush = this.genNotifyOnPushDetectors(r);
510+
return lastInDirectiveTemplate(onChanges, onPush);
511+
}
512+
474513
genNotifyOnChanges(r:ProtoRecord):string{
475514
var br = r.bindingRecord;
476515
if (r.lastInDirective && br.callOnChange()) {
@@ -480,6 +519,15 @@ export class ChangeDetectorJITGenerator {
480519
}
481520
}
482521

522+
genNotifyOnPushDetectors(r:ProtoRecord):string{
523+
var br = r.bindingRecord;
524+
if (r.lastInDirective && br.isOnPushChangeDetection()) {
525+
return notifyOnPushDetectorsTemplate(this.getDetector(br.directiveRecord));
526+
} else {
527+
return "";
528+
}
529+
}
530+
483531
genArgs(r:ProtoRecord):string {
484532
return r.args.map((arg) => this.localNames[arg]).join(", ");
485533
}

‎modules/angular2/src/change_detection/directive_record.js‎

Lines changed: 10 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,16 +1,24 @@
1+
import {ON_PUSH} from './constants';
2+
import {StringWrapper} from 'angular2/src/facade/lang';
3+
14
export class DirectiveRecord {
25
elementIndex:number;
36
directiveIndex:number;
47
callOnAllChangesDone:boolean;
58
callOnChange:boolean;
9+
changeDetection:string;
610

711
constructor(elementIndex:number, directiveIndex:number,
8-
callOnAllChangesDone:boolean,
9-
callOnChange:boolean) {
12+
callOnAllChangesDone:boolean, callOnChange:boolean, changeDetection:string) {
1013
this.elementIndex = elementIndex;
1114
this.directiveIndex = directiveIndex;
1215
this.callOnAllChangesDone = callOnAllChangesDone;
1316
this.callOnChange = callOnChange;
17+
this.changeDetection = changeDetection;
18+
}
19+
20+
isOnPushChangeDetection():boolean {
21+
return StringWrapper.equals(this.changeDetection, ON_PUSH);
1422
}
1523

1624
get name() {

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

Lines changed: 26 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -95,19 +95,31 @@ export class DynamicChangeDetector extends AbstractChangeDetector {
9595
var protos:List<ProtoRecord> = this.protos;
9696

9797
var changes = null;
98+
var isChanged = false;
9899
for (var i = 0; i < protos.length; ++i) {
99100
var proto:ProtoRecord = protos[i];
101+
var bindingRecord = proto.bindingRecord;
102+
var directiveRecord = bindingRecord.directiveRecord;
100103

101104
var change = this._check(proto);
102105
if (isPresent(change)) {
103106
if (throwOnChange) ChangeDetectionUtil.throwOnChange(proto, change);
104-
this._updateDirectiveOrElement(change, proto.bindingRecord);
105-
changes = this._addChange(proto.bindingRecord, change, changes);
107+
this._updateDirectiveOrElement(change, bindingRecord);
108+
isChanged = true;
109+
changes = this._addChange(bindingRecord, change, changes);
106110
}
107111

108-
if (proto.lastInDirective && isPresent(changes)) {
109-
this._directive(proto.bindingRecord.directiveRecord).onChange(changes);
110-
changes = null;
112+
if (proto.lastInDirective) {
113+
if (isPresent(changes)) {
114+
this._getDirectiveFor(directiveRecord).onChange(changes);
115+
changes = null;
116+
}
117+
118+
if (isChanged && bindingRecord.isOnPushChangeDetection()) {
119+
this._getDetectorFor(directiveRecord).markAsCheckOnce();
120+
}
121+
122+
isChanged = false;
111123
}
112124
}
113125
}
@@ -117,7 +129,7 @@ export class DynamicChangeDetector extends AbstractChangeDetector {
117129
for (var i = dirs.length - 1; i >= 0; --i) {
118130
var dir = dirs[i];
119131
if (dir.callOnAllChangesDone) {
120-
this._directive(dir).onAllChangesDone();
132+
this._getDirectiveFor(dir).onAllChangesDone();
121133
}
122134
}
123135
}
@@ -126,7 +138,7 @@ export class DynamicChangeDetector extends AbstractChangeDetector {
126138
if (isBlank(bindingRecord.directiveRecord)) {
127139
this.dispatcher.notifyOnBinding(bindingRecord, change.currentValue);
128140
} else {
129-
bindingRecord.setter(this._directive(bindingRecord.directiveRecord), change.currentValue);
141+
bindingRecord.setter(this._getDirectiveFor(bindingRecord.directiveRecord), change.currentValue);
130142
}
131143
}
132144

@@ -138,8 +150,12 @@ export class DynamicChangeDetector extends AbstractChangeDetector {
138150
}
139151
}
140152

141-
_directive(directive:DirectiveRecord) {
142-
return this.directives.directive(directive);
153+
_getDirectiveFor(directive:DirectiveRecord) {
154+
return this.directives.getDirectiveFor(directive);
155+
}
156+
157+
_getDetectorFor(directive:DirectiveRecord) {
158+
return this.directives.getDetectorFor(directive);
143159
}
144160

145161
_check(proto:ProtoRecord) {
@@ -249,7 +265,7 @@ export class DynamicChangeDetector extends AbstractChangeDetector {
249265
//
250266
// In the future, pipes declared in the bind configuration should
251267
// be able to access the changeDetectorRef of that component.
252-
var cdr = proto.mode === RECORD_TYPE_BINDING_PIPE ? this.changeDetectorRef : null;
268+
var cdr = proto.mode === RECORD_TYPE_BINDING_PIPE ? this.ref : null;
253269
var pipe = this.pipeRegistry.get(proto.name, context, cdr);
254270
this._writePipe(proto, pipe);
255271
return pipe;

‎modules/angular2/src/core/annotations/annotations.js‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
import {ABSTRACT, CONST, normalizeBlank, isPresent} from 'angular2/src/facade/lang';
22
import {ListWrapper, List} from 'angular2/src/facade/collection';
33
import {Injectable} from 'angular2/di';
4+
import {DEFAULT} from 'angular2/change_detection';
45

56
// type StringMap = {[idx: string]: string};
67

@@ -553,7 +554,7 @@ export class Component extends Directive {
553554
hostListeners,
554555
injectables,
555556
lifecycle,
556-
changeDetection
557+
changeDetection = DEFAULT
557558
}:{
558559
selector:string,
559560
properties:Object,

‎modules/angular2/src/core/compiler/element_injector.js‎

Lines changed: 18 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,7 @@ import * as viewModule from 'angular2/src/core/compiler/view';
88
import {ViewContainer} from 'angular2/src/core/compiler/view_container';
99
import {NgElement} from 'angular2/src/core/compiler/ng_element';
1010
import {Directive, Component, onChange, onDestroy, onAllChangesDone} from 'angular2/src/core/annotations/annotations';
11-
import {ChangeDetectorRef} from 'angular2/change_detection';
11+
import {ChangeDetector, ChangeDetectorRef} from 'angular2/change_detection';
1212
import {QueryList} from './query_list';
1313

1414
var _MAX_DIRECTIVE_CONSTRUCTION_COUNTER = 10;
@@ -277,6 +277,15 @@ export class DirectiveBinding extends ResolvedBinding {
277277
}
278278
}
279279

280+
get changeDetection() {
281+
if (this.annotation instanceof Component) {
282+
var c:Component = this.annotation;
283+
return c.changeDetection;
284+
} else {
285+
return null;
286+
}
287+
}
288+
280289
static createFromBinding(b:Binding, annotation:Directive):DirectiveBinding {
281290
var rb = b.resolve();
282291
var deps = ListWrapper.map(rb.dependencies, DirectiveDependency.createFrom);
@@ -298,13 +307,13 @@ export class PreBuiltObjects {
298307
view:viewModule.AppView;
299308
element:NgElement;
300309
viewContainer:ViewContainer;
301-
changeDetectorRef:ChangeDetectorRef;
310+
changeDetector:ChangeDetector;
302311
constructor(view, element:NgElement, viewContainer:ViewContainer,
303-
changeDetectorRef:ChangeDetectorRef) {
312+
changeDetector:ChangeDetector) {
304313
this.view = view;
305314
this.element = element;
306315
this.viewContainer = viewContainer;
307-
this.changeDetectorRef = changeDetectorRef;
316+
this.changeDetector = changeDetector;
308317
}
309318
}
310319

@@ -603,6 +612,10 @@ export class ElementInjector extends TreeNode {
603612
return this._preBuiltObjects.element;
604613
}
605614

615+
getChangeDetector() {
616+
return this._preBuiltObjects.changeDetector;
617+
}
618+
606619
getComponent() {
607620
if (this._proto._binding0IsComponent) {
608621
return this._obj0;
@@ -885,7 +898,7 @@ export class ElementInjector extends TreeNode {
885898
if (keyId === staticKeys.viewId) return this._preBuiltObjects.view;
886899
if (keyId === staticKeys.ngElementId) return this._preBuiltObjects.element;
887900
if (keyId === staticKeys.viewContainerId) return this._preBuiltObjects.viewContainer;
888-
if (keyId === staticKeys.changeDetectorRefId) return this._preBuiltObjects.changeDetectorRef;
901+
if (keyId === staticKeys.changeDetectorRefId) return this._preBuiltObjects.changeDetector.ref;
889902

890903
//TODO add other objects as needed
891904
return _undefined;

0 commit comments

Comments
 (0)