Skip to content

Commit

Permalink
feat(change_detection): updated handling ON_PUSH detectors so they ge…
Browse files Browse the repository at this point in the history
…t notified when their bindings change
  • Loading branch information
vsavkin committed Apr 15, 2015
1 parent dc9c614 commit 68faddb
Show file tree
Hide file tree
Showing 13 changed files with 249 additions and 60 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -9,13 +9,13 @@ export class AbstractChangeDetector extends ChangeDetector {
shadowDomChildren:List;
parent:ChangeDetector;
mode:string;
changeDetectorRef:ChangeDetectorRef;
ref:ChangeDetectorRef;

constructor() {
super();
this.lightDomChildren = [];
this.shadowDomChildren = [];
this.changeDetectorRef = new ChangeDetectorRef(this);
this.ref = new ChangeDetectorRef(this);
this.mode = null;
}

Expand Down Expand Up @@ -80,6 +80,10 @@ export class AbstractChangeDetector extends ChangeDetector {
}
}

markAsCheckOnce() {
this.mode = CHECK_ONCE;
}

markPathToRootAsCheckOnce() {
var c = this;
while(isPresent(c) && c.mode != DETACHED) {
Expand Down
4 changes: 4 additions & 0 deletions modules/angular2/src/change_detection/binding_record.js
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,10 @@ export class BindingRecord {
return isPresent(this.directiveRecord) && this.directiveRecord.callOnChange;
}

isOnPushChangeDetection() {
return isPresent(this.directiveRecord) && this.directiveRecord.isOnPushChangeDetection();
}

isDirective() {
return this.mode === DIRECTIVE;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,7 @@ var PIPE_REGISTRY_ACCESSOR = "this.pipeRegistry";
var PROTOS_ACCESSOR = "this.protos";
var DIRECTIVES_ACCESSOR = "this.directiveRecords";
var CONTEXT_ACCESSOR = "this.context";
var CHANGE_LOCAL = "change";
var IS_CHANGED_LOCAL = "isChanged";
var CHANGES_LOCAL = "changes";
var LOCALS_ACCESSOR = "this.locals";
var MODE_ACCESSOR = "this.mode";
Expand Down Expand Up @@ -78,10 +78,15 @@ function pipeOnDestroyTemplate(pipeNames:List) {
}

function hydrateTemplate(type:string, mode:string, fieldDefinitions:string, pipeOnDestroy:string,
directiveFieldNames:List<String>):string {
directiveFieldNames:List<String>, detectorFieldNames:List<String>):string {
var directiveInit = "";
for(var i = 0; i < directiveFieldNames.length; ++i) {
directiveInit += `${directiveFieldNames[i]} = directives.directive(this.directiveRecords[${i}]);\n`;
directiveInit += `${directiveFieldNames[i]} = directives.getDirectiveFor(this.directiveRecords[${i}]);\n`;
}

var detectorInit = "";
for(var i = 0; i < detectorFieldNames.length; ++i) {
detectorInit += `${detectorFieldNames[i]} = directives.getDetectorFor(this.directiveRecords[${i}]);\n`;
}

return `
Expand All @@ -90,6 +95,7 @@ ${type}.prototype.hydrate = function(context, locals, directives) {
${CONTEXT_ACCESSOR} = context;
${LOCALS_ACCESSOR} = locals;
${directiveInit}
${detectorInit}
}
${type}.prototype.dehydrate = function() {
${pipeOnDestroy}
Expand Down Expand Up @@ -128,7 +134,7 @@ function detectChangesBodyTemplate(localDefinitions:string, changeDefinitions:st
${localDefinitions}
${changeDefinitions}
var ${TEMP_LOCAL};
var ${CHANGE_LOCAL};
var ${IS_CHANGED_LOCAL} = false;
var ${CURRENT_PROTO};
var ${CHANGES_LOCAL} = null;

Expand Down Expand Up @@ -208,6 +214,7 @@ function updateDirectiveTemplate(oldValue:string, newValue:string, directiveProp
return `
if(throwOnChange) ${UTIL}.throwOnChange(${CURRENT_PROTO}, ${UTIL}.simpleChange(${oldValue}, ${newValue}));
${directiveProperty} = ${newValue};
${IS_CHANGED_LOCAL} = true;
`;
}
Expand All @@ -227,6 +234,22 @@ if(${CHANGES_LOCAL}) {
`;
}
function notifyOnPushDetectorsTemplate(detector:string):string{
return `
if(${IS_CHANGED_LOCAL}) {
${detector}.markAsCheckOnce();
}
`;
}
function lastInDirectiveTemplate(notifyOnChanges:string, notifyOnPush:string):string{
return `
${notifyOnChanges}
${notifyOnPush}
${IS_CHANGED_LOCAL} = false;
`;
}
export class ChangeDetectorJITGenerator {
typeName:string;
Expand Down Expand Up @@ -285,22 +308,32 @@ export class ChangeDetectorJITGenerator {
genHydrate():string {
var mode = ChangeDetectionUtil.changeDetectionMode(this.changeDetectionStrategy);
return hydrateTemplate(this.typeName, mode, this.genFieldDefinitions(),
pipeOnDestroyTemplate(this.getNonNullPipeNames()), this.getDirectiveFieldNames());
pipeOnDestroyTemplate(this.getNonNullPipeNames()),
this.getDirectiveFieldNames(), this.getDetectorFieldNames());
}
getDirectiveFieldNames():List<string> {
return this.directiveRecords.map((d) => this.getDirective(d));
}
getDetectorFieldNames():List<string> {
return this.directiveRecords.filter(r => r.isOnPushChangeDetection()).map((d) => this.getDetector(d));
}
getDirective(d:DirectiveRecord) {
return `this.directive_${d.name}`;
}
getDetector(d:DirectiveRecord) {
return `this.detector_${d.name}`;
}
genFieldDefinitions() {
var fields = [];
fields = fields.concat(this.fieldNames);
fields = fields.concat(this.getNonNullPipeNames());
fields = fields.concat(this.getDirectiveFieldNames());
fields = fields.concat(this.getDetectorFieldNames());
return fieldDefinitionsTemplate(fields);
}
Expand Down Expand Up @@ -362,11 +395,11 @@ export class ChangeDetectorJITGenerator {
var change = this.changeNames[r.selfIndex];
var pipe = this.pipeNames[r.selfIndex];
var cdRef = r.mode === RECORD_TYPE_BINDING_PIPE ? "this.changeDetectorRef" : "null";
var cdRef = r.mode === RECORD_TYPE_BINDING_PIPE ? "this.ref" : "null";
var update = this.genUpdateDirectiveOrElement(r);
var addToChanges = this.genAddToChanges(r);
var lastInDirective = this.genNotifyOnChanges(r);
var lastInDirective = this.genLastInDirective(r);
return pipeCheckTemplate(r.selfIndex - 1, context, cdRef, pipe, r.name, oldValue, newValue, change,
update, addToChanges, lastInDirective);
Expand All @@ -380,7 +413,7 @@ export class ChangeDetectorJITGenerator {
var update = this.genUpdateDirectiveOrElement(r);
var addToChanges = this.genAddToChanges(r);
var lastInDirective = this.genNotifyOnChanges(r);
var lastInDirective = this.genLastInDirective(r);
var check = referenceCheckTemplate(r.selfIndex - 1, assignment, oldValue, newValue, change,
update, addToChanges, lastInDirective);
Expand Down Expand Up @@ -471,6 +504,12 @@ export class ChangeDetectorJITGenerator {
return r.bindingRecord.callOnChange() ? addToChangesTemplate(oldValue, newValue) : "";
}

genLastInDirective(r:ProtoRecord):string{
var onChanges = this.genNotifyOnChanges(r);
var onPush = this.genNotifyOnPushDetectors(r);
return lastInDirectiveTemplate(onChanges, onPush);
}

genNotifyOnChanges(r:ProtoRecord):string{
var br = r.bindingRecord;
if (r.lastInDirective && br.callOnChange()) {
Expand All @@ -480,6 +519,15 @@ export class ChangeDetectorJITGenerator {
}
}

genNotifyOnPushDetectors(r:ProtoRecord):string{
var br = r.bindingRecord;
if (r.lastInDirective && br.isOnPushChangeDetection()) {
return notifyOnPushDetectorsTemplate(this.getDetector(br.directiveRecord));
} else {
return "";
}
}

genArgs(r:ProtoRecord):string {
return r.args.map((arg) => this.localNames[arg]).join(", ");
}
Expand Down
12 changes: 10 additions & 2 deletions modules/angular2/src/change_detection/directive_record.js
Original file line number Diff line number Diff line change
@@ -1,16 +1,24 @@
import {ON_PUSH} from './constants';
import {StringWrapper} from 'angular2/src/facade/lang';

export class DirectiveRecord {
elementIndex:number;
directiveIndex:number;
callOnAllChangesDone:boolean;
callOnChange:boolean;
changeDetection:string;

constructor(elementIndex:number, directiveIndex:number,
callOnAllChangesDone:boolean,
callOnChange:boolean) {
callOnAllChangesDone:boolean, callOnChange:boolean, changeDetection:string) {
this.elementIndex = elementIndex;
this.directiveIndex = directiveIndex;
this.callOnAllChangesDone = callOnAllChangesDone;
this.callOnChange = callOnChange;
this.changeDetection = changeDetection;
}

isOnPushChangeDetection():boolean {
return StringWrapper.equals(this.changeDetection, ON_PUSH);
}

get name() {
Expand Down
36 changes: 26 additions & 10 deletions modules/angular2/src/change_detection/dynamic_change_detector.js
Original file line number Diff line number Diff line change
Expand Up @@ -95,19 +95,31 @@ export class DynamicChangeDetector extends AbstractChangeDetector {
var protos:List<ProtoRecord> = this.protos;

var changes = null;
var isChanged = false;
for (var i = 0; i < protos.length; ++i) {
var proto:ProtoRecord = protos[i];
var bindingRecord = proto.bindingRecord;
var directiveRecord = bindingRecord.directiveRecord;

var change = this._check(proto);
if (isPresent(change)) {
if (throwOnChange) ChangeDetectionUtil.throwOnChange(proto, change);
this._updateDirectiveOrElement(change, proto.bindingRecord);
changes = this._addChange(proto.bindingRecord, change, changes);
this._updateDirectiveOrElement(change, bindingRecord);
isChanged = true;
changes = this._addChange(bindingRecord, change, changes);
}

if (proto.lastInDirective && isPresent(changes)) {
this._directive(proto.bindingRecord.directiveRecord).onChange(changes);
changes = null;
if (proto.lastInDirective) {
if (isPresent(changes)) {
this._getDirectiveFor(directiveRecord).onChange(changes);
changes = null;
}

if (isChanged && bindingRecord.isOnPushChangeDetection()) {
this._getDetectorFor(directiveRecord).markAsCheckOnce();
}

isChanged = false;
}
}
}
Expand All @@ -117,7 +129,7 @@ export class DynamicChangeDetector extends AbstractChangeDetector {
for (var i = dirs.length - 1; i >= 0; --i) {
var dir = dirs[i];
if (dir.callOnAllChangesDone) {
this._directive(dir).onAllChangesDone();
this._getDirectiveFor(dir).onAllChangesDone();
}
}
}
Expand All @@ -126,7 +138,7 @@ export class DynamicChangeDetector extends AbstractChangeDetector {
if (isBlank(bindingRecord.directiveRecord)) {
this.dispatcher.notifyOnBinding(bindingRecord, change.currentValue);
} else {
bindingRecord.setter(this._directive(bindingRecord.directiveRecord), change.currentValue);
bindingRecord.setter(this._getDirectiveFor(bindingRecord.directiveRecord), change.currentValue);
}
}

Expand All @@ -138,8 +150,12 @@ export class DynamicChangeDetector extends AbstractChangeDetector {
}
}

_directive(directive:DirectiveRecord) {
return this.directives.directive(directive);
_getDirectiveFor(directive:DirectiveRecord) {
return this.directives.getDirectiveFor(directive);
}

_getDetectorFor(directive:DirectiveRecord) {
return this.directives.getDetectorFor(directive);
}

_check(proto:ProtoRecord) {
Expand Down Expand Up @@ -249,7 +265,7 @@ export class DynamicChangeDetector extends AbstractChangeDetector {
//
// In the future, pipes declared in the bind configuration should
// be able to access the changeDetectorRef of that component.
var cdr = proto.mode === RECORD_TYPE_BINDING_PIPE ? this.changeDetectorRef : null;
var cdr = proto.mode === RECORD_TYPE_BINDING_PIPE ? this.ref : null;
var pipe = this.pipeRegistry.get(proto.name, context, cdr);
this._writePipe(proto, pipe);
return pipe;
Expand Down
3 changes: 2 additions & 1 deletion modules/angular2/src/core/annotations/annotations.js
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
import {ABSTRACT, CONST, normalizeBlank, isPresent} from 'angular2/src/facade/lang';
import {ListWrapper, List} from 'angular2/src/facade/collection';
import {Injectable} from 'angular2/di';
import {DEFAULT} from 'angular2/change_detection';

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

Expand Down Expand Up @@ -553,7 +554,7 @@ export class Component extends Directive {
hostListeners,
injectables,
lifecycle,
changeDetection
changeDetection = DEFAULT
}:{
selector:string,
properties:Object,
Expand Down
23 changes: 18 additions & 5 deletions modules/angular2/src/core/compiler/element_injector.js
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ import * as viewModule from 'angular2/src/core/compiler/view';
import {ViewContainer} from 'angular2/src/core/compiler/view_container';
import {NgElement} from 'angular2/src/core/compiler/ng_element';
import {Directive, Component, onChange, onDestroy, onAllChangesDone} from 'angular2/src/core/annotations/annotations';
import {ChangeDetectorRef} from 'angular2/change_detection';
import {ChangeDetector, ChangeDetectorRef} from 'angular2/change_detection';
import {QueryList} from './query_list';

var _MAX_DIRECTIVE_CONSTRUCTION_COUNTER = 10;
Expand Down Expand Up @@ -277,6 +277,15 @@ export class DirectiveBinding extends ResolvedBinding {
}
}

get changeDetection() {
if (this.annotation instanceof Component) {
var c:Component = this.annotation;
return c.changeDetection;
} else {
return null;
}
}

static createFromBinding(b:Binding, annotation:Directive):DirectiveBinding {
var rb = b.resolve();
var deps = ListWrapper.map(rb.dependencies, DirectiveDependency.createFrom);
Expand All @@ -298,13 +307,13 @@ export class PreBuiltObjects {
view:viewModule.AppView;
element:NgElement;
viewContainer:ViewContainer;
changeDetectorRef:ChangeDetectorRef;
changeDetector:ChangeDetector;
constructor(view, element:NgElement, viewContainer:ViewContainer,
changeDetectorRef:ChangeDetectorRef) {
changeDetector:ChangeDetector) {
this.view = view;
this.element = element;
this.viewContainer = viewContainer;
this.changeDetectorRef = changeDetectorRef;
this.changeDetector = changeDetector;
}
}

Expand Down Expand Up @@ -603,6 +612,10 @@ export class ElementInjector extends TreeNode {
return this._preBuiltObjects.element;
}

getChangeDetector() {
return this._preBuiltObjects.changeDetector;
}

getComponent() {
if (this._proto._binding0IsComponent) {
return this._obj0;
Expand Down Expand Up @@ -885,7 +898,7 @@ export class ElementInjector extends TreeNode {
if (keyId === staticKeys.viewId) return this._preBuiltObjects.view;
if (keyId === staticKeys.ngElementId) return this._preBuiltObjects.element;
if (keyId === staticKeys.viewContainerId) return this._preBuiltObjects.viewContainer;
if (keyId === staticKeys.changeDetectorRefId) return this._preBuiltObjects.changeDetectorRef;
if (keyId === staticKeys.changeDetectorRefId) return this._preBuiltObjects.changeDetector.ref;

//TODO add other objects as needed
return _undefined;
Expand Down
Loading

0 comments on commit 68faddb

Please sign in to comment.