fixes #174 -- minimize calls to discardChanges for if/bind/repeat

src/TemplateBinding.js

@@ -575,7 +575,7 @@
 
       this.refContent_ = undefined;
       this.iterator_.valueChanged();
-      this.iterator_.updateIteratedValue();
+      this.iterator_.updateIteratedValue(this.iterator_.getUpdatedValue());
     },
 
     clear: function() {
@@ -979,19 +979,22 @@
       var deps = this.deps = {};
       var template = this.templateElement_;
 
+      var ifValue = true;
       if (directives.if) {
         deps.hasIf = true;
         deps.ifOneTime = directives.if.onlyOneTime;
         deps.ifValue = processBinding(IF, directives.if, template, model);
 
+        ifValue = deps.ifValue;
+
         // oneTime if & predicate is false. nothing else to do.
-        if (deps.ifOneTime && !deps.ifValue) {
-          this.updateIteratedValue();
+        if (deps.ifOneTime && !ifValue) {
+          this.valueChanged();
           return;
         }
 
         if (!deps.ifOneTime)
-          deps.ifValue.open(this.updateIteratedValue, this);
+          ifValue = ifValue.open(this.updateIfValue, this);
       }
 
       if (directives.repeat) {
@@ -1004,13 +1007,40 @@
         deps.value = processBinding(BIND, directives.bind, template, model);
       }
 
+      var value = deps.value;
       if (!deps.oneTime)
-        deps.value.open(this.updateIteratedValue, this);
+        value = value.open(this.updateIteratedValue, this);
+
+      if (!ifValue) {
+        this.valueChanged();
+        return;
+      }
 
-      this.updateIteratedValue();
+      this.updateValue(value);
     },
 
-    updateIteratedValue: function() {
+    /**
+     * Gets the updated value of the bind/repeat. This can potentially call
+     * user code (if a bindingDelegate is set up) so we try to avoid it if we
+     * already have the value in hand (from Observer.open).
+     */
+    getUpdatedValue: function() {
+      var value = this.deps.value;
+      if (!this.deps.oneTime)
+        value = value.discardChanges();
+      return value;
+    },
+
+    updateIfValue: function(ifValue) {
+      if (!ifValue) {
+        this.valueChanged();
+        return;
+      }
+
+      this.updateValue(this.getUpdatedValue());
+    },
+
+    updateIteratedValue: function(value) {
       if (this.deps.hasIf) {
         var ifValue = this.deps.ifValue;
         if (!this.deps.ifOneTime)
@@ -1021,9 +1051,10 @@
         }
       }
 
-      var value = this.deps.value;
-      if (!this.deps.oneTime)
-        value = value.discardChanges();
+      this.updateValue(value);
+    },
+
+    updateValue: function(value) {
       if (!this.deps.repeat)
         value = [value];
       var observe = this.deps.repeat &&

tests/tests.js

@@ -430,6 +430,77 @@ suite('Template Instantiation', function() {
     });
   });
 
+  test('Bind If minimal discardChanges', function(done) {
+
+    var div = createTestHtml(
+        '<template bind="{{ bound }}" if="{{ predicate }}">' +
+          'value:{{ value }}' +
+        '</template>');
+    var m = { bound: null, predicate: 0 };
+    var template = div.firstChild;
+
+    var discardChangesCalled = { bound: 0, predicate: 0 };
+    template.bindingDelegate = {
+      prepareBinding: function(path, name, node) {
+        return function(model, node, oneTime) {
+          var result = new PathObserver(model, path); 
+          result.discardChanges = function() {
+            discardChangesCalled[path]++;
+            return PathObserver.prototype.discardChanges.call(this);
+          }
+          return result;
+        }
+      }
+    };
+
+    template.model = m;
+
+    then(function() {
+      assert.strictEqual(0, discardChangesCalled.bound);
+      assert.strictEqual(0, discardChangesCalled.predicate);
+
+      assert.strictEqual(1, div.childNodes.length);
+
+      m.predicate = 1;
+
+    }).then(function() {
+      assert.strictEqual(1, discardChangesCalled.bound);
+      assert.strictEqual(0, discardChangesCalled.predicate);
+
+      assert.strictEqual(2, div.childNodes.length);
+      assert.strictEqual('value:', div.lastChild.textContent);
+
+      m.bound = { value: 2 };
+
+    }).then(function() {
+      assert.strictEqual(1, discardChangesCalled.bound);
+      assert.strictEqual(1, discardChangesCalled.predicate);
+
+      assert.strictEqual(2, div.childNodes.length);
+      assert.strictEqual('value:2', div.lastChild.textContent);
+
+      m.bound.value = 3;
+
+    }).then(function() {
+      assert.strictEqual(1, discardChangesCalled.bound);
+      assert.strictEqual(1, discardChangesCalled.predicate);
+
+      assert.strictEqual(2, div.childNodes.length);
+      assert.strictEqual('value:3', div.lastChild.textContent);
+
+      template.model = undefined;
+
+    }).then(function() {
+      assert.strictEqual(1, discardChangesCalled.bound);
+      assert.strictEqual(1, discardChangesCalled.predicate);
+
+      assert.strictEqual(1, div.childNodes.length);
+
+      done();
+    });
+
+  });
+
   test('Empty If', function(done) {
     var div = createTestHtml(
         '<template if>{{ value }}</template>');