refactor(scope.$watch): rearrange arguments passed into watcher (newV…

…alue, oldValue, scope) As scopes are injected into controllers now, you have the reference anyway, so having scope as first argument makes no sense… Breaks $watcher gets arguments in different order (newValue, oldValue, scope)
angular · Jan 23, 2012 · 0196411 · 0196411
1 parent 992c790
commit 0196411
Show file tree

Hide file tree

Showing 14 changed files with 61 additions and 47 deletions.
diff --git a/docs/content/cookbook/mvc.ngdoc b/docs/content/cookbook/mvc.ngdoc
@@ -66,7 +66,7 @@ no connection between the controller and the view.
         function same(a, b, c) { return (a==b && b==c) ? a : '';};
       }
 
-      function readUrl(scope, value) {
+      function readUrl(value) {
         if (value) {
           value = value.split('/');
           $scope.nextMove = value[1];

diff --git a/docs/content/guide/dev_guide.scopes.internals.ngdoc b/docs/content/guide/dev_guide.scopes.internals.ngdoc
@@ -187,7 +187,7 @@ within the unit-tests.
   // example of a test
   it('should trigger a watcher', inject(function($rootScope) {
     var scope = $rootScope;
-    scope.$watch('name', function(scope, name){
+    scope.$watch('name', function(name) {
      scope.greeting = 'Hello ' + name + '!';
     });
 

diff --git a/docs/content/guide/dev_guide.services.$location.ngdoc b/docs/content/guide/dev_guide.services.$location.ngdoc
@@ -618,11 +618,11 @@ you will need to specify an extra property that has two watchers. For example:
 </pre>
 <pre>
 // js - controller
-this.$watch('locationPath', function(scope, path) {
+this.$watch('locationPath', function(path) {
   $location.path(path);
 });
 
-this.$watch('$location.path()', function(scope, path) {
+this.$watch('$location.path()', function(path) {
   scope.locationPath = path;
 });
 </pre>

diff --git a/docs/src/templates/docs.js b/docs/src/templates/docs.js
@@ -19,7 +19,7 @@ function DocsController(scope, $location, $window, $cookies, $filter) {
     $location.path('/api').replace();
   }
 
-  scope.$watch('$location.path()', function(scope, path) {
+  scope.$watch('$location.path()', function(path) {
     // ignore non-doc links which are used in examples
     if (DOCS_PATH.test(path)) {
       var parts = path.split('/');

diff --git a/src/directives.js b/src/directives.js
@@ -212,9 +212,10 @@ angularDirective("ng:bind", function(expression, element){
   element.addClass('ng-binding');
   return ['$exceptionHandler', '$parse', '$element', function($exceptionHandler, $parse, element) {
     var exprFn = $parse(expression),
-        lastValue = Number.NaN;
+        lastValue = Number.NaN,
+        scope = this;
 
-    this.$watch(function(scope) {
+    scope.$watch(function() {
       // TODO(misko): remove error handling https://github.com/angular/angular.js/issues/347
       var value, html, isHtml, isDomElement,
           hadOwnElement = scope.hasOwnProperty('$element'),
@@ -305,8 +306,10 @@ angularDirective("ng:bind-template", function(expression, element){
   element.addClass('ng-binding');
   var templateFn = compileBindTemplate(expression);
   return function(element) {
-    var lastValue;
-    this.$watch(function(scope) {
+    var lastValue,
+        scope = this;
+
+    scope.$watch(function() {
       var value = templateFn(scope, element, true);
       if (value != lastValue) {
         element.text(value);
@@ -391,8 +394,10 @@ angularDirective("ng:bind-template", function(expression, element){
  */
 angularDirective("ng:bind-attr", function(expression){
   return function(element){
-    var lastValue = {};
-    this.$watch(function(scope){
+    var lastValue = {},
+        scope = this;
+
+    scope.$watch(function() {
       var values = scope.$eval(expression);
       for(var key in values) {
         var value = compileBindTemplate(values[key])(scope, element);
@@ -518,7 +523,8 @@ angularDirective("ng:submit", function(expression, element) {
 function ngClass(selector) {
   return function(expression, element) {
     return function(element) {
-      this.$watch(expression, function(scope, newVal, oldVal) {
+      var scope = this;
+      scope.$watch(expression, function(newVal, oldVal) {
         if (selector(scope.$index)) {
           if (oldVal && (newVal !== oldVal)) {
             element.removeClass(isArray(oldVal) ? oldVal.join(' ') : oldVal);
@@ -687,8 +693,9 @@ angularDirective("ng:class-even", ngClass(function(i){return i % 2 === 1;}));
    </doc:example>
  */
 angularDirective("ng:show", function(expression, element){
-  return function(element){
-    this.$watch(expression, function(scope, value){
+  return function(element) {
+    var scope = this;
+    scope.$watch(expression, function(value) {
       element.css('display', toBoolean(value) ? '' : 'none');
     });
   };
@@ -727,8 +734,9 @@ angularDirective("ng:show", function(expression, element){
    </doc:example>
  */
 angularDirective("ng:hide", function(expression, element){
-  return function(element){
-    this.$watch(expression, function(scope, value){
+  return function(element) {
+    var scope = this;
+    scope.$watch(expression, function(value) {
       element.css('display', toBoolean(value) ? 'none' : '');
     });
   };
@@ -768,7 +776,8 @@ angularDirective("ng:hide", function(expression, element){
  */
 angularDirective("ng:style", function(expression, element) {
   return function(element) {
-    this.$watch(expression, function(scope, newStyles, oldStyles) {
+    var scope = this;
+    scope.$watch(expression, function(newStyles, oldStyles) {
       if (oldStyles && (newStyles !== oldStyles)) {
         forEach(oldStyles, function(val, style) { element.css(style, '');});
       }

diff --git a/src/service/formFactory.js b/src/service/formFactory.js
@@ -377,7 +377,7 @@ function $FormFactoryProvider() {
     // Set the state to something we know will change to get the process going.
     widget.$modelValue = Number.NaN;
     // watch for scope changes and update the view appropriately
-    modelScope.$watch(scopeGet, function(scope, value) {
+    modelScope.$watch(scopeGet, function(value) {
       if (!equals(widget.$modelValue, value)) {
         widget.$modelValue = value;
         widget.$parseModel ? widget.$parseModel() : (widget.$viewValue = value);

diff --git a/src/service/scope.js b/src/service/scope.js
@@ -207,7 +207,7 @@ function $RootScopeProvider(){
            scope.counter = 0;
 
            expect(scope.counter).toEqual(0);
-           scope.$watch('name', function(scope, newValue, oldValue) { counter = counter + 1; });
+           scope.$watch('name', function(newValue, oldValue) { counter = counter + 1; });
            expect(scope.counter).toEqual(0);
 
            scope.$digest();
@@ -231,22 +231,26 @@ function $RootScopeProvider(){
        *   the `watchExpression` changes.
        *
        *    - `string`: Evaluated as {@link guide/dev_guide.expressions expression}
-       *    - `function(scope, newValue, oldValue)`: called with current `scope` an previous and
-       *       current values as parameters.
+       *    - `function(newValue, oldValue, scope)`: called with current and previous values as parameters.
        * @returns {function()} Returns a deregistration function for this listener.
        */
       $watch: function(watchExp, listener) {
         var scope = this,
             get = compileToFn(watchExp, 'watch'),
-            listenFn = compileToFn(listener || noop, 'listener'),
             array = scope.$$watchers,
             watcher = {
-              fn: listenFn,
+              fn: listener,
               last: initWatchVal,
               get: get,
               exp: watchExp
             };
 
+        // in the case user pass string, we need to compile it, do we really need this ?
+        if (!isFunction(listener)) {
+          var listenFn = compileToFn(listener || noop, 'listener');
+          watcher.fn = function(newVal, oldVal, scope) {listenFn(scope);};
+        }
+
         if (!array) {
           array = scope.$$watchers = [];
         }
@@ -341,7 +345,7 @@ function $RootScopeProvider(){
                   if ((value = watch.get(current)) !== (last = watch.last) && !equals(value, last)) {
                     dirty = true;
                     watch.last = copy(value);
-                    watch.fn(current, value, ((last === initWatchVal) ? value : last));
+                    watch.fn(value, ((last === initWatchVal) ? value : last), current);
                     if (ttl < 5) {
                       logIdx = 4 - ttl;
                       if (!watchLog[logIdx]) watchLog[logIdx] = [];

diff --git a/src/widget/form.js b/src/widget/form.js
@@ -99,7 +99,7 @@ angularWidget('form', function(form){
     watch('valid');
     watch('invalid');
     function watch(name) {
-      form.$watch('$' + name, function(scope, value) {
+      form.$watch('$' + name, function(value) {
         formElement[value ? 'addClass' : 'removeClass']('ng-' + name);
       });
     }

diff --git a/src/widget/input.js b/src/widget/input.js
@@ -796,7 +796,7 @@ angularWidget('input', function(inputElement){
       });
 
       forEach(['valid', 'invalid', 'pristine', 'dirty'], function(name) {
-        widget.$watch('$' + name, function(scope, value) {
+        widget.$watch('$' + name, function(value) {
           inputElement[value ? 'addClass' : 'removeClass']('ng-' + name);
         });
       });
@@ -870,7 +870,7 @@ function watchElementProperty(modelScope, widget, name, element) {
         !!element[0].attributes[name])
     : element.attr(name);
   if (bindAttr[name] && match) {
-    modelScope.$watch(match[1], function(scope, value){
+    modelScope.$watch(match[1], function(value) {
       widget['$' + name] = isBoolean ? !!value : value;
       widget.$emit('$validate');
       widget.$render && widget.$render();

diff --git a/src/widget/select.js b/src/widget/select.js
@@ -167,7 +167,7 @@ angularWidget('select', function(element){
     });
 
     forEach(['valid', 'invalid', 'pristine', 'dirty'], function(name) {
-      widget.$watch('$' + name, function(scope, value) {
+      widget.$watch('$' + name, function(value) {
         selectElement[value ? 'addClass' : 'removeClass']('ng-' + name);
       });
     });

diff --git a/src/widgets.js b/src/widgets.js
@@ -111,7 +111,7 @@ angularWidget('ng:include', function(element){
         var includeScope = scope.$eval(scopeExp);
         if (includeScope) return includeScope.$id;
       }, incrementChange);
-      this.$watch(function() {return changeCounter;}, function(scope, newChangeCounter) {
+      this.$watch(function() {return changeCounter;}, function(newChangeCounter) {
         var src = scope.$eval(srcExp),
             useScope = scope.$eval(scopeExp);
 
@@ -233,8 +233,9 @@ angularWidget('ng:switch', function(element) {
     var changeCounter = 0;
     var childScope;
     var selectedTemplate;
+    var scope = this;
 
-    this.$watch(watchExpr, function(scope, value) {
+    this.$watch(watchExpr, function(value) {
       element.html('');
       if ((selectedTemplate = casesTemplate[value] || defaultCaseTemplate)) {
         changeCounter++;
@@ -577,7 +578,7 @@ angularWidget('ng:view', function(element) {
         changeCounter++;
       });
 
-      this.$watch(function() {return changeCounter;}, function(scope, newChangeCounter) {
+      this.$watch(function() {return changeCounter;}, function(newChangeCounter) {
         var template = $route.current && $route.current.template;
 
         function clearContent() {
@@ -802,7 +803,7 @@ angularWidget('ng:pluralize', function(element) {
       } else {
         return '';
       }
-    }, function(scope, newVal) {
+    }, function(newVal) {
       element.text(newVal);
     });
   }];

diff --git a/test/service/compilerSpec.js b/test/service/compilerSpec.js
@@ -17,7 +17,7 @@ describe('compiler', function() {
 
       observe: function(expression, element){
         return function() {
-          this.$watch(expression, function(scope, val){
+          this.$watch(expression, function(val) {
             if (val)
               log += ":" + val;
           });

diff --git a/test/service/routeSpec.js b/test/service/routeSpec.js
@@ -372,7 +372,7 @@ describe('$route', function() {
       function FooCtrl($scope) {
         $scope.$watch(function() {
           return $route.current.params;
-        }, function(scope, value) {
+        }, function(value) {
           routeParams(value);
         });
       }

diff --git a/test/service/scopeSpec.js b/test/service/scopeSpec.js
@@ -68,7 +68,7 @@ describe('Scope', function() {
       expect(spy).not.wasCalled();
       $rootScope.name = 'misko';
       $rootScope.$digest();
-      expect(spy).wasCalledWith($rootScope, 'misko', undefined);
+      expect(spy).wasCalledWith('misko', undefined, $rootScope);
     }));
 
 
@@ -156,9 +156,9 @@ describe('Scope', function() {
 
     it('should repeat watch cycle while model changes are identified', inject(function($rootScope) {
       var log = '';
-      $rootScope.$watch('c', function(self, v) {self.d = v; log+='c'; });
-      $rootScope.$watch('b', function(self, v) {self.c = v; log+='b'; });
-      $rootScope.$watch('a', function(self, v) {self.b = v; log+='a'; });
+      $rootScope.$watch('c', function(v) {$rootScope.d = v; log+='c'; });
+      $rootScope.$watch('b', function(v) {$rootScope.c = v; log+='b'; });
+      $rootScope.$watch('a', function(v) {$rootScope.b = v; log+='a'; });
       $rootScope.$digest();
       log = '';
       $rootScope.a = 1;
@@ -182,8 +182,8 @@ describe('Scope', function() {
 
     it('should prevent infinite recursion and print watcher expression',inject(
         function($rootScope) {
-      $rootScope.$watch('a', function(self) {self.b++;});
-      $rootScope.$watch('b', function(self) {self.a++;});
+      $rootScope.$watch('a', function() {$rootScope.b++;});
+      $rootScope.$watch('b', function() {$rootScope.a++;});
       $rootScope.a = $rootScope.b = 0;
 
       expect(function() {
@@ -200,8 +200,8 @@ describe('Scope', function() {
 
     it('should prevent infinite recursion and print print watcher function name or body',
         inject(function($rootScope) {
-      $rootScope.$watch(function watcherA() {return $rootScope.a;}, function(self) {self.b++;});
-      $rootScope.$watch(function() {return $rootScope.b;}, function(self) {self.a++;});
+      $rootScope.$watch(function watcherA() {return $rootScope.a;}, function() {$rootScope.b++;});
+      $rootScope.$watch(function() {return $rootScope.b;}, function() {$rootScope.a++;});
       $rootScope.a = $rootScope.b = 0;
 
       try {
@@ -229,11 +229,11 @@ describe('Scope', function() {
       var log = '';
       $rootScope.a = [];
       $rootScope.b = {};
-      $rootScope.$watch('a', function(scope, value) {
+      $rootScope.$watch('a', function(value) {
         log +='.';
         expect(value).toBe($rootScope.a);
       });
-      $rootScope.$watch('b', function(scope, value) {
+      $rootScope.$watch('b', function(value) {
         log +='!';
         expect(value).toBe($rootScope.b);
       });
@@ -427,7 +427,7 @@ describe('Scope', function() {
     it('should apply expression with full lifecycle', inject(function($rootScope) {
       var log = '';
       var child = $rootScope.$new();
-      $rootScope.$watch('a', function(scope, a) { log += '1'; });
+      $rootScope.$watch('a', function(a) { log += '1'; });
       child.$apply('$parent.a=0');
       expect(log).toEqual('1');
     }));
@@ -440,7 +440,7 @@ describe('Scope', function() {
       inject(function($rootScope, $exceptionHandler, $log) {
         var log = '';
         var child = $rootScope.$new();
-        $rootScope.$watch('a', function(scope, a) { log += '1'; });
+        $rootScope.$watch('a', function(a) { log += '1'; });
         $rootScope.a = 0;
         child.$apply(function() { throw new Error('MyError'); });
         expect(log).toEqual('1');
@@ -520,7 +520,7 @@ describe('Scope', function() {
           function($rootScope) {
         var childScope2 = $rootScope.$new();
         childScope2.$apply(function() {
-          childScope2.$watch('x', function(scope, newVal, oldVal) {
+          childScope2.$watch('x', function(newVal, oldVal) {
             if (newVal !== oldVal) {
               childScope2.$apply();
             }