From 0196411dbe179afe24f4faa6d6503ff3f69472da Mon Sep 17 00:00:00 2001 From: Vojta Jina Date: Wed, 30 Nov 2011 12:23:58 -0800 Subject: [PATCH] refactor(scope.$watch): rearrange arguments passed into watcher (newValue, oldValue, scope) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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) --- docs/content/cookbook/mvc.ngdoc | 2 +- .../guide/dev_guide.scopes.internals.ngdoc | 2 +- .../guide/dev_guide.services.$location.ngdoc | 4 +-- docs/src/templates/docs.js | 2 +- src/directives.js | 33 ++++++++++++------- src/service/formFactory.js | 2 +- src/service/scope.js | 16 +++++---- src/widget/form.js | 2 +- src/widget/input.js | 4 +-- src/widget/select.js | 2 +- src/widgets.js | 9 ++--- test/service/compilerSpec.js | 2 +- test/service/routeSpec.js | 2 +- test/service/scopeSpec.js | 26 +++++++-------- 14 files changed, 61 insertions(+), 47 deletions(-) diff --git a/docs/content/cookbook/mvc.ngdoc b/docs/content/cookbook/mvc.ngdoc index 71e771bde08..cdb5ebdcb47 100644 --- 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 index 66d57a9fd0b..a81d88032a0 100644 --- 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 index 57c39191d66..2afbf1d268f 100644 --- 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: 
 // 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;
 });
diff --git a/docs/src/templates/docs.js b/docs/src/templates/docs.js index bf279df5836..7fb9cb22f96 100644 --- 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 index 53d03573a23..61bb6139f01 100644 --- 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;})); */ 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){ */ 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 index 565b22a43ea..5d64131e89b 100644 --- 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 index 089e4a41fc9..f59417bc524 100644 --- 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 index f3134db49bf..962cb6b8972 100644 --- 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 index 5db52704c4e..e666a0c1770 100644 --- 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 index b0f5eac587c..a3633ddd50c 100644 --- 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 index 6b3e93ee6db..53be8b1458f 100644 --- 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 index 6e6c211b224..2df15c51761 100644 --- 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 index 1bb1312f7ea..d6f7d753c24 100644 --- 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 index 68ef2834dcb..308056847a9 100644 --- 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(); }