From 014d5f83f8421be17d271fb40ee1fa990ac3abb1 Mon Sep 17 00:00:00 2001
From: James deBoer <james@huronbox.com>
Date: Wed, 7 May 2014 14:09:18 -0700
Subject: [PATCH] fix(compiler): OneWayOneTime bindings now wait for the model
 to stablize

BREAKING CHANGE

Previously, OneWayOneTime bindings would remove their watch after the
first value assignment.  For models that take more than one digest loop
to stablize, this meant the OneWayOneTime bindings took a un-stablized
value.

With this change, these bindings will continue to accept value
assignments until their stablized value is non-null. The assignment may
occur multiple times.

We expect that most uses of OneWayOneTime will be 'ok' with
being called a few times as the model stablizes.

However, applications which depend on OneWayOneTime bindings being
called exactly once will break.  The suggested upgrade path is to
move the senstive code into a domWrite callback. e.g. the old code:

    @Decorator(
        map: const { 'one-time': '=>!value' })
    class OneTime {
      set value(v) => onlyCalledOnce(v);
    }

becomes

    @Decorator(
        map: const { 'one-time': '=>!value' })
    class OneTime {
      var _value;
      set value(v) {
        if (_value == null) {
	  scope.rootScope.domWrite(() => onlyCalledOnce(_value));
	}
      	_value = v;
      }
    }
---
 lib/core_dom/element_binder.dart | 13 +++++-
 test/core_dom/compiler_spec.dart | 72 +++++++++++++++++++++++++++++++-
 2 files changed, 82 insertions(+), 3 deletions(-)

diff --git a/lib/core_dom/element_binder.dart b/lib/core_dom/element_binder.dart
index bc562c675..222b20e23 100644
--- a/lib/core_dom/element_binder.dart
+++ b/lib/core_dom/element_binder.dart
@@ -176,9 +176,18 @@ class ElementBinder {
 
           Expression attrExprFn = _parser(nodeAttrs[attrName]);
           var watch;
+          var lastOneTimeValue;
           watch = scope.watch(nodeAttrs[attrName], (value, _) {
-            if (dstPathFn.assign(controller, value) != null) {
-              watch.remove();
+            if ((lastOneTimeValue = dstPathFn.assign(controller, value)) != null && watch != null) {
+                var watchToRemove = watch;
+                watch = null;
+                scope.rootScope.domWrite(() {
+                  if (lastOneTimeValue != null) {
+                    watchToRemove.remove();
+                  } else {  // It was set to non-null, but stablized to null, wait.
+                    watch = watchToRemove;
+                  }
+                });
             }
           }, formatters: formatters);
           break;
diff --git a/test/core_dom/compiler_spec.dart b/test/core_dom/compiler_spec.dart
index 750ed46c5..c8e48cd28 100644
--- a/test/core_dom/compiler_spec.dart
+++ b/test/core_dom/compiler_spec.dart
@@ -269,7 +269,8 @@ void main() {
           ..bind(SometimesComponent)
           ..bind(ExprAttrComponent)
           ..bind(LogElementComponent)
-          ..bind(SayHelloFormatter);
+          ..bind(SayHelloFormatter)
+          ..bind(OneTimeDecorator);
       });
 
       it('should select on element', async(() {
@@ -690,6 +691,64 @@ void main() {
           }
         }));
       });
+
+      describe('bindings', () {
+        it('should set a one-time binding with the correct value', (Logger logger) {
+          _.compile(r'<div one-time="v"></div>');
+
+          _.rootScope.context['v'] = 1;
+
+          var context = _.rootScope.context;
+          _.rootScope.watch('3+4', (v, _) => context['v'] = v);
+
+          // In the 1st digest iteration:
+          //   v will be set to 7
+          //   OneTimeDecorator.value will be set to 1
+          // In the 2nd digest iteration:
+          //   OneTimeDecorator.value will be set to 7
+          _.rootScope.apply();
+
+          expect(logger).toEqual([1, 7]);
+        });
+
+        it('should keep one-time binding until it is set to non-null', (Logger logger) {
+          _.compile(r'<div one-time="v"></div>');
+          _.rootScope.context['v'] = null;
+          _.rootScope.apply();
+          expect(logger).toEqual([null]);
+
+          _.rootScope.context['v'] = 7;
+          _.rootScope.apply();
+          expect(logger).toEqual([null, 7]);
+
+          // Check that the binding is removed.
+          _.rootScope.context['v'] = 8;
+          _.rootScope.apply();
+          expect(logger).toEqual([null, 7]);
+        });
+
+        it('should remove the one-time binding only if it stablizied to null', (Logger logger) {
+          _.compile(r'<div one-time="v"></div>');
+
+          _.rootScope.context['v'] = 1;
+
+          var context = _.rootScope.context;
+          _.rootScope.watch('3+4', (v, _) => context['v'] = null);
+
+          _.rootScope.apply();
+          expect(logger).toEqual([1, null]);
+
+          // Even though there was a null in the unstable model, we shouldn't remove the binding
+          context['v'] = 8;
+          _.rootScope.apply();
+           expect(logger).toEqual([1, null, 8]);
+
+          // Check that the binding is removed.
+          _.rootScope.context['v'] = 9;
+          _.rootScope.apply();
+          expect(logger).toEqual([1, null, 8]);
+        });
+      });
     });
 
 
@@ -1130,3 +1189,14 @@ class LogElementComponent{
     logger(shadowRoot);
   }
 }
+
+@Decorator(
+    selector: '[one-time]',
+    map: const {
+      'one-time': '=>!value'
+})
+class OneTimeDecorator {
+  Logger log;
+  OneTimeDecorator(this.log);
+  set value(v) => log(v);
+}