Skip to content

Commit 0f912be

Browse files
btfordgkalpak
authored andcommitted
fix(ngModelOptions): preserve context of getter/setters
Many thanks to @NevilleS and @jbedard for collaborating with me on a solution to this! Closes angular#9394 Closes angular#9865 BREAKING CHANGE: previously, ngModel invoked getter/setters in the global context. For example: ```js <input ng-model="model.value" ng-model-options="{ getterSetter: true }"> ``` would previously invoke `model.value()` in the global context. Now, ngModel invokes `value` with `model` as the context. It's unlikely that real apps relied on this behavior. If they did they can use `.bind` to explicilty bind a getter/getter to the global context, or just reference globals normally without `this`.
1 parent d209a33 commit 0f912be

File tree

2 files changed

+61
-25
lines changed

2 files changed

+61
-25
lines changed

src/ng/directive/input.js

+25-24
Original file line numberDiff line numberDiff line change
@@ -1751,32 +1751,33 @@ var NgModelController = ['$scope', '$exceptionHandler', '$attrs', '$element', '$
17511751

17521752

17531753
var parsedNgModel = $parse($attr.ngModel),
1754+
parsedNgModelAssign = parsedNgModel.assign,
1755+
ngModelGet = parsedNgModel,
1756+
ngModelSet = parsedNgModelAssign,
17541757
pendingDebounce = null,
17551758
ctrl = this;
17561759

1757-
var ngModelGet = function ngModelGet() {
1758-
var modelValue = parsedNgModel($scope);
1759-
if (ctrl.$options && ctrl.$options.getterSetter && isFunction(modelValue)) {
1760-
modelValue = modelValue();
1761-
}
1762-
return modelValue;
1763-
};
1764-
1765-
var ngModelSet = function ngModelSet(newValue) {
1766-
var getterSetter;
1767-
if (ctrl.$options && ctrl.$options.getterSetter &&
1768-
isFunction(getterSetter = parsedNgModel($scope))) {
1769-
1770-
getterSetter(ctrl.$modelValue);
1771-
} else {
1772-
parsedNgModel.assign($scope, ctrl.$modelValue);
1773-
}
1774-
};
1775-
17761760
this.$$setOptions = function(options) {
17771761
ctrl.$options = options;
1778-
1779-
if (!parsedNgModel.assign && (!options || !options.getterSetter)) {
1762+
if (options && options.getterSetter) {
1763+
var invokeModelGetter = $parse($attr.ngModel + '()'),
1764+
invokeModelSetter = $parse($attr.ngModel + '($$$p)');
1765+
1766+
ngModelGet = function($scope) {
1767+
var modelValue = parsedNgModel($scope);
1768+
if (isFunction(modelValue)) {
1769+
modelValue = invokeModelGetter($scope);
1770+
}
1771+
return modelValue;
1772+
};
1773+
ngModelSet = function($scope, newValue) {
1774+
if (isFunction(parsedNgModel($scope))) {
1775+
invokeModelSetter($scope, {$$$p: ctrl.$modelValue});
1776+
} else {
1777+
parsedNgModelAssign($scope, ctrl.$modelValue);
1778+
}
1779+
};
1780+
} else if (!parsedNgModel.assign) {
17801781
throw $ngModelMinErr('nonassign', "Expression '{0}' is non-assignable. Element: {1}",
17811782
$attr.ngModel, startingTag($element));
17821783
}
@@ -2189,7 +2190,7 @@ var NgModelController = ['$scope', '$exceptionHandler', '$attrs', '$element', '$
21892190
}
21902191
if (isNumber(ctrl.$modelValue) && isNaN(ctrl.$modelValue)) {
21912192
// ctrl.$modelValue has not been touched yet...
2192-
ctrl.$modelValue = ngModelGet();
2193+
ctrl.$modelValue = ngModelGet($scope);
21932194
}
21942195
var prevModelValue = ctrl.$modelValue;
21952196
var allowInvalid = ctrl.$options && ctrl.$options.allowInvalid;
@@ -2217,7 +2218,7 @@ var NgModelController = ['$scope', '$exceptionHandler', '$attrs', '$element', '$
22172218
};
22182219

22192220
this.$$writeModelToScope = function() {
2220-
ngModelSet(ctrl.$modelValue);
2221+
ngModelSet($scope, ctrl.$modelValue);
22212222
forEach(ctrl.$viewChangeListeners, function(listener) {
22222223
try {
22232224
listener();
@@ -2313,7 +2314,7 @@ var NgModelController = ['$scope', '$exceptionHandler', '$attrs', '$element', '$
23132314
// ng-change executes in apply phase
23142315
// 4. view should be changed back to 'a'
23152316
$scope.$watch(function ngModelWatch() {
2316-
var modelValue = ngModelGet();
2317+
var modelValue = ngModelGet($scope);
23172318

23182319
// if scope model value and ngModel value are out of sync
23192320
// TODO(perf): why not move this to the action fn?

test/ng/directive/inputSpec.js

+36-1
Original file line numberDiff line numberDiff line change
@@ -2102,7 +2102,7 @@ describe('input', function() {
21022102
expect(inputElm.val()).toBe('a');
21032103
});
21042104

2105-
it('should always try to invoke a model if getterSetter is true', function() {
2105+
it('should try to invoke a function model if getterSetter is true', function() {
21062106
compileInput(
21072107
'<input type="text" ng-model="name" ' +
21082108
'ng-model-options="{ getterSetter: true }" />');
@@ -2117,6 +2117,12 @@ describe('input', function() {
21172117
expect(inputElm.val()).toBe('b');
21182118
expect(spy).toHaveBeenCalledWith('a');
21192119
expect(scope.name).toBe(spy);
2120+
});
2121+
2122+
it('should assign to non-function models if getterSetter is true', function() {
2123+
compileInput(
2124+
'<input type="text" ng-model="name" ' +
2125+
'ng-model-options="{ getterSetter: true }" />');
21202126

21212127
scope.name = 'c';
21222128
changeInputValueTo('d');
@@ -2136,6 +2142,35 @@ describe('input', function() {
21362142
'ng-model-options="{ getterSetter: true }" />');
21372143
});
21382144

2145+
it('should invoke a model in the correct context if getterSetter is true', function() {
2146+
compileInput(
2147+
'<input type="text" ng-model="someService.getterSetter" ' +
2148+
'ng-model-options="{ getterSetter: true }" />');
2149+
2150+
scope.someService = {
2151+
value: 'a',
2152+
getterSetter: function(newValue) {
2153+
this.value = newValue || this.value;
2154+
return this.value;
2155+
}
2156+
};
2157+
spyOn(scope.someService, 'getterSetter').andCallThrough();
2158+
scope.$apply();
2159+
2160+
expect(inputElm.val()).toBe('a');
2161+
expect(scope.someService.getterSetter).toHaveBeenCalledWith();
2162+
expect(scope.someService.value).toBe('a');
2163+
2164+
changeInputValueTo('b');
2165+
expect(scope.someService.getterSetter).toHaveBeenCalledWith('b');
2166+
expect(scope.someService.value).toBe('b');
2167+
2168+
scope.someService.value = 'c';
2169+
scope.$apply();
2170+
expect(inputElm.val()).toBe('c');
2171+
expect(scope.someService.getterSetter).toHaveBeenCalledWith();
2172+
});
2173+
21392174
it('should assign invalid values to the scope if allowInvalid is true', function() {
21402175
compileInput('<input type="text" name="input" ng-model="value" maxlength="1" ' +
21412176
'ng-model-options="{allowInvalid: true}" />');

0 commit comments

Comments
 (0)