ngModelController $parsers function not called consistently

[visnup]

When enforcing input values to conform to certain rules using an ngModelController $parser function, the $parser function isn't called on repeated "invalid" keystrokes, which allows invalid characters to get added.

In the example below, the directive is meant to remove all non-digit characters. it mostly works, but if you type the same non-digit character twice in a row (e.g. 'xx'), a single 'x' will show up in the input. if you type 'x' a third time, it will remove the 'x'.

I've also included a failing test case.

all code is running at http://codepen.io/visnup/pen/YXgLVq?editors=101

directive usage

<div ng-app="digits">
  <input ng-model="number" placeholder="digits only" digits-only />
</div>

directive

angular
  .module('digits', [])
  .directive('digitsOnly', function() {
    return {
      require: 'ngModel',
      link: function link(scope, element, attrs, ngModel) {
        ngModel.$parsers.push(function(value) {
          var numbers = value.replace(/\D/g, '');
          element.val(numbers);
          return numbers;
        });
      }
    };
  });

failing test case

describe('digits', function() {
  beforeEach(angular.mock.module('digits'));

  let scope, input;
  beforeEach(inject(function($compile, $rootScope) {
    scope = $rootScope;
    input = $compile('<input ng-model="number" digits-only />')(scope);
  }));

  // works
  it('should block non-digits', function() {
    input.val('111x');
    input.triggerHandler('input');
    expect(input.val()).to.equal('111');
  });

  // doesn't work
  it('should not flash incorrect input', function() {
    input.val('111x');
    input.triggerHandler('input');
    input.val('111x');
    input.triggerHandler('input');
    expect(input.val()).to.equal('111');
  });
});

This has been brought up before in #10700.

[gkalpak]

$parsers is for transforming (a copy of) the $viewValue before "saving" it as $modelValue (and that is what your example is doing). It is not actually changing the $viewValue (although it does update the element's value), which leads to unexpected behaviour.

This is better illustrated with an example:

1. User enters '1'.
   • Before parsing:
     • $modelValue: undefined
     • $viewValue: 1
     • element.val(): 1
   • After parsing:
     • $modelValue: 1
     • $viewValue: 1
     • element.val(): 1
2. User enters 'x'.
   • Before parsing:
     • $modelValue: 1
     • $viewValue: 1x
     • element.val(): 1x
   • After parsing:
     • $modelValue: 1
     • $viewValue: 1x (never updated from parser fn)
     • element.val(): 1 (updated from parser fn)
3. User enters 'x'.
   • Before parsing:
     • $modelValue: 1
     • $viewValue: 1x (No change detected, since previous $viewValue was also 1x)
     • element.val(): 1x
   • No parsing (since no change detected in $viewValue) !!!
     • $modelValue: 1
     • $viewValue: 1x
     • element.val(): 1x
4. User enters 'x'.
   • Before parsing:
     • $modelValue: 1
     • $viewValue: 1xx
     • element.val(): 1xx
   • After parsing:
     • $modelValue: 1
     • $viewValue: 1xx
     • element.val(): 1
5. (...back to step (2)...)

There are several "Angular" ways to properly handle this, e.g.:

ngModel.$parsers.push(function (value) {
  var numbers = value.replace(/\D/g, '');
  if (numbers !== value) {
    ngModel.$setViewValue(numbers);   // Update the `$viewValue`
    ngModel.$render();                // Update the element's displayed value
  }
  return numbers;
});

Updated pen

---

I am closing this, as everything seems to work as expected.
@visnup, feel free to follow up with questions if anything isn't clear.

[matthewerwin]

This problem occurs with repeat keystrokes any time that new $viewValue is not committed. The suggested use of $setViewValue in the $parsers collection is sometimes avoided with a preference to directly setting $viewValue in some forums.

The following would also work:

ngModel.$viewValue = numbers;   // Update the '$viewValue'
ngModel.$commitViewValue(); //update $$lastCommittedViewValue
ngModel.$render();                // Update the element's displayed value

If you want to avoid any recursion pitfalls at all then $$lastCommittedViewValue could be updated directly if you don't mind violating encapsulation. Unfortunately when infinite recursion does occur in these functions I've seen cases where it isn't reported to the console even after max occurrences. Worth taking the few minutes to put a break point in during testing and verify any recursions.

[jeserkin]

@gkalpak and @snaptech explanations combined together make a golden solution :) 👍