fix($parse): always pass the intercepted value to watchers

jbedard · jbedard · commit 2ee5033967d5 · 2017-07-16T22:01:40.000-07:00
Fixes angular#16021
diff --git a/src/ng/directive/ngClass.js b/src/ng/directive/ngClass.js
@@ -91,12 +91,6 @@ function classDirective(name, selector) {
         }
 
         function ngClassWatchAction(newClassString) {
-          // When using a one-time binding the newClassString will return
-          // the pre-interceptor value until the one-time is complete
-          if (!isString(newClassString)) {
-            newClassString = toClassString(newClassString);
-          }
-
           if (oldModulo === selector) {
             updateClasses(oldClassString, newClassString);
           }
diff --git a/src/ng/parse.js b/src/ng/parse.js
@@ -1884,28 +1884,37 @@ function $ParseProvider() {
     function oneTimeWatchDelegate(scope, listener, objectEquality, parsedExpression, prettyPrintExpression) {
       var isDone = parsedExpression.literal ? isAllDefined : isDefined;
       var unwatch, lastValue;
-      if (parsedExpression.inputs) {
-        unwatch = inputsWatchDelegate(scope, oneTimeListener, objectEquality, parsedExpression, prettyPrintExpression);
-      } else {
-        unwatch = scope.$watch(oneTimeWatch, oneTimeListener, objectEquality);
-      }
+
+      var exp = parsedExpression.$$intercepted || parsedExpression;
+      var post = parsedExpression.$$interceptor || identity;
+
+      var useInputs = parsedExpression.inputs && !exp.inputs;
+
+      // Propogate the literal/inputs/constant attributes
+      // ... but not oneTime since we are handling it
+      oneTimeWatch.literal = parsedExpression.literal;
+      oneTimeWatch.constant = parsedExpression.constant;
+      oneTimeWatch.inputs = parsedExpression.inputs;
+
+      // Allow other delegates to run on this wrapped expression
+      addWatchDelegate(oneTimeWatch);
+
+      unwatch = scope.$watch(oneTimeWatch, listener, objectEquality, prettyPrintExpression);
+
       return unwatch;
 
-      function oneTimeWatch(scope) {
-        return parsedExpression(scope);
-      }
-      function oneTimeListener(value, old, scope) {
-        lastValue = value;
-        if (isFunction(listener)) {
-          listener(value, old, scope);
+      function unwatchIfDone() {
+        if (isDone(lastValue)) {
+          unwatch();
         }
-        if (isDone(value)) {
-          scope.$$postDigest(function() {
-            if (isDone(lastValue)) {
-              unwatch();
-            }
-          });
+      }
+
+      function oneTimeWatch(scope, locals, assign, inputs) {
+        lastValue = useInputs && inputs ? inputs[0] : exp(scope, locals, assign, inputs);
+        if (isDone(lastValue)) {
+          scope.$$postDigest(unwatchIfDone);
         }
+        return post(lastValue, scope, locals);
       }
     }
 
@@ -1937,27 +1946,35 @@ function $ParseProvider() {
       return parsedExpression;
     }
 
+    function chainInterceptors(first, second) {
+      function chainedInterceptor(value) {
+        return second(first(value));
+      }
+      chainedInterceptor.$stateful = first.$stateful || second.$stateful;
+
+      return chainedInterceptor;
+    }
+
     function addInterceptor(parsedExpression, interceptorFn) {
       if (!interceptorFn) return parsedExpression;
 
-      var useInputs = false;
+      // Extract any existing interceptors out of the parsedExpression
+      // to ensure the original parsedExpression is always the $$intercepted
+      if (parsedExpression.$$interceptor) {
+        interceptorFn = chainInterceptors(parsedExpression.$$interceptor, interceptorFn);
+        parsedExpression = parsedExpression.$$intercepted;
+      }
 
-      var isDone = parsedExpression.literal ? isAllDefined : isDefined;
+      var useInputs = false;
 
-      function regularInterceptedExpression(scope, locals, assign, inputs) {
+      var fn = function interceptedExpression(scope, locals, assign, inputs) {
         var value = useInputs && inputs ? inputs[0] : parsedExpression(scope, locals, assign, inputs);
         return interceptorFn(value);
-      }
-
-      function oneTimeInterceptedExpression(scope, locals, assign, inputs) {
-        var value = useInputs && inputs ? inputs[0] : parsedExpression(scope, locals, assign, inputs);
-        var result = interceptorFn(value);
-        // we only return the interceptor's result if the
-        // initial value is defined (for bind-once)
-        return isDone(value) ? result : value;
-      }
+      };
 
-      var fn = parsedExpression.oneTime ? oneTimeInterceptedExpression : regularInterceptedExpression;
+      // Maintain references to the interceptor/intercepted
+      fn.$$intercepted = parsedExpression;
+      fn.$$interceptor = interceptorFn;
 
       // Propogate the literal/oneTime/constant attributes
       fn.literal = parsedExpression.literal;
diff --git a/test/ng/interpolateSpec.js b/test/ng/interpolateSpec.js
@@ -149,6 +149,22 @@ describe('$interpolate', function() {
       expect($rootScope.$countWatchers()).toBe(0);
     }));
 
+    it('should respect one-time bindings for literals', inject(function($interpolate, $rootScope) {
+      var calls = [];
+      $rootScope.$watch($interpolate('{{ ::{x: x} }}'), function(val) {
+        calls.push(val);
+      });
+
+      $rootScope.$apply();
+      expect(calls.pop()).toBe('{}');
+
+      $rootScope.$apply('x = 1');
+      expect(calls.pop()).toBe('{"x":1}');
+
+      $rootScope.$apply('x = 2');
+      expect(calls.pop()).toBeUndefined();
+    }));
+
     it('should stop watching strings with no expressions after first execution',
       inject(function($interpolate, $rootScope) {
         var spy = jasmine.createSpy();
diff --git a/test/ng/parseSpec.js b/test/ng/parseSpec.js
@@ -3380,13 +3380,12 @@ describe('parser', function() {
 
             scope.$watch($parse('::[a]', interceptor));
 
-            // Would be great if interceptor-output was checked for changes and this didn't throw...
             interceptorCalls = 0;
-            expect(function() { scope.$digest(); }).toThrowMinErr('$rootScope', 'infdig');
+            scope.$digest();
             expect(interceptorCalls).not.toBe(0);
 
             interceptorCalls = 0;
-            expect(function() { scope.$digest(); }).toThrowMinErr('$rootScope', 'infdig');
+            scope.$digest();
             expect(interceptorCalls).not.toBe(0);
           }));
 
@@ -3501,6 +3500,57 @@ describe('parser', function() {
             expect(scope.$$watchersCount).toBe(0);
           }));
 
+          it('should watch the intercepted value of one-time bindings', inject(function($parse, log) {
+            scope.$watch($parse('::{x:x, y:y}', function(lit) { return lit.x; }), log);
+
+            scope.$apply();
+            expect(log.empty()).toEqual([undefined]);
+
+            scope.$apply('x = 1');
+            expect(log.empty()).toEqual([1]);
+
+            scope.$apply('x = 2; y=1');
+            expect(log.empty()).toEqual([2]);
+
+            scope.$apply('x = 1; y=2');
+            expect(log.empty()).toEqual([]);
+          }));
+
+          it('should watch the intercepted value of one-time bindings in nested interceptors', inject(function($parse, log) {
+            scope.$watch($parse($parse('::{x:x, y:y}', function(lit) { return lit.x; }), identity), log);
+
+            scope.$apply();
+            expect(log.empty()).toEqual([undefined]);
+
+            scope.$apply('x = 1');
+            expect(log.empty()).toEqual([1]);
+
+            scope.$apply('x = 2; y=1');
+            expect(log.empty()).toEqual([2]);
+
+            scope.$apply('x = 1; y=2');
+            expect(log.empty()).toEqual([]);
+          }));
+
+          it('should nest interceptors around eachother, not around the intercepted', inject(function($parse) {
+            function origin() { return 0; }
+
+            var fn = origin;
+            function addOne(n) { return n + 1; }
+
+            fn = $parse(fn, addOne);
+            expect(fn.$$intercepted).toBe(origin);
+            expect(fn()).toBe(1);
+
+            fn = $parse(fn, addOne);
+            expect(fn.$$intercepted).toBe(origin);
+            expect(fn()).toBe(2);
+
+            fn = $parse(fn, addOne);
+            expect(fn.$$intercepted).toBe(origin);
+            expect(fn()).toBe(3);
+          }));
+
           it('should not propogate $$watchDelegate to the interceptor wrapped expression', inject(function($parse) {
             function getter(s) {
               return s.x;

Original file line number	Diff line number	Diff line change
`@@ -91,12 +91,6 @@ function classDirective(name, selector) {`
`91`	`91`	`}`
`92`	`92`
`93`	`93`	`function ngClassWatchAction(newClassString) {`
`94`		`- // When using a one-time binding the newClassString will return`
`95`		`- // the pre-interceptor value until the one-time is complete`
`96`		`- if (!isString(newClassString)) {`
`97`		`- newClassString = toClassString(newClassString);`
`98`		`- }`
`99`		`-`
`100`	`94`	`if (oldModulo === selector) {`
`101`	`95`	`updateClasses(oldClassString, newClassString);`
`102`	`96`	`}`