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

Setting model to NaN causes infdig if $asyncValidators is used #11315

Closed
tomyam1 opened this issue Mar 13, 2015 · 4 comments
Closed

Setting model to NaN causes infdig if $asyncValidators is used #11315

tomyam1 opened this issue Mar 13, 2015 · 4 comments
Assignees
@Narretz
Labels
component: forms frequency: low severity: broken expected use type: bug
Milestone
1.4.0-rc.0 / 1.3.17

Comments

@tomyam1
Copy link
Contributor

tomyam1 commented Mar 13, 2015

An infinite digest error is thrown when a model is set to NaN and there is an asynchronous validator.

Please check plunker at http://plnkr.co/edit/u9Sq12?p=preview.
When we set model to a string or to null - everything works as expected.
When we set model to NaN there is an infdig error:

Error: [$rootScope:infdig] 10 $digest() iterations reached. Aborting!
Watchers fired in the last 5 iterations: []
http://errors.angularjs.org/1.3.14/$rootScope/infdig?p0=10&p1=%5B%5D
    at angular.js:63
    at Scope.$digest (angular.js:14281)
    at Scope.$apply (angular.js:14506)
    at HTMLButtonElement.<anonymous> (angular.js:21443)
    at HTMLButtonElement.eventHandler (angular.js:3014)
angular.js:63 Uncaught Error: [$rootScope:infdig] 10 $digest() iterations reached. Aborting!
Watchers fired in the last 5 iterations: []
http://errors.angularjs.org/1.3.14/$rootScope/infdig?p0=10&p1=%5B%5D

This happens because ngModelWatch checks that modelValue !== ctrl.$modelValue, which is true for NaN.

Thus, when we set the model to NaN, $$runValidators is called in every digest cycle. If we have an async validator, it causes a push to asyncQueue and so another cycle is needed.

ngModelWatch returns modelValue, which is NaN so the watch is not considered dirty because $digest() treats NaN as equal to NaN.

I think a good solution would be to treat NaN as equal to NaN in ngModelWatch
Edit:
Check http://plnkr.co/edit/aZZmyP?p=preview
ngModelWatch is changed to

if (modelValue !== ctrl.$modelValue &&
      !(typeof modelValue === 'number' && typeof ctrl.$modelValue === 'number' && isNaN(modelValue) && isNaN(ctrl.$modelValue))) {
      ctrl.$modelValue = ctrl.$$rawModelValue = modelValue;

and this indeed solved the infdig
@zhongsp
Copy link

zhongsp commented Mar 15, 2015

In ngModelWatch , we could update to modelValue and ctrl.$modelValue are not equal, and any of both is not NaN.

    // if (modelValue !== ctrl.$modelValue)
    if (modelValue !== ctrl.$modelValue && 
        (modelValue === modelValue || ctrl.$modelValue === ctrl.$modelValue)) {

@wesleycho
Copy link
Contributor

It would be better to do

if (modelValue !== ctrl.$modelValue &&
  equals(modelValue, modelValue) || equals(ctrl.$modelValue, ctrl.$modelValue)) {
...
}

I think - angular.equals already has a check for NaN, so it would screen out this case and do a more proper check.

@Narretz Narretz added this to the 1.4.0-rc.0 / 1.3.17 milestone Mar 19, 2015
@Narretz Narretz self-assigned this Mar 19, 2015
@gkalpak
Copy link
Member

gkalpak commented Mar 22, 2015

I don't think using the (potentially expensive) equals in a rather critical path is a good idea.
@zhongsp's suggestion seems fine though.

@Narretz Narretz mentioned this issue Mar 24, 2015
Closed
@Narretz
Copy link
Contributor

Narretz commented Mar 24, 2015

I opened #11411 for your consideration. equals is too expensive, but @zhongsp solution works.

Narretz added a commit to Narretz/angular.js that referenced this issue Mar 24, 2015
@Narretz
fix(ngModel): allow setting model to NaN when asyncValidator is present
014e8cd 
Closes angular#11315
Fixes angular#11411
@Narretz Narretz closed this as completed in 948120e Apr 3, 2015
Narretz added a commit that referenced this issue Apr 3, 2015
@Narretz @petebacondarwin
fix(ngModel): allow setting model to NaN when asyncValidator is present
b64519f 
Closes #11315
Closes #11411
netman92 pushed a commit to netman92/angular.js that referenced this issue Aug 8, 2015
@Narretz @netman92
fix(ngModel): allow setting model to NaN when asyncValidator is present
da56110 
Closes angular#11315
Closes angular#11411
@jumph4x jumph4x mentioned this issue Jul 24, 2016
Closed
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Assignees

@Narretz Narretz

Labels
component: forms frequency: low severity: broken expected use type: bug
Projects
None yet
Milestone
1.4.0-rc.0 / 1.3.17
Development

Successfully merging a pull request may close this issue.

fix(ngModel): allow setting model to NaN when asyncValidator is present Narretz/angular.js
5 participants
@Narretz @zhongsp @wesleycho @tomyam1 @gkalpak