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

fix(ngModel): allow setting model to NaN when asyncValidator is present #11411

Closed
wants to merge 1 commit into from
Closed

fix(ngModel): allow setting model to NaN when asyncValidator is present #11411

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 4 additions & 1 deletion src/ng/directive/ngModel.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -810,7 +810,10 @@ var NgModelController = ['$scope', '$exceptionHandler', '$attrs', '$element', '$

// if scope model value and ngModel value are out of sync
// TODO(perf): why not move this to the action fn?
if (modelValue !== ctrl.$modelValue) {
// Second check is needed to allow setting the model to NaN when there's an asyncValidator
if (modelValue !== ctrl.$modelValue &&
(ctrl.$modelValue === ctrl.$modelValue || modelValue === modelValue)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we use !isNaN(...) here?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Quick tests shows that isNaN does not work, but Number,isNaN (ECMA6) does. :(

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

isNaN(undefined) === true!

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Don't take it personally, but we all are NaN: isNaN('@petebacondarwin') === true 😃

) {
ctrl.$modelValue = ctrl.$$rawModelValue = modelValue;
parserValid = undefined;

Expand Down
24 changes: 24 additions & 0 deletions test/ng/directive/ngModelSpec.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -578,6 +578,30 @@ describe('ngModel', function() {

dealoc(form);
}));


it('should set NaN as the $modelValue when an asyncValidator is present',
inject(function($q) {

ctrl.$asyncValidators.test = function() {
return $q(function(resolve, reject) {
resolve();
});
};

scope.$apply('value = 10');
expect(ctrl.$modelValue).toBe(10);

expect(function() {
scope.$apply(function() {
scope.value = NaN;
});
}).not.toThrowMinErr('$rootScope', 'infdig', '10 $digest() iterations reached. Aborting!\n' +
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would prefer a plain not.toThrow(), because this test might become useless if the minErr changes.

'Watchers fired in the last 5 iterations: []');

expect(ctrl.$modelValue).toBeNaN();

}));
});


Expand Down