fix(ngModelOptions): preserve context of getter/setters

Many thanks to @NevilleS and @jbedard for collaborating with me on a solution to this!

Closes #9394
Closes #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`.
This commit is contained in:
Brian Ford
2014-11-19 16:57:27 -08:00
parent 6dbd606ad7
commit bb4d3b73a1
2 changed files with 60 additions and 24 deletions

View File

@@ -1751,32 +1751,33 @@ var NgModelController = ['$scope', '$exceptionHandler', '$attrs', '$element', '$
var parsedNgModel = $parse($attr.ngModel),
parsedNgModelAssign = parsedNgModel.assign,
ngModelGet = parsedNgModel,
ngModelSet = parsedNgModelAssign,
pendingDebounce = null,
ctrl = this;
var ngModelGet = function ngModelGet() {
var modelValue = parsedNgModel($scope);
if (ctrl.$options && ctrl.$options.getterSetter && isFunction(modelValue)) {
modelValue = modelValue();
}
return modelValue;
};
var ngModelSet = function ngModelSet(newValue) {
var getterSetter;
if (ctrl.$options && ctrl.$options.getterSetter &&
isFunction(getterSetter = parsedNgModel($scope))) {
getterSetter(ctrl.$modelValue);
} else {
parsedNgModel.assign($scope, ctrl.$modelValue);
}
};
this.$$setOptions = function(options) {
ctrl.$options = options;
if (options && options.getterSetter) {
var invokeModelGetter = $parse($attr.ngModel + '()'),
invokeModelSetter = $parse($attr.ngModel + '($$$p)');
if (!parsedNgModel.assign && (!options || !options.getterSetter)) {
ngModelGet = function($scope) {
var modelValue = parsedNgModel($scope);
if (isFunction(modelValue)) {
modelValue = invokeModelGetter($scope);
}
return modelValue;
};
ngModelSet = function($scope, newValue) {
if (isFunction(parsedNgModel($scope))) {
invokeModelSetter($scope, {$$$p: ctrl.$modelValue});
} else {
parsedNgModelAssign($scope, ctrl.$modelValue);
}
};
} else if (!parsedNgModel.assign) {
throw $ngModelMinErr('nonassign', "Expression '{0}' is non-assignable. Element: {1}",
$attr.ngModel, startingTag($element));
}
@@ -2189,7 +2190,7 @@ var NgModelController = ['$scope', '$exceptionHandler', '$attrs', '$element', '$
}
if (isNumber(ctrl.$modelValue) && isNaN(ctrl.$modelValue)) {
// ctrl.$modelValue has not been touched yet...
ctrl.$modelValue = ngModelGet();
ctrl.$modelValue = ngModelGet($scope);
}
var prevModelValue = ctrl.$modelValue;
var allowInvalid = ctrl.$options && ctrl.$options.allowInvalid;
@@ -2217,7 +2218,7 @@ var NgModelController = ['$scope', '$exceptionHandler', '$attrs', '$element', '$
};
this.$$writeModelToScope = function() {
ngModelSet(ctrl.$modelValue);
ngModelSet($scope, ctrl.$modelValue);
forEach(ctrl.$viewChangeListeners, function(listener) {
try {
listener();
@@ -2313,7 +2314,7 @@ var NgModelController = ['$scope', '$exceptionHandler', '$attrs', '$element', '$
// ng-change executes in apply phase
// 4. view should be changed back to 'a'
$scope.$watch(function ngModelWatch() {
var modelValue = ngModelGet();
var modelValue = ngModelGet($scope);
// if scope model value and ngModel value are out of sync
// TODO(perf): why not move this to the action fn?

View File

@@ -2102,7 +2102,7 @@ describe('input', function() {
expect(inputElm.val()).toBe('a');
});
it('should always try to invoke a model if getterSetter is true', function() {
it('should try to invoke a function model if getterSetter is true', function() {
compileInput(
'<input type="text" ng-model="name" ' +
'ng-model-options="{ getterSetter: true }" />');
@@ -2117,6 +2117,12 @@ describe('input', function() {
expect(inputElm.val()).toBe('b');
expect(spy).toHaveBeenCalledWith('a');
expect(scope.name).toBe(spy);
});
it('should assign to non-function models if getterSetter is true', function() {
compileInput(
'<input type="text" ng-model="name" ' +
'ng-model-options="{ getterSetter: true }" />');
scope.name = 'c';
changeInputValueTo('d');
@@ -2136,6 +2142,35 @@ describe('input', function() {
'ng-model-options="{ getterSetter: true }" />');
});
it('should invoke a model in the correct context if getterSetter is true', function() {
compileInput(
'<input type="text" ng-model="someService.getterSetter" ' +
'ng-model-options="{ getterSetter: true }" />');
scope.someService = {
value: 'a',
getterSetter: function(newValue) {
this.value = newValue || this.value;
return this.value;
}
};
spyOn(scope.someService, 'getterSetter').andCallThrough();
scope.$apply();
expect(inputElm.val()).toBe('a');
expect(scope.someService.getterSetter).toHaveBeenCalledWith();
expect(scope.someService.value).toBe('a');
changeInputValueTo('b');
expect(scope.someService.getterSetter).toHaveBeenCalledWith('b');
expect(scope.someService.value).toBe('b');
scope.someService.value = 'c';
scope.$apply();
expect(inputElm.val()).toBe('c');
expect(scope.someService.getterSetter).toHaveBeenCalledWith();
});
it('should assign invalid values to the scope if allowInvalid is true', function() {
compileInput('<input type="text" name="input" ng-model="value" maxlength="1" ' +
'ng-model-options="{allowInvalid: true}" />');