feat(change_detection): ensure that expression do not change after they have been checked

modules/change_detection/src/change_detector.js

@@ -6,14 +6,37 @@ import {ListWrapper, List} from 'facade/collection';
 export * from './record';
 export * from './record_range'
 
+class ExpressionChangedAfterItHasBeenChecked extends Error {
+  message:string;
+
+  constructor(record:Record) {
+    this.message = `Expression '${record.expressionAsString()}' has changed after it was checked. ` +
+      `Previous value: '${record.previousValue}'. Current value: '${record.currentValue}'`;
+  }
+
+  toString():string {
+    return this.message;
+  }
+}
+
 export class ChangeDetector {
   _rootRecordRange:RecordRange;
+  _enforceNoNewChanges:boolean;
 
-  constructor(recordRange:RecordRange) {
+  constructor(recordRange:RecordRange, enforceNoNewChanges:boolean = false) {
     this._rootRecordRange = recordRange;
+    this._enforceNoNewChanges = enforceNoNewChanges;
   }
 
   detectChanges():int {
+    var count = this._detectChanges(false);
+    if (this._enforceNoNewChanges) {
+      this._detectChanges(true)
+    }
+    return count;
+  }
+
+  _detectChanges(throwOnChange:boolean):int {
     var count = 0;
     var updatedRecords = null;
     var record = this._rootRecordRange.findFirstEnabledRecord();
@@ -23,6 +46,7 @@ export class ChangeDetector {
       if (record.check()) {
         count++;
         if (record.terminatesExpression()) {
+          if (throwOnChange) throw new ExpressionChangedAfterItHasBeenChecked(record);
           currentRange = record.recordRange;
           currentGroup = record.groupMemento();
           updatedRecords = this._addRecord(updatedRecords, record);

modules/change_detection/src/parser/ast.js

@@ -17,6 +17,10 @@ export class AST {
 
   visit(visitor, args) {
   }
+
+  toString():string {
+    return "AST";
+  }
 }
 
 export class EmptyExpr extends AST {
@@ -378,13 +382,33 @@ export class FunctionCall extends AST {
   }
 }
 
-export class ASTWithSource {
+export class ASTWithSource extends AST {
   ast:AST;
   source:string;
   constructor(ast:AST, source:string) {
     this.source = source;
     this.ast = ast;
   }
+
+  eval(context) {
+    return this.ast.eval(context);
+  }
+
+  get isAssignable() {
+    return this.ast.isAssignable;
+  }
+
+  assign(context, value) {
+    return this.ast.assign(context, value);
+  }
+
+  visit(visitor, args) {
+    return this.ast.visit(visitor, args);
+  }
+
+  toString():string {
+    return this.source;
+  }
 }
 
 export class TemplateBinding {

modules/change_detection/src/record.js

@@ -35,6 +35,7 @@ export class ProtoRecord {
   name:string;
   dest:any;
   groupMemento:any;
+  expressionAsString:string;
 
   next:ProtoRecord;
 
@@ -46,7 +47,8 @@ export class ProtoRecord {
               arity:int,
               name:string,
               dest,
-              groupMemento) {
+              groupMemento,
+              expressionAsString:string) {
 
     this.recordRange = recordRange;
     this._mode = mode;
@@ -55,6 +57,7 @@ export class ProtoRecord {
     this.name = name;
     this.dest = dest;
     this.groupMemento = groupMemento;
+    this.expressionAsString = expressionAsString;
 
     this.next = null;
     // The concrete Record instantiated from this ProtoRecord
@@ -341,6 +344,10 @@ export class Record {
     return this.protoRecord.dest;
   }
 
+  expressionAsString() {
+    return this.protoRecord.expressionAsString;
+  }
+
   groupMemento() {
     return isPresent(this.protoRecord) ? this.protoRecord.groupMemento : null;
   }

modules/change_detection/src/record_range.js

@@ -44,7 +44,7 @@ export class ProtoRecordRange {
   /**
    * Parses [ast] into [ProtoRecord]s and adds them to [ProtoRecordRange].
    *
-   * @param ast The expression to watch
+   * @param astWithSource The expression to watch
    * @param expressionMemento an opaque object which will be passed to ChangeDispatcher on
    *        detecting a change.
    * @param groupMemento
@@ -62,7 +62,6 @@ export class ProtoRecordRange {
     if (content) {
       ast = new Collection(ast);
     }
-
     this.recordCreator.createRecordsFromAST(ast, expressionMemento, groupMemento);
   }
 
@@ -318,11 +317,13 @@ class ProtoRecordCreator {
   headRecord:ProtoRecord;
   tailRecord:ProtoRecord;
   groupMemento:any;
+  expressionAsString:string;
 
   constructor(protoRecordRange) {
     this.protoRecordRange = protoRecordRange;
     this.headRecord = null;
     this.tailRecord = null;
+    this.expressionAsString = null;
   }
 
   visitImplicitReceiver(ast:ImplicitReceiver, args) {
@@ -434,11 +435,13 @@ class ProtoRecordCreator {
 
   createRecordsFromAST(ast:AST, expressionMemento:any, groupMemento:any){
     this.groupMemento = groupMemento;
+    this.expressionAsString = ast.toString();
     ast.visit(this, expressionMemento);
   }
 
   construct(recordType, funcOrValue, arity, name, dest) {
-    return new ProtoRecord(this.protoRecordRange, recordType, funcOrValue, arity, name, dest, this.groupMemento);
+    return new ProtoRecord(this.protoRecordRange, recordType, funcOrValue, arity,
+        name, dest, this.groupMemento, this.expressionAsString);
   }
 
   add(protoRecord:ProtoRecord) {

modules/change_detection/test/change_detector_spec.js

@@ -20,7 +20,7 @@ import {Record} from 'change_detection/record';
 export function main() {
   function ast(exp:string) {
     var parser = new Parser(new Lexer());
-    return parser.parseBinding(exp).ast;
+    return parser.parseBinding(exp);
   }
 
   function createChangeDetector(memo:string, exp:string, context = null, formatters = null,
@@ -433,6 +433,27 @@ export function main() {
           expect(dispatcher.loggedValues).toEqual(['InvokeA', ['a'], 'InvokeB', 'InvokeC', ['b', 'c']]);
         });
       });
+
+      describe("enforce no new changes", () => {
+        it("should throw when a record gets changed after it has been checked", () => {
+          var prr = new ProtoRecordRange();
+          prr.addRecordsFromAST(ast("a"), "a", 1);
+          prr.addRecordsFromAST(ast("b()"), "b", 2);
+
+          var tr = new TestRecord();
+          tr.a = "a";
+          tr.b = () => {tr.a = "newA";};
+
+          var dispatcher = new TestDispatcher();
+          var rr = prr.instantiate(dispatcher, null);
+          rr.setContext(tr);
+
+          expect(() => {
+            var cd = new ChangeDetector(rr, true);
+            cd.detectChanges();
+          }).toThrowError(new RegExp("Expression 'a' has changed after it was checked"));
+        });
+      });
     });
   });
 }
@@ -513,6 +534,6 @@ class TestDispatcher extends ChangeDispatcher {
   }
 
   _asString(value) {
-    return (value === null ? 'null' : value.toString());
+    return (isBlank(value) ? 'null' : value.toString());
   }
 }

modules/change_detection/test/record_range_spec.js

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

modules/core/src/application.js

@@ -1,5 +1,5 @@
 import {Injector, bind, OpaqueToken} from 'di/di';
-import {Type, FIELD, isBlank, isPresent, BaseException} from 'facade/lang';
+import {Type, FIELD, isBlank, isPresent, BaseException, assertionsEnabled} from 'facade/lang';
 import {DOM, Element} from 'facade/dom';
 import {Compiler, CompilerCache} from './compiler/compiler';
 import {ProtoView} from './compiler/view';
@@ -63,7 +63,7 @@ export function documentDependentBindings(appComponentType) {
       bind(appRecordRangeToken).toFactory((rootView) => rootView.recordRange,
           [appViewToken]),
       bind(ChangeDetector).toFactory((appRecordRange) =>
-          new ChangeDetector(appRecordRange), [appRecordRangeToken]),
+          new ChangeDetector(appRecordRange, assertionsEnabled()), [appRecordRangeToken]),
       bind(appComponentType).toFactory((rootView) => rootView.elementInjectors[0].getComponent(),
           [appViewToken])
   ];

modules/core/src/compiler/pipeline/element_binder_builder.js

@@ -68,19 +68,19 @@ export class ElementBinderBuilder extends CompileStep {
 
   _bindTextNodes(protoView, compileElement) {
     MapWrapper.forEach(compileElement.textNodeBindings, (expression, indexInParent) => {
-      protoView.bindTextNode(indexInParent, expression.ast);
+      protoView.bindTextNode(indexInParent, expression);
     });
   }
 
   _bindElementProperties(protoView, compileElement) {
     MapWrapper.forEach(compileElement.propertyBindings, (expression, property) => {
-      protoView.bindElementProperty(property,  expression.ast);
+      protoView.bindElementProperty(property,  expression);
     });
   }
 
   _bindEvents(protoView, compileElement) {
     MapWrapper.forEach(compileElement.eventBindings, (expression, eventName) => {
-      protoView.bindEvent(eventName,  expression.ast);
+      protoView.bindEvent(eventName,  expression);
     });
   }
 
@@ -118,7 +118,7 @@ export class ElementBinderBuilder extends CompileStep {
         }
         protoView.bindDirectiveProperty(
           directiveIndex++,
-          expression.ast,
+          expression,
           dirProp,
           reflector.setter(dirProp)
         );

modules/core/test/compiler/view_spec.js

@@ -105,7 +105,7 @@ export function main() {
           it('should collect property bindings on the root element if it has the ng-binding class', () => {
             var pv = new ProtoView(templateAwareCreateElement('<div [prop]="a" class="ng-binding"></div>'), new ProtoRecordRange());
             pv.bindElement(null);
-            pv.bindElementProperty('prop', parser.parseBinding('a').ast);
+            pv.bindElementProperty('prop', parser.parseBinding('a'));
 
             var view = pv.instantiate(null);
             view.hydrate(null, null, null);
@@ -117,7 +117,7 @@ export function main() {
             var pv = new ProtoView(templateAwareCreateElement('<div><span></span><span class="ng-binding"></span></div>'),
               new ProtoRecordRange());
             pv.bindElement(null);
-            pv.bindElementProperty('a', parser.parseBinding('b').ast);
+            pv.bindElementProperty('a', parser.parseBinding('b'));
 
             var view = pv.instantiate(null);
             view.hydrate(null, null, null);
@@ -132,8 +132,8 @@ export function main() {
           it('should collect text nodes under the root element', () => {
             var pv = new ProtoView(templateAwareCreateElement('<div class="ng-binding">{{}}<span></span>{{}}</div>'), new ProtoRecordRange());
             pv.bindElement(null);
-            pv.bindTextNode(0, parser.parseBinding('a').ast);
+            pv.bindTextNode(0, parser.parseBinding('a'));
-            pv.bindTextNode(2, parser.parseBinding('b').ast);
+            pv.bindTextNode(2, parser.parseBinding('b'));
 
             var view = pv.instantiate(null);
             view.hydrate(null, null, null);
@@ -146,7 +146,7 @@ export function main() {
             var pv = new ProtoView(templateAwareCreateElement('<div><span> </span><span class="ng-binding">{{}}</span></div>'),
               new ProtoRecordRange());
             pv.bindElement(null);
-            pv.bindTextNode(0, parser.parseBinding('b').ast);
+            pv.bindTextNode(0, parser.parseBinding('b'));
 
             var view = pv.instantiate(null);
             view.hydrate(null, null, null);
@@ -357,7 +357,7 @@ export function main() {
           var pv = new ProtoView(createElement('<div class="ng-binding">{{}}</div>'),
             new ProtoRecordRange());
           pv.bindElement(null);
-          pv.bindTextNode(0, parser.parseBinding('foo').ast);
+          pv.bindTextNode(0, parser.parseBinding('foo'));
           createViewAndChangeDetector(pv);
 
           ctx.foo = 'buz';
@@ -369,7 +369,7 @@ export function main() {
           var pv = new ProtoView(createElement('<div class="ng-binding"></div>'),
             new ProtoRecordRange());
           pv.bindElement(null);
-          pv.bindElementProperty('id', parser.parseBinding('foo').ast);
+          pv.bindElementProperty('id', parser.parseBinding('foo'));
           createViewAndChangeDetector(pv);
 
           ctx.foo = 'buz';
@@ -381,7 +381,7 @@ export function main() {
           var pv = new ProtoView(createElement('<div class="ng-binding"></div>'),
             new ProtoRecordRange());
           pv.bindElement(new ProtoElementInjector(null, 0, [SomeDirective]));
-          pv.bindDirectiveProperty(0, parser.parseBinding('foo').ast, 'prop', reflector.setter('prop'));
+          pv.bindDirectiveProperty(0, parser.parseBinding('foo'), 'prop', reflector.setter('prop'));
           createViewAndChangeDetector(pv);
 
           ctx.foo = 'buz';
@@ -394,8 +394,8 @@ export function main() {
             new ProtoRecordRange());
 
           pv.bindElement(new ProtoElementInjector(null, 0, [DirectiveImplementingOnChange]));
-          pv.bindDirectiveProperty( 0, parser.parseBinding('a').ast, 'a', reflector.setter('a'));
+          pv.bindDirectiveProperty( 0, parser.parseBinding('a'), 'a', reflector.setter('a'));
-          pv.bindDirectiveProperty( 0, parser.parseBinding('b').ast, 'b', reflector.setter('b'));
+          pv.bindDirectiveProperty( 0, parser.parseBinding('b'), 'b', reflector.setter('b'));
           createViewAndChangeDetector(pv);
 
           ctx.a = 100;
@@ -413,8 +413,7 @@ export function main() {
           pv.bindElement(new ProtoElementInjector(null, 0, [DirectiveImplementingOnChange]));
           pv.bindDirectiveProperty( 0, parser.parseBinding('a').ast, 'a', reflector.setter('a'));
           pv.bindDirectiveProperty( 0, parser.parseBinding('b').ast, 'b', reflector.setter('b'));
-          createViewAndChangeDetector(pv);
+          createView(pv);
-
           ctx.a = 0;
           ctx.b = 0;
           cd.detectChanges();

modules/facade/src/lang.dart

@@ -169,3 +169,12 @@ bool isJsObject(o) {
   return false;
 }
 
+
+assertionsEnabled() {
+  try {
+    assert(false);
+    return false;
+  } catch (e) {
+    return true;
+  }
+}

modules/facade/src/lang.es6

@@ -206,3 +206,12 @@ export function normalizeBlank(obj) {
 export function isJsObject(o):boolean {
   return o !== null && (typeof o === "function" || typeof o === "object");
 }
+
+export function assertionsEnabled() {
+  try {
+    var x:int = "string";
+    return false;
+  } catch (e) {
+    return true;
+  }
+}