Skip to content
This repository was archived by the owner on Apr 12, 2024. It is now read-only.

Commit 0196411

Browse files
vojtajinavojtajina
committed
refactor(scope.$watch): rearrange arguments passed into watcher (newValue, 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)
1 parent 992c790 commit 0196411

File tree

14 files changed

+61
-47
lines changed

14 files changed

+61
-47
lines changed

‎docs/content/cookbook/mvc.ngdoc

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -66,7 +66,7 @@ no connection between the controller and the view.
6666
function same(a, b, c) { return (a==b && b==c) ? a : '';};
6767
}
6868

69-
function readUrl(scope, value) {
69+
function readUrl(value) {
7070
if (value) {
7171
value = value.split('/');
7272
$scope.nextMove = value[1];

‎docs/content/guide/dev_guide.scopes.internals.ngdoc

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -187,7 +187,7 @@ within the unit-tests.
187187
// example of a test
188188
it('should trigger a watcher', inject(function($rootScope) {
189189
var scope = $rootScope;
190-
scope.$watch('name', function(scope, name){
190+
scope.$watch('name', function(name) {
191191
scope.greeting = 'Hello ' + name + '!';
192192
});
193193

‎docs/content/guide/dev_guide.services.$location.ngdoc

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -618,11 +618,11 @@ you will need to specify an extra property that has two watchers. For example:
618618
</pre>
619619
<pre>
620620
// js - controller
621-
this.$watch('locationPath', function(scope, path) {
621+
this.$watch('locationPath', function(path) {
622622
$location.path(path);
623623
});
624624

625-
this.$watch('$location.path()', function(scope, path) {
625+
this.$watch('$location.path()', function(path) {
626626
scope.locationPath = path;
627627
});
628628
</pre>

‎docs/src/templates/docs.js

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -19,7 +19,7 @@ function DocsController(scope, $location, $window, $cookies, $filter) {
1919
$location.path('/api').replace();
2020
}
2121

22-
scope.$watch('$location.path()', function(scope, path) {
22+
scope.$watch('$location.path()', function(path) {
2323
// ignore non-doc links which are used in examples
2424
if (DOCS_PATH.test(path)) {
2525
var parts = path.split('/');

‎src/directives.js

Lines changed: 21 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -212,9 +212,10 @@ angularDirective("ng:bind", function(expression, element){
212212
element.addClass('ng-binding');
213213
return ['$exceptionHandler', '$parse', '$element', function($exceptionHandler, $parse, element) {
214214
var exprFn = $parse(expression),
215-
lastValue = Number.NaN;
215+
lastValue = Number.NaN,
216+
scope = this;
216217

217-
this.$watch(function(scope) {
218+
scope.$watch(function() {
218219
// TODO(misko): remove error handling https://github.com/angular/angular.js/issues/347
219220
var value, html, isHtml, isDomElement,
220221
hadOwnElement = scope.hasOwnProperty('$element'),
@@ -305,8 +306,10 @@ angularDirective("ng:bind-template", function(expression, element){
305306
element.addClass('ng-binding');
306307
var templateFn = compileBindTemplate(expression);
307308
return function(element) {
308-
var lastValue;
309-
this.$watch(function(scope) {
309+
var lastValue,
310+
scope = this;
311+
312+
scope.$watch(function() {
310313
var value = templateFn(scope, element, true);
311314
if (value != lastValue) {
312315
element.text(value);
@@ -391,8 +394,10 @@ angularDirective("ng:bind-template", function(expression, element){
391394
*/
392395
angularDirective("ng:bind-attr", function(expression){
393396
return function(element){
394-
var lastValue = {};
395-
this.$watch(function(scope){
397+
var lastValue = {},
398+
scope = this;
399+
400+
scope.$watch(function() {
396401
var values = scope.$eval(expression);
397402
for(var key in values) {
398403
var value = compileBindTemplate(values[key])(scope, element);
@@ -518,7 +523,8 @@ angularDirective("ng:submit", function(expression, element) {
518523
function ngClass(selector) {
519524
return function(expression, element) {
520525
return function(element) {
521-
this.$watch(expression, function(scope, newVal, oldVal) {
526+
var scope = this;
527+
scope.$watch(expression, function(newVal, oldVal) {
522528
if (selector(scope.$index)) {
523529
if (oldVal && (newVal !== oldVal)) {
524530
element.removeClass(isArray(oldVal) ? oldVal.join(' ') : oldVal);
@@ -687,8 +693,9 @@ angularDirective("ng:class-even", ngClass(function(i){return i % 2 === 1;}));
687693
</doc:example>
688694
*/
689695
angularDirective("ng:show", function(expression, element){
690-
return function(element){
691-
this.$watch(expression, function(scope, value){
696+
return function(element) {
697+
var scope = this;
698+
scope.$watch(expression, function(value) {
692699
element.css('display', toBoolean(value) ? '' : 'none');
693700
});
694701
};
@@ -727,8 +734,9 @@ angularDirective("ng:show", function(expression, element){
727734
</doc:example>
728735
*/
729736
angularDirective("ng:hide", function(expression, element){
730-
return function(element){
731-
this.$watch(expression, function(scope, value){
737+
return function(element) {
738+
var scope = this;
739+
scope.$watch(expression, function(value) {
732740
element.css('display', toBoolean(value) ? 'none' : '');
733741
});
734742
};
@@ -768,7 +776,8 @@ angularDirective("ng:hide", function(expression, element){
768776
*/
769777
angularDirective("ng:style", function(expression, element) {
770778
return function(element) {
771-
this.$watch(expression, function(scope, newStyles, oldStyles) {
779+
var scope = this;
780+
scope.$watch(expression, function(newStyles, oldStyles) {
772781
if (oldStyles && (newStyles !== oldStyles)) {
773782
forEach(oldStyles, function(val, style) { element.css(style, '');});
774783
}

‎src/service/formFactory.js

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -377,7 +377,7 @@ function $FormFactoryProvider() {
377377
// Set the state to something we know will change to get the process going.
378378
widget.$modelValue = Number.NaN;
379379
// watch for scope changes and update the view appropriately
380-
modelScope.$watch(scopeGet, function(scope, value) {
380+
modelScope.$watch(scopeGet, function(value) {
381381
if (!equals(widget.$modelValue, value)) {
382382
widget.$modelValue = value;
383383
widget.$parseModel ? widget.$parseModel() : (widget.$viewValue = value);

‎src/service/scope.js

Lines changed: 10 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -207,7 +207,7 @@ function $RootScopeProvider(){
207207
scope.counter = 0;
208208
209209
expect(scope.counter).toEqual(0);
210-
scope.$watch('name', function(scope, newValue, oldValue) { counter = counter + 1; });
210+
scope.$watch('name', function(newValue, oldValue) { counter = counter + 1; });
211211
expect(scope.counter).toEqual(0);
212212
213213
scope.$digest();
@@ -231,22 +231,26 @@ function $RootScopeProvider(){
231231
* the `watchExpression` changes.
232232
*
233233
* - `string`: Evaluated as {@link guide/dev_guide.expressions expression}
234-
* - `function(scope, newValue, oldValue)`: called with current `scope` an previous and
235-
* current values as parameters.
234+
* - `function(newValue, oldValue, scope)`: called with current and previous values as parameters.
236235
* @returns {function()} Returns a deregistration function for this listener.
237236
*/
238237
$watch: function(watchExp, listener) {
239238
var scope = this,
240239
get = compileToFn(watchExp, 'watch'),
241-
listenFn = compileToFn(listener || noop, 'listener'),
242240
array = scope.$$watchers,
243241
watcher = {
244-
fn: listenFn,
242+
fn: listener,
245243
last: initWatchVal,
246244
get: get,
247245
exp: watchExp
248246
};
249247

248+
// in the case user pass string, we need to compile it, do we really need this ?
249+
if (!isFunction(listener)) {
250+
var listenFn = compileToFn(listener || noop, 'listener');
251+
watcher.fn = function(newVal, oldVal, scope) {listenFn(scope);};
252+
}
253+
250254
if (!array) {
251255
array = scope.$$watchers = [];
252256
}
@@ -341,7 +345,7 @@ function $RootScopeProvider(){
341345
if ((value = watch.get(current)) !== (last = watch.last) && !equals(value, last)) {
342346
dirty = true;
343347
watch.last = copy(value);
344-
watch.fn(current, value, ((last === initWatchVal) ? value : last));
348+
watch.fn(value, ((last === initWatchVal) ? value : last), current);
345349
if (ttl < 5) {
346350
logIdx = 4 - ttl;
347351
if (!watchLog[logIdx]) watchLog[logIdx] = [];

‎src/widget/form.js

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -99,7 +99,7 @@ angularWidget('form', function(form){
9999
watch('valid');
100100
watch('invalid');
101101
function watch(name) {
102-
form.$watch('$' + name, function(scope, value) {
102+
form.$watch('$' + name, function(value) {
103103
formElement[value ? 'addClass' : 'removeClass']('ng-' + name);
104104
});
105105
}

‎src/widget/input.js

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -796,7 +796,7 @@ angularWidget('input', function(inputElement){
796796
});
797797

798798
forEach(['valid', 'invalid', 'pristine', 'dirty'], function(name) {
799-
widget.$watch('$' + name, function(scope, value) {
799+
widget.$watch('$' + name, function(value) {
800800
inputElement[value ? 'addClass' : 'removeClass']('ng-' + name);
801801
});
802802
});
@@ -870,7 +870,7 @@ function watchElementProperty(modelScope, widget, name, element) {
870870
!!element[0].attributes[name])
871871
: element.attr(name);
872872
if (bindAttr[name] && match) {
873-
modelScope.$watch(match[1], function(scope, value){
873+
modelScope.$watch(match[1], function(value) {
874874
widget['$' + name] = isBoolean ? !!value : value;
875875
widget.$emit('$validate');
876876
widget.$render && widget.$render();

‎src/widget/select.js

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -167,7 +167,7 @@ angularWidget('select', function(element){
167167
});
168168

169169
forEach(['valid', 'invalid', 'pristine', 'dirty'], function(name) {
170-
widget.$watch('$' + name, function(scope, value) {
170+
widget.$watch('$' + name, function(value) {
171171
selectElement[value ? 'addClass' : 'removeClass']('ng-' + name);
172172
});
173173
});

‎src/widgets.js

Lines changed: 5 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -111,7 +111,7 @@ angularWidget('ng:include', function(element){
111111
var includeScope = scope.$eval(scopeExp);
112112
if (includeScope) return includeScope.$id;
113113
}, incrementChange);
114-
this.$watch(function() {return changeCounter;}, function(scope, newChangeCounter) {
114+
this.$watch(function() {return changeCounter;}, function(newChangeCounter) {
115115
var src = scope.$eval(srcExp),
116116
useScope = scope.$eval(scopeExp);
117117

@@ -233,8 +233,9 @@ angularWidget('ng:switch', function(element) {
233233
var changeCounter = 0;
234234
var childScope;
235235
var selectedTemplate;
236+
var scope = this;
236237

237-
this.$watch(watchExpr, function(scope, value) {
238+
this.$watch(watchExpr, function(value) {
238239
element.html('');
239240
if ((selectedTemplate = casesTemplate[value] || defaultCaseTemplate)) {
240241
changeCounter++;
@@ -577,7 +578,7 @@ angularWidget('ng:view', function(element) {
577578
changeCounter++;
578579
});
579580

580-
this.$watch(function() {return changeCounter;}, function(scope, newChangeCounter) {
581+
this.$watch(function() {return changeCounter;}, function(newChangeCounter) {
581582
var template = $route.current && $route.current.template;
582583

583584
function clearContent() {
@@ -802,7 +803,7 @@ angularWidget('ng:pluralize', function(element) {
802803
} else {
803804
return '';
804805
}
805-
}, function(scope, newVal) {
806+
}, function(newVal) {
806807
element.text(newVal);
807808
});
808809
}];

‎test/service/compilerSpec.js

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -17,7 +17,7 @@ describe('compiler', function() {
1717

1818
observe: function(expression, element){
1919
return function() {
20-
this.$watch(expression, function(scope, val){
20+
this.$watch(expression, function(val) {
2121
if (val)
2222
log += ":" + val;
2323
});

‎test/service/routeSpec.js

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -372,7 +372,7 @@ describe('$route', function() {
372372
function FooCtrl($scope) {
373373
$scope.$watch(function() {
374374
return $route.current.params;
375-
}, function(scope, value) {
375+
}, function(value) {
376376
routeParams(value);
377377
});
378378
}

‎test/service/scopeSpec.js

Lines changed: 13 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -68,7 +68,7 @@ describe('Scope', function() {
6868
expect(spy).not.wasCalled();
6969
$rootScope.name = 'misko';
7070
$rootScope.$digest();
71-
expect(spy).wasCalledWith($rootScope, 'misko', undefined);
71+
expect(spy).wasCalledWith('misko', undefined, $rootScope);
7272
}));
7373

7474

@@ -156,9 +156,9 @@ describe('Scope', function() {
156156

157157
it('should repeat watch cycle while model changes are identified', inject(function($rootScope) {
158158
var log = '';
159-
$rootScope.$watch('c', function(self, v) {self.d = v; log+='c'; });
160-
$rootScope.$watch('b', function(self, v) {self.c = v; log+='b'; });
161-
$rootScope.$watch('a', function(self, v) {self.b = v; log+='a'; });
159+
$rootScope.$watch('c', function(v) {$rootScope.d = v; log+='c'; });
160+
$rootScope.$watch('b', function(v) {$rootScope.c = v; log+='b'; });
161+
$rootScope.$watch('a', function(v) {$rootScope.b = v; log+='a'; });
162162
$rootScope.$digest();
163163
log = '';
164164
$rootScope.a = 1;
@@ -182,8 +182,8 @@ describe('Scope', function() {
182182

183183
it('should prevent infinite recursion and print watcher expression',inject(
184184
function($rootScope) {
185-
$rootScope.$watch('a', function(self) {self.b++;});
186-
$rootScope.$watch('b', function(self) {self.a++;});
185+
$rootScope.$watch('a', function() {$rootScope.b++;});
186+
$rootScope.$watch('b', function() {$rootScope.a++;});
187187
$rootScope.a = $rootScope.b = 0;
188188

189189
expect(function() {
@@ -200,8 +200,8 @@ describe('Scope', function() {
200200

201201
it('should prevent infinite recursion and print print watcher function name or body',
202202
inject(function($rootScope) {
203-
$rootScope.$watch(function watcherA() {return $rootScope.a;}, function(self) {self.b++;});
204-
$rootScope.$watch(function() {return $rootScope.b;}, function(self) {self.a++;});
203+
$rootScope.$watch(function watcherA() {return $rootScope.a;}, function() {$rootScope.b++;});
204+
$rootScope.$watch(function() {return $rootScope.b;}, function() {$rootScope.a++;});
205205
$rootScope.a = $rootScope.b = 0;
206206

207207
try {
@@ -229,11 +229,11 @@ describe('Scope', function() {
229229
var log = '';
230230
$rootScope.a = [];
231231
$rootScope.b = {};
232-
$rootScope.$watch('a', function(scope, value) {
232+
$rootScope.$watch('a', function(value) {
233233
log +='.';
234234
expect(value).toBe($rootScope.a);
235235
});
236-
$rootScope.$watch('b', function(scope, value) {
236+
$rootScope.$watch('b', function(value) {
237237
log +='!';
238238
expect(value).toBe($rootScope.b);
239239
});
@@ -427,7 +427,7 @@ describe('Scope', function() {
427427
it('should apply expression with full lifecycle', inject(function($rootScope) {
428428
var log = '';
429429
var child = $rootScope.$new();
430-
$rootScope.$watch('a', function(scope, a) { log += '1'; });
430+
$rootScope.$watch('a', function(a) { log += '1'; });
431431
child.$apply('$parent.a=0');
432432
expect(log).toEqual('1');
433433
}));
@@ -440,7 +440,7 @@ describe('Scope', function() {
440440
inject(function($rootScope, $exceptionHandler, $log) {
441441
var log = '';
442442
var child = $rootScope.$new();
443-
$rootScope.$watch('a', function(scope, a) { log += '1'; });
443+
$rootScope.$watch('a', function(a) { log += '1'; });
444444
$rootScope.a = 0;
445445
child.$apply(function() { throw new Error('MyError'); });
446446
expect(log).toEqual('1');
@@ -520,7 +520,7 @@ describe('Scope', function() {
520520
function($rootScope) {
521521
var childScope2 = $rootScope.$new();
522522
childScope2.$apply(function() {
523-
childScope2.$watch('x', function(scope, newVal, oldVal) {
523+
childScope2.$watch('x', function(newVal, oldVal) {
524524
if (newVal !== oldVal) {
525525
childScope2.$apply();
526526
}

0 commit comments

Comments
 (0)