feat(change_detector): notify directives on property changes

Author: vsavkin

modules/change_detection/src/change_detector.js

@@ -1,25 +1,70 @@
 import {ProtoRecordRange, RecordRange} from './record_range';
 import {ProtoRecord, Record} from './record';
-import {FIELD, int, isPresent} from 'facade/lang';
+import {int, isPresent, isBlank} from 'facade/lang';
+import {ListWrapper, List} from 'facade/collection';
 
 export * from './record';
 export * from './record_range'
 
 export class ChangeDetector {
  _rootRecordRange:RecordRange;
+
  constructor(recordRange:RecordRange) {
  this._rootRecordRange = recordRange;
  }
 
  detectChanges():int {
- var count:int = 0;
- for (var record = this._rootRecordRange.findFirstEnabledRecord();
- isPresent(record);
- record = record.nextEnabled) {
+ var count = 0;
+ var updatedRecords = null;
+ var record = this._rootRecordRange.findFirstEnabledRecord();
+
+ while (isPresent(record)) {
+ var currentRange = record.recordRange;
+ var currentGroup = record.groupMemento();
+
+ var nextEnabled = record.nextEnabled;
+ var nextRange = isPresent(nextEnabled) ? nextEnabled.recordRange : null;
+ var nextGroup = isPresent(nextEnabled) ? nextEnabled.groupMemento() : null;
+
  if (record.check()) {
- count++;
+ count ++;
+ if (record.terminatesExpression()) {
+ updatedRecords = this._addRecord(updatedRecords, record);
+ }
  }
+
+ if (this._shouldNotifyDispatcher(currentRange, nextRange, currentGroup, nextGroup, updatedRecords)) {
+ currentRange.dispatcher.onRecordChange(currentGroup, updatedRecords);
+ updatedRecords = null;
+ }
+
+ record = record.nextEnabled;
  }
+
  return count;
  }
+
+ _groupChanged(currentRange, nextRange, currentGroup, nextGroup) {
+ return currentRange != nextRange || currentGroup != nextGroup;
+ }
+
+ _shouldNotifyDispatcher(currentRange, nextRange, currentGroup, nextGroup, updatedRecords) {
+ return this._groupChanged(currentRange, nextRange, currentGroup, nextGroup) && isPresent(updatedRecords);
+ }
+
+ _addRecord(updatedRecords:List, record:Record) {
+ if (isBlank(updatedRecords)) {
+ updatedRecords = _singleElementList;
+ updatedRecords[0] = record;
+
+ } else if (updatedRecords === _singleElementList) {
+ updatedRecords = [_singleElementList[0], record];
+
+ } else {
+ ListWrapper.push(updatedRecords, record);
+ }
+ return updatedRecords;
+ }
 }
+
+var _singleElementList = [null];

modules/change_detection/src/record.js

@@ -34,22 +34,27 @@ export class ProtoRecord {
  arity:int;
  name:string;
  dest:any;
+ groupMemento:any;
+
  next:ProtoRecord;
 
  recordInConstruction:Record;
+
  constructor(recordRange:ProtoRecordRange,
  mode:int,
  funcOrValue,
  arity:int,
  name:string,
- dest) {
+ dest,
+ groupMemento) {
 
  this.recordRange = recordRange;
  this._mode = mode;
  this.funcOrValue = funcOrValue;
  this.arity = arity;
  this.name = name;
  this.dest = dest;
+ this.groupMemento = groupMemento;
 
  this.next = null;
  // The concrete Record instantiated from this ProtoRecord
@@ -190,21 +195,20 @@ export class Record {
 
  check():boolean {
  if (this.isCollection) {
- var changed = this._checkCollection();
- if (changed) {
- this._notifyDispatcher();
- return true;
- }
- return false;
+ return this._checkCollection();
  } else {
- this.previousValue = this.currentValue;
- this.currentValue = this._calculateNewValue();
- if (isSame(this.previousValue, this.currentValue)) return false;
- this._updateDestination();
- return true;
+ return this._checkSingleRecord();
  }
  }
 
+ _checkSingleRecord():boolean {
+ this.previousValue = this.currentValue;
+ this.currentValue = this._calculateNewValue();
+ if (isSame(this.previousValue, this.currentValue)) return false;
+ this._updateDestination();
+ return true;
+ }
+
  _updateDestination() {
  // todo(vicb): compute this info only once in ctor ? (add a bit in mode not to grow the mem req)
  if (this.dest instanceof Record) {
@@ -213,15 +217,9 @@ export class Record {
  } else {
  this.dest.updateContext(this.currentValue);
  }
- } else {
- this._notifyDispatcher();
  }
  }
 
- _notifyDispatcher() {
- this.recordRange.dispatcher.onRecordChange(this, this.protoRecord.dest);
- }
-
  // return whether the content has changed
  _checkCollection():boolean {
  switch(this.type) {
@@ -307,9 +305,21 @@ export class Record {
  }
  }
 
+ terminatesExpression():boolean {
+ return !(this.dest instanceof Record);
+ }
+
  get isMarkerRecord() {
  return this.type == RECORD_TYPE_MARKER;
  }
+
+ expressionMemento() {
+ return this.protoRecord.dest;
+ }
+
+ groupMemento() {
+ return isPresent(this.protoRecord) ? this.protoRecord.groupMemento : null;
+ }
 }
 
 function isSame(a, b) {

modules/change_detection/src/record_range.js

@@ -45,21 +45,24 @@ export class ProtoRecordRange {
  * Parses [ast] into [ProtoRecord]s and adds them to [ProtoRecordRange].
  *
  * @param ast The expression to watch
- * @param memento an opaque object which will be passed to WatchGroupDispatcher on
+ * @param expressionMemento an opaque object which will be passed to WatchGroupDispatcher on
  * detecting a change.
  * @param content Wether to watch collection content (true) or reference (false, default)
  */
  addRecordsFromAST(ast:AST,
- memento,
+ expressionMemento,
+ groupMemento,
  content:boolean = false)
  {
  if (this.recordCreator === null) {
  this.recordCreator = new ProtoRecordCreator(this);
  }
+ 
  if (content) {
  ast = new Collection(ast);
  }
- this.recordCreator.createRecordsFromAST(ast, memento);
+ 
+ this.recordCreator.createRecordsFromAST(ast, expressionMemento, groupMemento);
  }
 
  // TODO(rado): the type annotation should be dispatcher:WatchGroupDispatcher.
@@ -85,6 +88,8 @@ export class ProtoRecordRange {
  for (var proto = this.recordCreator.headRecord; proto != null; proto = proto.next) {
  if (proto.dest instanceof Destination) {
  proto.recordInConstruction.dest = proto.dest.record.recordInConstruction;
+ } else {
+ proto.recordInConstruction.dest = proto.dest;
  }
  proto.recordInConstruction = null;
  }
@@ -378,6 +383,8 @@ class ProtoRecordCreator {
  protoRecordRange:ProtoRecordRange;
  headRecord:ProtoRecord;
  tailRecord:ProtoRecord;
+ groupMemento:any;
+
  constructor(protoRecordRange) {
  this.protoRecordRange = protoRecordRange;
  this.headRecord = null;
@@ -491,12 +498,13 @@ class ProtoRecordCreator {
 
  visitTemplateBindings(ast, dest) {this._unsupported();}
 
- createRecordsFromAST(ast:AST, memento){
- ast.visit(this, memento);
+ createRecordsFromAST(ast:AST, expressionMemento:any, groupMemento:any){
+ this.groupMemento = groupMemento;
+ ast.visit(this, expressionMemento);
  }
 
  construct(recordType, funcOrValue, arity, name, dest) {
- return new ProtoRecord(this.protoRecordRange, recordType, funcOrValue, arity, name, dest);
+ return new ProtoRecord(this.protoRecordRange, recordType, funcOrValue, arity, name, dest, this.groupMemento);
  }
 
  add(protoRecord:ProtoRecord) {

modules/change_detection/test/change_detector_spec.js

@@ -26,7 +26,7 @@ export function main() {
  function createChangeDetector(memo:string, exp:string, context = null, formatters = null,
  content = false) {
  var prr = new ProtoRecordRange();
- prr.addRecordsFromAST(ast(exp), memo, content);
+ prr.addRecordsFromAST(ast(exp), memo, memo, content);
 
  var dispatcher = new LoggingDispatcher();
  var rr = prr.instantiate(dispatcher, formatters);
@@ -97,21 +97,21 @@ export function main() {
  it("should support literal array", () => {
  var c = createChangeDetector('array', '[1,2]');
  c["changeDetector"].detectChanges();
- expect(c["dispatcher"].loggedValues).toEqual([[1,2]]);
+ expect(c["dispatcher"].loggedValues).toEqual([[[1,2]]]);
 
  c = createChangeDetector('array', '[1,a]', new TestData(2));
  c["changeDetector"].detectChanges();
- expect(c["dispatcher"].loggedValues).toEqual([[1,2]]);
+ expect(c["dispatcher"].loggedValues).toEqual([[[1,2]]]);
  });
 
  it("should support literal maps", () => {
  var c = createChangeDetector('map', '{z:1}');
  c["changeDetector"].detectChanges();
- expect(MapWrapper.get(c["dispatcher"].loggedValues[0], 'z')).toEqual(1);
+ expect(MapWrapper.get(c["dispatcher"].loggedValues[0][0], 'z')).toEqual(1);
 
  c = createChangeDetector('map', '{z:a}', new TestData(1));
  c["changeDetector"].detectChanges();
- expect(MapWrapper.get(c["dispatcher"].loggedValues[0], 'z')).toEqual(1);
+ expect(MapWrapper.get(c["dispatcher"].loggedValues[0][0], 'z')).toEqual(1);
  });
 
  it("should support binary operations", () => {
@@ -242,6 +242,7 @@ export function main() {
 
  context.a = [0];
  cd.detectChanges();
+
  expect(dsp.log).toEqual(["a=" +
  arrayChangesAsString({
  collection: ['0[null->0]'],
@@ -343,10 +344,69 @@ export function main() {
  });
  }
  });
+
+ describe("onGroupChange", () => {
+ it("should notify the dispatcher when a group of records changes", () => {
+ var prr = new ProtoRecordRange();
+ prr.addRecordsFromAST(ast("1 + 2"), "memo", 1);
+ prr.addRecordsFromAST(ast("10 + 20"), "memo", 1);
+ prr.addRecordsFromAST(ast("100 + 200"), "memo2", 2);
+
+ var dispatcher = new LoggingDispatcher();
+ var rr = prr.instantiate(dispatcher, null);
+
+ var cd = new ChangeDetector(rr);
+ cd.detectChanges();
+
+ expect(dispatcher.loggedValues).toEqual([[3, 30], [300]]);
+ });
+
+ it("should update every instance of a group individually", () => {
+ var prr = new ProtoRecordRange();
+ prr.addRecordsFromAST(ast("1 + 2"), "memo", "memo");
+
+ var dispatcher = new LoggingDispatcher();
+ var rr = new RecordRange(null, dispatcher);
+ rr.addRange(prr.instantiate(dispatcher, null));
+ rr.addRange(prr.instantiate(dispatcher, null));
+
+ var cd = new ChangeDetector(rr);
+ cd.detectChanges();
+
+ expect(dispatcher.loggedValues).toEqual([[3], [3]]);
+ });
+
+ it("should notify the dispatcher before switching to the next group", () => {
+ var prr = new ProtoRecordRange();
+ prr.addRecordsFromAST(ast("a()"), "a", 1);
+ prr.addRecordsFromAST(ast("b()"), "b", 2);
+ prr.addRecordsFromAST(ast("c()"), "b", 2);
+
+ var dispatcher = new LoggingDispatcher();
+ var rr = prr.instantiate(dispatcher, null);
+
+ var tr = new TestRecord();
+ tr.a = () => {dispatcher.logValue('InvokeA'); return 'a'};
+ tr.b = () => {dispatcher.logValue('InvokeB'); return 'b'};
+ tr.c = () => {dispatcher.logValue('InvokeC'); return 'c'};
+ rr.setContext(tr);
+
+ var cd = new ChangeDetector(rr);
+ cd.detectChanges();
+
+ expect(dispatcher.loggedValues).toEqual(['InvokeA', ['a'], 'InvokeB', 'InvokeC', ['b', 'c']]);
+ });
+ });
  });
  });
 }
 
+class TestRecord {
+ a;
+ b;
+ c;
+}
+
 class Person {
  name:string;
  address:Address;
@@ -387,6 +447,7 @@ class TestData {
 class LoggingDispatcher extends WatchGroupDispatcher {
  log:List;
  loggedValues:List;
+
  constructor() {
  this.log = null;
  this.loggedValues = null;
@@ -397,10 +458,18 @@ class LoggingDispatcher extends WatchGroupDispatcher {
  this.log = ListWrapper.create();
  this.loggedValues = ListWrapper.create();
  }
+ 
+ logValue(value) {
+ ListWrapper.push(this.loggedValues, value);
+ }
+
+ onRecordChange(group, records:List) {
+ var value = records[0].currentValue;
+ var dest = records[0].protoRecord.dest;
+ ListWrapper.push(this.log, dest + '=' + this._asString(value));
 
- onRecordChange(record:Record, context) {
- ListWrapper.push(this.loggedValues, record.currentValue);
- ListWrapper.push(this.log, context + '=' + this._asString(record.currentValue));
+ var values = ListWrapper.map(records, (r) => r.currentValue);
+ ListWrapper.push(this.loggedValues, values);
  }
 
  _asString(value) {

modules/change_detection/test/record_range_spec.js

@@ -46,7 +46,7 @@ export function main() {
  }
 
  function createRecord(rr) {
- return new Record(rr, new ProtoRecord(null, 0, null, null, null, null), null);
+ return new Record(rr, new ProtoRecord(null, 0, null, null, null, null, null), null);
  }
 
  describe('record range', () => {

modules/core/src/compiler/interfaces.js

@@ -0,0 +1,5 @@
+export class OnChange {
+ onChange(changes) {
+ throw "not implemented";
+ }
+}