fix(change_detector): adding new ranges when disabling the current enabled record

Author: vsavkin

modules/change_detection/src/change_detector.js

@@ -40,7 +40,7 @@ export class ChangeDetector {
  }
  }
 
- record = record.nextEnabled;
+ record = record.findNextEnabled();
  }
 
  return count;

modules/change_detection/src/record.js

@@ -173,6 +173,10 @@ export class Record {
  return (this._mode & RECORD_FLAG_DISABLED) === RECORD_FLAG_DISABLED;
  }
 
+ isEnabled():boolean {
+ return ! this.disabled;
+ }
+
  set disabled(value:boolean) {
  if (value) {
  this._mode |= RECORD_FLAG_DISABLED;
@@ -316,6 +320,49 @@ export class Record {
  groupMemento() {
  return isPresent(this.protoRecord) ? this.protoRecord.groupMemento : null;
  }
+
+
+ /**
+ * Returns the next enabled record. This search is not limited to the current range.
+ *
+ * [H ER1 T] [H ER2 T] _nextEnable(ER1) will return ER2
+ *
+ * The function skips disabled sub ranges.
+ */
+ findNextEnabled() {
+ if (this.isEnabled()) return this.nextEnabled;
+
+ var record = this.next;
+ while (isPresent(record) && record.disabled) {
+ if (record.isMarkerRecord && record.recordRange.disabled) {
+ record = record.recordRange.tailRecord.next;
+ } else {
+ record = record.next;
+ }
+ }
+ return record;
+ }
+
+ /**
+ * Returns the prev enabled record. This search is not limited to the current range.
+ *
+ * [H ER1 T] [H ER2 T] _nextEnable(ER2) will return ER1
+ *
+ * The function skips disabled sub ranges.
+ */
+ findPrevEnabled() {
+ if (this.isEnabled()) return this.prevEnabled;
+
+ var record = this.prev;
+ while (isPresent(record) && record.disabled) {
+ if (record.isMarkerRecord && record.recordRange.disabled) {
+ record = record.recordRange.headRecord.prev;
+ } else {
+ record = record.prev;
+ }
+ }
+ return record;
+ }
 }
 
 function isSame(a, b) {

modules/change_detection/src/record_range.js

@@ -129,8 +129,8 @@ export class RecordRange {
 
  addRange(child:RecordRange) {
  var lastRecord = this.tailRecord.prev;
- var prevEnabledRecord = RecordRange._prevEnabled(this.tailRecord);
- var nextEnabledRerord = RecordRange._nextEnabled(this.tailRecord);
+ var prevEnabledRecord = this.tailRecord.findPrevEnabled();
+ var nextEnabledRerord = this.tailRecord.findNextEnabled();
 
  var firstEnabledChildRecord = child.findFirstEnabledRecord();
  var lastEnabledChildRecord = child.findLastEnabledRecord();
@@ -174,10 +174,10 @@ export class RecordRange {
  }
 
  enableRecord(record:Record) {
- if (!record.disabled) return;
+ if (record.isEnabled()) return;
 
- var prevEnabled = RecordRange._prevEnabled(record);
- var nextEnabled = RecordRange._nextEnabled(record);
+ var prevEnabled = record.findPrevEnabled();
+ var nextEnabled = record.findNextEnabled();
 
  record.prevEnabled = prevEnabled;
  record.nextEnabled = nextEnabled;
@@ -203,8 +203,8 @@ export class RecordRange {
  }
 
  enable() {
- var prevEnabledRecord = RecordRange._prevEnabled(this.headRecord);
- var nextEnabledRecord = RecordRange._nextEnabled(this.tailRecord);
+ var prevEnabledRecord = this.headRecord.findPrevEnabled();
+ var nextEnabledRecord = this.tailRecord.findNextEnabled();
 
  var firstEnabledthisRecord = this.findFirstEnabledRecord();
  var lastEnabledthisRecord = this.findLastEnabledRecord();
@@ -268,44 +268,6 @@ export class RecordRange {
  return record === this.headRecord ? null : record;
  }
 
- /**
- * Returns the next enabled record. This search is not limited to the current range.
- *
- * [H ER1 T] [H ER2 T] _nextEnable(ER1) will return ER2
- *
- * The function skips disabled sub ranges.
- */
- static _nextEnabled(record:Record) {
- record = record.next;
- while (isPresent(record) && record.disabled) {
- if (record.isMarkerRecord && record.recordRange.disabled) {
- record = record.recordRange.tailRecord.next;
- } else {
- record = record.next;
- }
- }
- return record;
- }
-
- /**
- * Returns the prev enabled record. This search is not limited to the current range.
- *
- * [H ER1 T] [H ER2 T] _nextEnable(ER2) will return ER1
- *
- * The function skips disabled sub ranges.
- */
- static _prevEnabled(record:Record) {
- record = record.prev;
- while (isPresent(record) && record.disabled) {
- if (record.isMarkerRecord && record.recordRange.disabled) {
- record = record.recordRange.headRecord.prev;
- } else {
- record = record.prev;
- }
- }
- return record;
- }
-
  /**
  * Sets the context (the object) on which the change detection expressions will
  * dereference themselves on. Since the RecordRange can be reused the context

modules/change_detection/test/change_detector_spec.js

@@ -1,4 +1,4 @@
-import {ddescribe, describe, it, iit, xit, expect} from 'test_lib/test_lib';
+import {ddescribe, describe, it, iit, xit, expect, beforeEach} from 'test_lib/test_lib';
 
 import {isPresent, isBlank, isJsObject} from 'facade/lang';
 import {List, ListWrapper, MapWrapper} from 'facade/collection';
@@ -28,7 +28,7 @@ export function main() {
  var prr = new ProtoRecordRange();
  prr.addRecordsFromAST(ast(exp), memo, memo, content);
 
- var dispatcher = new LoggingDispatcher();
+ var dispatcher = new TestDispatcher();
  var rr = prr.instantiate(dispatcher, formatters);
  rr.setContext(context);
 
@@ -46,6 +46,17 @@ export function main() {
 
  describe('change_detection', () => {
  describe('ChangeDetection', () => {
+ function createRange(dispatcher, ast, group) {
+ var prr = new ProtoRecordRange();
+ prr.addRecordsFromAST(ast, "memo", group);
+ return prr.instantiate(dispatcher, null);
+ }
+
+ function detectChangesInRange(recordRange) {
+ var cd = new ChangeDetector(recordRange);
+ cd.detectChanges();
+ }
+
  it('should do simple watching', () => {
  var person = new Person("misko");
 
@@ -345,18 +356,45 @@ export function main() {
  }
  });
 
- describe("onGroupChange", () => {
+ describe("adding new ranges", () => {
+ var dispatcher;
+
+ beforeEach(() => {
+ dispatcher = new TestDispatcher();
+ });
+
+ /**
+ * Tests that we can add a new range after the current
+ * record has been disabled. The new range must be processed
+ * during the same change detection run.
+ */
+ it("should work when disabling the last enabled record", () => {
+ var rr = createRange(dispatcher, ast("1"), 1);
+
+ dispatcher.onChange = (group, _) => {
+ if (group === 1) { // to prevent infinite loop
+ var rangeToAppend = createRange(dispatcher, ast("2"), 2);
+ rr.addRange(rangeToAppend);
+ }
+ };
+
+ detectChangesInRange(rr);
+
+ expect(dispatcher.loggedValues).toEqual([[1], [2]]);
+ });
+ });
+
+ describe("group changes", () => {
  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 dispatcher = new TestDispatcher();
  var rr = prr.instantiate(dispatcher, null);
 
- var cd = new ChangeDetector(rr);
- cd.detectChanges();
+ detectChangesInRange(rr);
 
  expect(dispatcher.loggedValues).toEqual([[3, 30], [300]]);
  });
@@ -365,13 +403,12 @@ export function main() {
  var prr = new ProtoRecordRange();
  prr.addRecordsFromAST(ast("1 + 2"), "memo", "memo");
 
- var dispatcher = new LoggingDispatcher();
+ var dispatcher = new TestDispatcher();
  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();
+ detectChangesInRange(rr);
 
  expect(dispatcher.loggedValues).toEqual([[3], [3]]);
  });
@@ -382,7 +419,7 @@ export function main() {
  prr.addRecordsFromAST(ast("b()"), "b", 2);
  prr.addRecordsFromAST(ast("c()"), "b", 2);
 
- var dispatcher = new LoggingDispatcher();
+ var dispatcher = new TestDispatcher();
  var rr = prr.instantiate(dispatcher, null);
 
  var tr = new TestRecord();
@@ -391,8 +428,7 @@ export function main() {
  tr.c = () => {dispatcher.logValue('InvokeC'); return 'c'};
  rr.setContext(tr);
 
- var cd = new ChangeDetector(rr);
- cd.detectChanges();
+ detectChangesInRange(rr);
 
  expect(dispatcher.loggedValues).toEqual(['InvokeA', ['a'], 'InvokeB', 'InvokeC', ['b', 'c']]);
  });
@@ -444,21 +480,23 @@ class TestData {
  }
 }
 
-class LoggingDispatcher extends WatchGroupDispatcher {
+class TestDispatcher extends WatchGroupDispatcher {
  log:List;
  loggedValues:List;
+ onChange:Function;
 
  constructor() {
  this.log = null;
  this.loggedValues = null;
+ this.onChange = (_, __) => {};
  this.clear();
  }
 
  clear() {
  this.log = ListWrapper.create();
  this.loggedValues = ListWrapper.create();
  }
- 
+
  logValue(value) {
  ListWrapper.push(this.loggedValues, value);
  }
@@ -470,6 +508,8 @@ class LoggingDispatcher extends WatchGroupDispatcher {
 
  var values = ListWrapper.map(records, (r) => r.currentValue);
  ListWrapper.push(this.loggedValues, values);
+
+ this.onChange(group, records);
  }
 
  _asString(value) {