Merge pull request #14 from scalyr/czerwin-include-fix

czerwingithub · czerwingithub · commit 3f1cbbba3168 · 2014-09-29T10:36:42.000-07:00
Change slyEvaluateOnlyWhen to evaluate newly created children
diff --git a/src/js/directives/slyEvaluate.js b/src/js/directives/slyEvaluate.js
@@ -8,9 +8,10 @@
  *
  *  slyEvaluateOnlyWhen:  A directive that prevents updating / evaluating
  *      all bindings for the current element and its children unless
- *      the expression has changed values.  It currently assumes the
+ *      the expression has changed values.  If new children are added, they
+ *      are always evaluated at least once.  It currently assumes the
  *      expression evaluates to an object and detects changes only by
- *      a change in object reference.
+ *      a change in object reference.  
  *
  *  slyAlwaysEvaluate: Can only be used in conjunction with the
  *      slyEvaluateOnlyWhen directive.  This directive will ensure that
@@ -75,7 +76,7 @@ defineScalyrAngularModule('slyEvaluate', ['gatedScope'])
             // be gated.
             return isNull(alwaysEvaluateString) || 
                    !(isStringNonempty(watchExpression) && (watchExpression.indexOf(alwaysEvaluateString) >= 0));
-          });
+          }, true /* Evaluate any newly added watchers when they are added */);
         },
       };
     },
diff --git a/src/js/lib/gatedScope.js b/src/js/lib/gatedScope.js
@@ -52,6 +52,7 @@ defineScalyrAngularModule('gatedScope', [])
       result.$$parentGatingFunction = this.$$gatingFunction;
       result.$$shouldGateFunction = this.$$shouldGateFunction;
       result.$$gatedWatchers = [];
+      result.$$cleanUpQueue = this.$$cleanUpQueue;
 
       return result;
     };
@@ -83,6 +84,14 @@ defineScalyrAngularModule('gatedScope', [])
               if (watch.gatingFunction !== targetGatingFunction)
                 continue;
 
+              // Since we are about to execute the watcher as part of a digestGated
+              // call, we can remove it from the normal digest queue if it was placed
+              // there because the watcher was added after the gate function's first
+              // evaluation.
+              if (watch && !isNull(watch.cleanUp)) {
+                watch.cleanUp();
+                watch.cleanUp = null;
+              }
               // Most common watches are on primitives, in which case we can short
               // circuit it with === operator, only when === fails do we use .equals
               if (watch && (value = watch.get(current)) !== (last = watch.last) &&
@@ -117,6 +126,8 @@ defineScalyrAngularModule('gatedScope', [])
         }
       } while ((current = next));
 
+      // Mark that this gating function has digested all children.
+      targetGatingFunction.hasDigested = true;
       return dirty;
     };
 
@@ -141,11 +152,38 @@ defineScalyrAngularModule('gatedScope', [])
         var result = scopePrototype.$watch.call(this, watchExpression, listener, objectEquality);
         this.$$watchers = tmp;
         this.$$gatedWatchers[0].gatingFunction = this.$$gatingFunction;
+        this.$$gatedWatchers[0].cleanUp = null;
 
         // We know that the last field of the watcher object will be set to initWatchVal, so we
         // grab it here.
         initWatchVal = this.$$gatedWatchers[0].last;
+        var watch = this.$$gatedWatchers[0];
+
+        // We should make sure the watch expression gets evaluated fully on at least one
+        // digest cycle even if the gate function is now closed if requested by the gating function's
+        // value for shouldEvalNewWatchers.  We do this by adding in normal watcher that will execute
+        // the watcher we just added and remove itself after the digest cycle completes.
+        if (this.$$gatingFunction.shouldEvalNewWatchers && this.$$gatingFunction.hasDigested) {
+          var self = this;
+          watch.cleanUp = scopePrototype.$watch.call(self, function() {
+            if (!isNull(watch.cleanUp)) {
+              self.$$cleanUpQueue.unshift(watch.cleanUp);
+              watch.cleanUp = null;
+            }
+            var value;
+            var last = initWatchVal;
 
+            if (watch && (value = watch.get(self)) !== (last = watch.last) &&
+                  !(watch.eq
+                      ? areEqual(value, last)
+                      : (typeof value == 'number' && typeof last == 'number'
+                        && isNaN(value) && isNaN(last)))) {
+                watch.last = watch.eq ? copy(value) : value;
+                watch.fn(value, ((last === initWatchVal) ? value : last), self);
+             }
+            return watch.last;
+          });
+        }
         return result;
       } else {
         return scopePrototype.$watch.call(this, watchExpression, listener, objectEquality);
@@ -168,8 +206,8 @@ defineScalyrAngularModule('gatedScope', [])
       // functions and should be evaluated at all.  However, if a caller is invoking
       // $digest on a particular scope, we assume the caller is doing that because it
       // knows the watchers should be evaluated.
+      var dirty = false;
       if (!isNull(this.$$parentGatingFunction) && this.$$parentGatingFunction()) {
-        var dirty = false;
         var ttl = 5;
         do {
           dirty = this.$digestGated(this.$$parentGatingFunction);
@@ -181,7 +219,19 @@ defineScalyrAngularModule('gatedScope', [])
           }
         } while (dirty);
       }
-      return scopePrototype.$digest.call(this) || dirty;
+
+      dirty = scopePrototype.$digest.call(this) || dirty;
+      
+      var cleanUpQueue = this.$$cleanUpQueue;
+
+      while (cleanUpQueue.length)
+        try {
+          cleanUpQueue.shift()();
+        } catch (e) {
+          $exceptionHandler(e);
+        }
+
+      return dirty;
     }
 
     /**
@@ -198,8 +248,13 @@ defineScalyrAngularModule('gatedScope', [])
      *   a new watcher will be gated using gatingFunction.  It is evaluated with the
      *   arguments to $watch and should return true if the watcher created by those
      *   arguments should be gated
+     * @param {Boolean} shouldEvalNewWatchers If true, if a watcher is added
+     *   after the gating function has returned true on a previous digest cycle, the
+     *   the new watcher will be evaluated on the next digest cycle even if the
+     *   gating function is currently return false.
      */
-    methodsToAdd.$addWatcherGate = function(gatingFunction, shouldGateFunction) {
+    methodsToAdd.$addWatcherGate = function(gatingFunction, shouldGateFunction,
+                                            shouldEvalNewWatchers) {
       var changeCount = 0;
       var self = this;
 
@@ -215,30 +270,36 @@ defineScalyrAngularModule('gatedScope', [])
       // true (which we can tell if the watcher we register here is evaluated), then
       // we always evaluate our watcher until our gating function returns true.
       var hasNestedGates = !isNull(this.$$gatingFunction);
-      var promotedWatcher = null;
-
-      this.$watch(function() {
-        if (gatingFunction()) {
-          if (self.$digestGated(gatingFunction))
-            ++changeCount;
-        } else if (hasNestedGates && isNull(promotedWatcher)) {
-          promotedWatcher = scopePrototype.$watch.call(self, function() {
-            if (gatingFunction()) {
-              promotedWatcher();
-              promotedWatcher = null;
-              if (self.$digestGated(gatingFunction))
-                ++changeCount;
-            }
-            return changeCount;
-          });
-        }
-        return changeCount;
-      });
+
+      (function() {
+        var promotedWatcher = null;
+
+        self.$watch(function() {
+          if (gatingFunction()) {
+            if (self.$digestGated(gatingFunction))
+              ++changeCount;
+          } else if (hasNestedGates && isNull(promotedWatcher)) {
+            promotedWatcher = scopePrototype.$watch.call(self, function() {
+              if (gatingFunction()) {
+                promotedWatcher();
+                promotedWatcher = null;
+                if (self.$digestGated(gatingFunction))
+                  ++changeCount;
+              }
+              return changeCount;
+            });
+          }
+          return changeCount;
+        });
+      })();
 
 
       if (isUndefined(shouldGateFunction))
         shouldGateFunction = null;
+      if (isUndefined(shouldEvalNewWatchers))
+        shouldEvalNewWatchers = false;
       this.$$gatingFunction = gatingFunction;
+      this.$$gatingFunction.shouldEvalNewWatchers = shouldEvalNewWatchers;
       this.$$shouldGateFunction = shouldGateFunction;
     };
 
@@ -254,6 +315,7 @@ defineScalyrAngularModule('gatedScope', [])
     $rootScope.$$parentGatingFunction = null;
     $rootScope.$$shouldGateFunction = null;
     $rootScope.$$gatedWatchers = [];
+    $rootScope.$$cleanUpQueue = [];
 
     return $rootScope;
   }]);
diff --git a/src/tests/directives/slyEvaluateTest.js b/src/tests/directives/slyEvaluateTest.js
@@ -69,6 +69,30 @@ describe('slyEvaluate.slyEvaluateOnlyWhen', function() {
 
     expect(span.eq(0).text()).toEqual('12');
   });
+
+  it('should evaluate a new child no matter what', 
+     inject(function($rootScope, $compile) {
+       scope = $rootScope;
+       scope.dataObject = {
+         value: 5,
+       };
+       scope.x = 12;
+      
+      page = angular.element('<div sly-evaluate-only-when="dataObject"></div>');
+
+      $compile(page)(scope);
+      scope.$digest();
+
+      // We simulate adding a new child to the div by just creating a new element
+      // and compiling it in.  This is a bit hacky and hopefully won't break.
+      divScope = page.scope();
+      span = angular.element('<span>{{x}}</span>');
+      $compile(span)(divScope);
+
+      scope.$digest();
+
+      expect(span.eq(0).text()).toEqual('12');
+  }));
 });
 
 describe('slyEvaluate.slyAlwaysEvaluate', function() {
diff --git a/src/tests/lib/gatedScopeTest.js b/src/tests/lib/gatedScopeTest.js
@@ -312,4 +312,55 @@ describe('GatedScope', function() {
     // When both gates are down, the watcher should not be evaluated.
     $rootScope.$digest();
   });
+
+  it('should evaluate new watchers when gating function has shouldEvalNewWatchers = true', function() {
+    var gateClosed = false;
+
+    var child = $rootScope.$new();
+    child.$addWatcherGate(function() {
+      return !gateClosed;
+    }, null, true);
+
+    $rootScope.$digest();
+
+    gateClosed = true;
+
+    var counter = 0;
+    var watchedVal = 1;
+    function watcher() {
+      ++counter;
+      return watchedVal;
+    }
+
+    child.$watch(watcher);
+    $rootScope.$digest();
+
+    // Should have been evaluated twice, one for the first dirty cycle, and then for
+    // cycle it was not dirty on.
+    expect(counter).toEqual(2);
+  });
+
+  it('should not evaluate new watchers when gating function has shouldEvalNewWatchers = false', function() {
+    var gateClosed = false;
+
+    var child = $rootScope.$new();
+    child.$addWatcherGate(function() {
+      return !gateClosed;
+    }, null, false);
+
+    $rootScope.$digest();
+
+    gateClosed = true;
+    var counter = 0;
+    var watchedVal = 1;
+    function watcher() {
+      ++counter;
+      return watchedVal;
+    }
+
+    child.$watch(watcher);
+    $rootScope.$digest();
+
+    expect(counter).toEqual(0);
+  });
 });
