Skip to content

Commit 081195a

Browse files
committed
fix(ngModel): execute getter/setter models with context
Closes angular#9394 BREAKING CHANGE: - the ngModelController getterSetter property is read once at link time and can not change afterwards - previously ngModel invoked the getterSetter in the global context - the ngModel getterSetter expression must work when `()` (a method invokation) is appended to it
1 parent 2b41a58 commit 081195a

File tree

2 files changed

+74
-22
lines changed

2 files changed

+74
-22
lines changed

src/ng/directive/input.js

+25-22
Original file line numberDiff line numberDiff line change
@@ -1743,29 +1743,32 @@ var NgModelController = ['$scope', '$exceptionHandler', '$attrs', '$element', '$
17431743
pendingDebounce = null,
17441744
ctrl = this;
17451745

1746-
var ngModelGet = function ngModelGet() {
1747-
var modelValue = parsedNgModel($scope);
1748-
if (ctrl.$options && ctrl.$options.getterSetter && isFunction(modelValue)) {
1749-
modelValue = modelValue();
1750-
}
1751-
return modelValue;
1752-
};
1753-
1754-
var ngModelSet = function ngModelSet(newValue) {
1755-
var getterSetter;
1756-
if (ctrl.$options && ctrl.$options.getterSetter &&
1757-
isFunction(getterSetter = parsedNgModel($scope))) {
1758-
1759-
getterSetter(ctrl.$modelValue);
1760-
} else {
1761-
parsedNgModel.assign($scope, ctrl.$modelValue);
1762-
}
1763-
};
1746+
var ngModelGet = parsedNgModel;
1747+
var ngModelSet = ngModelGet.assign;
17641748

17651749
this.$$setOptions = function(options) {
17661750
ctrl.$options = options;
17671751

1768-
if (!parsedNgModel.assign && (!options || !options.getterSetter)) {
1752+
if (options && options.getterSetter) {
1753+
var ngModelGetterGet = $parse($attr.ngModel + "()");
1754+
var ngModelGetterSet = $parse($attr.ngModel + "($$ngModelValue)");
1755+
1756+
//TODO: remove support for the ngModel being a non-function when getterSetter is enabled
1757+
ngModelGet = function ngModelGetterSetterGet($scope) {
1758+
var value = parsedNgModel($scope);
1759+
if (isFunction(value)) {
1760+
value = ngModelGetterGet($scope);
1761+
}
1762+
return value;
1763+
};
1764+
ngModelSet = function ngModelGetterSetterSet($scope, value) {
1765+
if (!parsedNgModel.assign || isFunction(parsedNgModel($scope))) {
1766+
ngModelGetterSet($scope, {$$ngModelValue: value});
1767+
} else {
1768+
parsedNgModel.assign($scope, value);
1769+
}
1770+
};
1771+
} else if (!ngModelSet) {
17691772
throw $ngModelMinErr('nonassign', "Expression '{0}' is non-assignable. Element: {1}",
17701773
$attr.ngModel, startingTag($element));
17711774
}
@@ -2164,7 +2167,7 @@ var NgModelController = ['$scope', '$exceptionHandler', '$attrs', '$element', '$
21642167
}
21652168
if (isNumber(ctrl.$modelValue) && isNaN(ctrl.$modelValue)) {
21662169
// ctrl.$modelValue has not been touched yet...
2167-
ctrl.$modelValue = ngModelGet();
2170+
ctrl.$modelValue = ngModelGet($scope);
21682171
}
21692172
var prevModelValue = ctrl.$modelValue;
21702173
var allowInvalid = ctrl.$options && ctrl.$options.allowInvalid;
@@ -2192,7 +2195,7 @@ var NgModelController = ['$scope', '$exceptionHandler', '$attrs', '$element', '$
21922195
};
21932196

21942197
this.$$writeModelToScope = function() {
2195-
ngModelSet(ctrl.$modelValue);
2198+
ngModelSet($scope, ctrl.$modelValue);
21962199
forEach(ctrl.$viewChangeListeners, function(listener) {
21972200
try {
21982201
listener();
@@ -2288,7 +2291,7 @@ var NgModelController = ['$scope', '$exceptionHandler', '$attrs', '$element', '$
22882291
// ng-change executes in apply phase
22892292
// 4. view should be changed back to 'a'
22902293
$scope.$watch(function ngModelWatch() {
2291-
var modelValue = ngModelGet();
2294+
var modelValue = ngModelGet($scope);
22922295

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

test/ng/directive/inputSpec.js

+49
Original file line numberDiff line numberDiff line change
@@ -2093,6 +2093,55 @@ describe('input', function() {
20932093
'ng-model-options="{ getterSetter: true }" />');
20942094
});
20952095

2096+
it('should invoke a model in the correct context if getterSetter is true', function() {
2097+
compileInput(
2098+
'<input type="text" ng-model="someService.getterSetter" ' +
2099+
'ng-model-options="{ getterSetter: true }" />');
2100+
2101+
scope.someService = {
2102+
value: 'a',
2103+
getterSetter: function(newValue) {
2104+
this.value = newValue || this.value;
2105+
return this.value;
2106+
}
2107+
};
2108+
spyOn(scope.someService, 'getterSetter').andCallThrough();
2109+
scope.$apply();
2110+
2111+
expect(inputElm.val()).toBe('a');
2112+
expect(scope.someService.getterSetter).toHaveBeenCalledWith();
2113+
expect(scope.someService.value).toBe('a');
2114+
2115+
changeInputValueTo('b');
2116+
expect(scope.someService.getterSetter).toHaveBeenCalledWith('b');
2117+
expect(scope.someService.value).toBe('b');
2118+
2119+
scope.someService.value = 'c';
2120+
scope.$apply();
2121+
expect(inputElm.val()).toBe('c');
2122+
expect(scope.someService.getterSetter).toHaveBeenCalledWith();
2123+
});
2124+
2125+
it('should invoke weird model expressions when getterSetter is true', function() {
2126+
compileInput(
2127+
'<input type="text" ng-model="(useA ? a : b)" ' +
2128+
'ng-model-options="{ getterSetter: true }" />');
2129+
2130+
scope.useA = true;
2131+
scope.a = jasmine.createSpy('aGetter');
2132+
scope.b = jasmine.createSpy('bGetter');
2133+
2134+
scope.$apply();
2135+
expect(scope.a).toHaveBeenCalled();
2136+
scope.$apply();
2137+
expect(scope.a).toHaveBeenCalled();
2138+
expect(scope.b).not.toHaveBeenCalled();
2139+
2140+
scope.useA = false;
2141+
scope.$apply();
2142+
expect(scope.b).toHaveBeenCalled();
2143+
});
2144+
20962145
it('should assign invalid values to the scope if allowInvalid is true', function() {
20972146
compileInput('<input type="text" name="input" ng-model="value" maxlength="1" ' +
20982147
'ng-model-options="{allowInvalid: true}" />');

0 commit comments

Comments
 (0)