perf(input): prevent multiple validations on initialization

This commit updates in-built validators with observers to prevent
multiple calls to $validate that could happen on initial linking of the directives in
certain circumstances:

- when an input is wrapped in a transclude: element directive (e.g. ngRepeat),
the order of execution between ngModel and the input / validation directives changes so that
the initial observer call happens when ngModel has already been initalized,
leading to another call to $validate, which calls *all* defined validators again.
Without ngRepeat, ngModel hasn't been initialized yet, and $validate does not call the validators.

When using validators with scope expressions, the expression value is not available when
ngModel first runs the validators (e.g. ngMinlength="myMinlength"). Only in the first call to
the observer does the value become available, making a call to $validate a necessity.

This commit solves the first problem by storing the validation attribute value so we can compare
the current value and the observed value - which will be the same after compilation.

The second problem is solved by parsing the validation expression once in the link function,
so the value is available when ngModel first validates.

Closes #14691 
Closes #16760

Author: Narretz

src/ng/directive/input.js

@@ -1497,7 +1497,7 @@ function createDateParser(regexp, mapping) {
 }
 
 function createDateInputType(type, regexp, parseDate, format) {
-  return function dynamicDateInputType(scope, element, attr, ctrl, $sniffer, $browser, $filter) {
+  return function dynamicDateInputType(scope, element, attr, ctrl, $sniffer, $browser, $filter, $parse) {
     badInputChecker(scope, element, attr, ctrl, type);
     baseInputType(scope, element, attr, ctrl, $sniffer, $browser);
 
@@ -1540,24 +1540,34 @@ function createDateInputType(type, regexp, parseDate, format) {
     });
 
     if (isDefined(attr.min) || attr.ngMin) {
-      var minVal;
+      var minVal = attr.min || $parse(attr.ngMin)(scope);
+      var parsedMinVal = parseObservedDateValue(minVal);
+
       ctrl.$validators.min = function(value) {
-        return !isValidDate(value) || isUndefined(minVal) || parseDate(value) >= minVal;
+        return !isValidDate(value) || isUndefined(parsedMinVal) || parseDate(value) >= parsedMinVal;
       };
       attr.$observe('min', function(val) {
-        minVal = parseObservedDateValue(val);
-        ctrl.$validate();
+        if (val !== minVal) {
+          parsedMinVal = parseObservedDateValue(val);
+          minVal = val;
+          ctrl.$validate();
+        }
       });
     }
 
     if (isDefined(attr.max) || attr.ngMax) {
-      var maxVal;
+      var maxVal = attr.max || $parse(attr.ngMax)(scope);
+      var parsedMaxVal = parseObservedDateValue(maxVal);
+
       ctrl.$validators.max = function(value) {
-        return !isValidDate(value) || isUndefined(maxVal) || parseDate(value) <= maxVal;
+        return !isValidDate(value) || isUndefined(parsedMaxVal) || parseDate(value) <= parsedMaxVal;
       };
       attr.$observe('max', function(val) {
-        maxVal = parseObservedDateValue(val);
-        ctrl.$validate();
+        if (val !== maxVal) {
+          parsedMaxVal = parseObservedDateValue(val);
+          maxVal = val;
+          ctrl.$validate();
+        }
       });
     }
 
@@ -1709,50 +1719,68 @@ function isValidForStep(viewValue, stepBase, step) {
   return (value - stepBase) % step === 0;
 }
 
-function numberInputType(scope, element, attr, ctrl, $sniffer, $browser) {
+function numberInputType(scope, element, attr, ctrl, $sniffer, $browser, $filter, $parse) {
   badInputChecker(scope, element, attr, ctrl, 'number');
   numberFormatterParser(ctrl);
   baseInputType(scope, element, attr, ctrl, $sniffer, $browser);
 
-  var minVal;
-  var maxVal;
+  var parsedMinVal;
 
   if (isDefined(attr.min) || attr.ngMin) {
+    var minVal = attr.min || $parse(attr.ngMin)(scope);
+    parsedMinVal = parseNumberAttrVal(minVal);
+
     ctrl.$validators.min = function(modelValue, viewValue) {
-      return ctrl.$isEmpty(viewValue) || isUndefined(minVal) || viewValue >= minVal;
+      return ctrl.$isEmpty(viewValue) || isUndefined(parsedMinVal) || viewValue >= parsedMinVal;
     };
 
     attr.$observe('min', function(val) {
-      minVal = parseNumberAttrVal(val);
-      // TODO(matsko): implement validateLater to reduce number of validations
-      ctrl.$validate();
+      if (val !== minVal) {
+        parsedMinVal = parseNumberAttrVal(val);
+        minVal = val;
+        // TODO(matsko): implement validateLater to reduce number of validations
+        ctrl.$validate();
+      }
     });
   }
 
   if (isDefined(attr.max) || attr.ngMax) {
+    var maxVal = attr.max || $parse(attr.ngMax)(scope);
+    var parsedMaxVal = parseNumberAttrVal(maxVal);
+
     ctrl.$validators.max = function(modelValue, viewValue) {
-      return ctrl.$isEmpty(viewValue) || isUndefined(maxVal) || viewValue <= maxVal;
+      return ctrl.$isEmpty(viewValue) || isUndefined(parsedMaxVal) || viewValue <= parsedMaxVal;
     };
 
     attr.$observe('max', function(val) {
-      maxVal = parseNumberAttrVal(val);
-      // TODO(matsko): implement validateLater to reduce number of validations
-      ctrl.$validate();
+      if (val !== maxVal) {
+        parsedMaxVal = parseNumberAttrVal(val);
+        maxVal = val;
+        // TODO(matsko): implement validateLater to reduce number of validations
+        ctrl.$validate();
+      }
     });
   }
 
   if (isDefined(attr.step) || attr.ngStep) {
-    var stepVal;
+    var stepVal = attr.step || $parse(attr.ngStep)(scope);
+    var parsedStepVal = parseNumberAttrVal(stepVal);
+
     ctrl.$validators.step = function(modelValue, viewValue) {
-      return ctrl.$isEmpty(viewValue) || isUndefined(stepVal) ||
-             isValidForStep(viewValue, minVal || 0, stepVal);
+      return ctrl.$isEmpty(viewValue) || isUndefined(parsedStepVal) ||
+        isValidForStep(viewValue, parsedMinVal || 0, parsedStepVal);
     };
 
     attr.$observe('step', function(val) {
-      stepVal = parseNumberAttrVal(val);
       // TODO(matsko): implement validateLater to reduce number of validations
-      ctrl.$validate();
+      if (val !== stepVal) {
+        parsedStepVal = parseNumberAttrVal(val);
+        stepVal = val;
+        ctrl.$validate();
+      }
+
     });
+
   }
 }
 
@@ -1782,6 +1810,8 @@ function rangeInputType(scope, element, attr, ctrl, $sniffer, $browser) {
     originalRender;
 
   if (hasMinAttr) {
+    minVal = parseNumberAttrVal(attr.min);
+
     ctrl.$validators.min = supportsRange ?
       // Since all browsers set the input to a valid value, we don't need to check validity
       function noopMinValidator() { return true; } :
@@ -1794,6 +1824,8 @@ function rangeInputType(scope, element, attr, ctrl, $sniffer, $browser) {
   }
 
   if (hasMaxAttr) {
+    maxVal = parseNumberAttrVal(attr.max);
+
     ctrl.$validators.max = supportsRange ?
       // Since all browsers set the input to a valid value, we don't need to check validity
       function noopMaxValidator() { return true; } :
@@ -1806,6 +1838,8 @@ function rangeInputType(scope, element, attr, ctrl, $sniffer, $browser) {
   }
 
   if (hasStepAttr) {
+    stepVal = parseNumberAttrVal(attr.step);
+
     ctrl.$validators.step = supportsRange ?
       function nativeStepValidator() {
         // Currently, only FF implements the spec on step change correctly (i.e. adjusting the
@@ -1827,7 +1861,13 @@ function rangeInputType(scope, element, attr, ctrl, $sniffer, $browser) {
     // attribute value when the input is first rendered, so that the browser can adjust the
     // input value based on the min/max value
     element.attr(htmlAttrName, attr[htmlAttrName]);
-    attr.$observe(htmlAttrName, changeFn);
+    var oldVal = attr[htmlAttrName];
+    attr.$observe(htmlAttrName, function wrappedObserver(val) {
+      if (val !== oldVal) {
+        oldVal = val;
+        changeFn(val);
+      }
+    });
   }
 
   function minChange(val) {
@@ -1881,11 +1921,11 @@ function rangeInputType(scope, element, attr, ctrl, $sniffer, $browser) {
     }
 
     // Some browsers don't adjust the input value correctly, but set the stepMismatch error
-    if (supportsRange && ctrl.$viewValue !== element.val()) {
-      ctrl.$setViewValue(element.val());
-    } else {
+    if (!supportsRange) {
       // TODO(matsko): implement validateLater to reduce number of validations
       ctrl.$validate();
+    } else if (ctrl.$viewValue !== element.val()) {
+      ctrl.$setViewValue(element.val());
     }
   }
 }

src/ng/directive/ngModel.js

@@ -562,6 +562,7 @@ NgModelController.prototype = {
    * `$modelValue`, i.e. either the last parsed value or the last value set from the scope.
    */
   $validate: function() {
+
     // ignore $validate before model is initialized
     if (isNumberNaN(this.$modelValue)) {
       return;

src/ng/directive/validators.js

@@ -62,24 +62,29 @@
  *   </file>
  * </example>
  */
-var requiredDirective = function() {
+var requiredDirective = ['$parse', function($parse) {
   return {
     restrict: 'A',
     require: '?ngModel',
     link: function(scope, elm, attr, ctrl) {
       if (!ctrl) return;
+      var oldVal = attr.required || $parse(attr.ngRequired)(scope);
+
       attr.required = true; // force truthy in case we are on non input element
 
       ctrl.$validators.required = function(modelValue, viewValue) {
         return !attr.required || !ctrl.$isEmpty(viewValue);
       };
 
-      attr.$observe('required', function() {
-        ctrl.$validate();
+      attr.$observe('required', function(val) {
+        if (oldVal !== val) {
+          oldVal = val;
+          ctrl.$validate();
+        }
       });
     }
   };
-};
+}];
 
 /**
  * @ngdoc directive
@@ -162,36 +167,59 @@ var requiredDirective = function() {
  *   </file>
  * </example>
  */
-var patternDirective = function() {
+var patternDirective = ['$parse', function($parse) {
   return {
     restrict: 'A',
     require: '?ngModel',
-    link: function(scope, elm, attr, ctrl) {
-      if (!ctrl) return;
+    compile: function(tElm, tAttr) {
+      var patternExp;
+      var parseFn;
+
+      if (tAttr.ngPattern) {
+        patternExp = tAttr.ngPattern;
 
-      var regexp, patternExp = attr.ngPattern || attr.pattern;
-      attr.$observe('pattern', function(regex) {
-        if (isString(regex) && regex.length > 0) {
-          regex = new RegExp('^' + regex + '$');
+        // ngPattern might be a scope expression, or an inlined regex, which is not parsable.
+        // We get value of the attribute here, so we can compare the old and the new value
+        // in the observer to avoid unnecessary validations
+        if (tAttr.ngPattern.charAt(0) === '/' && REGEX_STRING_REGEXP.test(tAttr.ngPattern)) {
+          parseFn = function() { return tAttr.ngPattern; };
+        } else {
+          parseFn = $parse(tAttr.ngPattern);
         }
+      }
+
+      return function(scope, elm, attr, ctrl) {
+        if (!ctrl) return;
 
-        if (regex && !regex.test) {
-          throw minErr('ngPattern')('noregexp',
-            'Expected {0} to be a RegExp but was {1}. Element: {2}', patternExp,
-            regex, startingTag(elm));
+        var attrVal = attr.pattern;
+
+        if (attr.ngPattern) {
+          attrVal = parseFn(scope);
+        } else {
+          patternExp = attr.pattern;
         }
 
-        regexp = regex || undefined;
-        ctrl.$validate();
-      });
+        var regexp = parsePatternAttr(attrVal, patternExp, elm);
+
+        attr.$observe('pattern', function(newVal) {
+          var oldRegexp = regexp;
 
-      ctrl.$validators.pattern = function(modelValue, viewValue) {
-        // HTML5 pattern constraint validates the input value, so we validate the viewValue
-        return ctrl.$isEmpty(viewValue) || isUndefined(regexp) || regexp.test(viewValue);
+          regexp = parsePatternAttr(newVal, patternExp, elm);
+
+          if ((oldRegexp && oldRegexp.toString()) !== (regexp && regexp.toString())) {
+            ctrl.$validate();
+          }
+        });
+
+        ctrl.$validators.pattern = function(modelValue, viewValue) {
+          // HTML5 pattern constraint validates the input value, so we validate the viewValue
+          return ctrl.$isEmpty(viewValue) || isUndefined(regexp) || regexp.test(viewValue);
+        };
       };
     }
+
   };
-};
+}];
 
 /**
  * @ngdoc directive
@@ -264,25 +292,29 @@ var patternDirective = function() {
  *   </file>
  * </example>
  */
-var maxlengthDirective = function() {
+var maxlengthDirective = ['$parse', function($parse) {
   return {
     restrict: 'A',
     require: '?ngModel',
     link: function(scope, elm, attr, ctrl) {
       if (!ctrl) return;
 
-      var maxlength = -1;
+      var maxlength = attr.maxlength || $parse(attr.ngMaxlength)(scope);
+      var maxlengthParsed = parseLength(maxlength);
+
       attr.$observe('maxlength', function(value) {
-        var intVal = toInt(value);
-        maxlength = isNumberNaN(intVal) ? -1 : intVal;
-        ctrl.$validate();
+        if (maxlength !== value) {
+          maxlengthParsed = parseLength(value);
+          maxlength = value;
+          ctrl.$validate();
+        }
       });
       ctrl.$validators.maxlength = function(modelValue, viewValue) {
-        return (maxlength < 0) || ctrl.$isEmpty(viewValue) || (viewValue.length <= maxlength);
+        return (maxlengthParsed < 0) || ctrl.$isEmpty(viewValue) || (viewValue.length <= maxlengthParsed);
       };
     }
   };
-};
+}];
 
 /**
  * @ngdoc directive
@@ -353,21 +385,49 @@ var maxlengthDirective = function() {
  *   </file>
  * </example>
  */
-var minlengthDirective = function() {
+var minlengthDirective = ['$parse', function($parse) {
   return {
     restrict: 'A',
     require: '?ngModel',
     link: function(scope, elm, attr, ctrl) {
       if (!ctrl) return;
 
-      var minlength = 0;
+      var minlength = attr.minlength || $parse(attr.ngMinlength)(scope);
+      var minlengthParsed = parseLength(minlength) || -1;
+
       attr.$observe('minlength', function(value) {
-        minlength = toInt(value) || 0;
-        ctrl.$validate();
+        if (minlength !== value) {
+          minlengthParsed = parseLength(value) || -1;
+          minlength = value;
+          ctrl.$validate();
+        }
+
       });
       ctrl.$validators.minlength = function(modelValue, viewValue) {
-        return ctrl.$isEmpty(viewValue) || viewValue.length >= minlength;
+        return ctrl.$isEmpty(viewValue) || viewValue.length >= minlengthParsed;
       };
     }
   };
-};
+}];
+
+
+function parsePatternAttr(regex, patternExp, elm) {
+  if (!regex) return undefined;
+
+  if (isString(regex)) {
+    regex = new RegExp('^' + regex + '$');
+  }
+
+  if (!regex.test) {
+    throw minErr('ngPattern')('noregexp',
+      'Expected {0} to be a RegExp but was {1}. Element: {2}', patternExp,
+      regex, startingTag(elm));
+  }
+
+  return regex;
+}
+
+function parseLength(val) {
+  var intVal = toInt(val);
+  return isNumberNaN(intVal) ? -1 : intVal;
+}

test/helpers/testabilityPatch.js

@@ -312,7 +312,28 @@ window.dump = function() {
 
 function generateInputCompilerHelper(helper) {
   beforeEach(function() {
+    helper.validationCounter = {};
+
     module(function($compileProvider) {
+      $compileProvider.directive('validationSpy', function() {
+        return {
+          priority: 1,
+          require: 'ngModel',
+          link: function(scope, element, attrs, ctrl) {
+            var validationName = attrs.validationSpy;
+
+            var originalValidator = ctrl.$validators[validationName];
+            helper.validationCounter[validationName] = 0;
+
+            ctrl.$validators[validationName] = function(modelValue, viewValue) {
+              helper.validationCounter[validationName]++;
+
+              return originalValidator(modelValue, viewValue);
+            };
+          }
+        };
+      });
+
       $compileProvider.directive('attrCapture', function() {
         return function(scope, element, $attrs) {
           helper.attrs = $attrs;

test/ng/directive/inputSpec.js

@@ -863,7 +863,6 @@ describe('ngModel', function() {
         });
       });
 
-
       describe('view -> model update', function() {
 
         it('should always perform validations using the parsed model value', function() {

test/ng/directive/ngModelSpec.js

@@ -230,6 +230,29 @@ describe('validators', function() {
       expect(ctrl.$error.pattern).toBe(true);
       expect(ctrlNg.$error.pattern).toBe(true);
     }));
+
+    it('should only validate once after compilation when inside ngRepeat', function() {
+
+      $rootScope.pattern = /\d{4}/;
+
+      helper.compileInput(
+         '<div ng-repeat="input in [0]">' +
+           '<input type="text" ng-model="value" pattern="\\d{4}" validation-spy="pattern" />' +
+         '</div>');
+
+      $rootScope.$digest();
+
+      expect(helper.validationCounter.pattern).toBe(1);
+
+      helper.compileInput(
+         '<div ng-repeat="input in [0]">' +
+           '<input type="text" ng-model="value" ng-pattern="pattern" validation-spy="pattern" />' +
+         '</div>');
+
+      $rootScope.$digest();
+
+      expect(helper.validationCounter.pattern).toBe(1);
+    });
   });
 
 
@@ -312,9 +335,31 @@ describe('validators', function() {
       expect(ctrl.$error.minlength).toBe(true);
       expect(ctrlNg.$error.minlength).toBe(true);
     }));
-  });
 
 
+    it('should only validate once after compilation when inside ngRepeat', function() {
+      $rootScope.minlength = 5;
+
+      var element = helper.compileInput(
+         '<div ng-repeat="input in [0]">' +
+           '<input type="text" ng-model="value" minlength="{{minlength}}" validation-spy="minlength" />' +
+         '</div>');
+
+      $rootScope.$digest();
+
+      expect(helper.validationCounter.minlength).toBe(1);
+
+      element = helper.compileInput(
+         '<div ng-repeat="input in [0]">' +
+           '<input type="text" ng-model="value" ng-minlength="minlength" validation-spy="minlength" />' +
+         '</div>');
+
+      $rootScope.$digest();
+
+      expect(helper.validationCounter.minlength).toBe(1);
+    });
+  });
+
   describe('maxlength', function() {
 
     it('should invalidate values that are longer than the given maxlength', function() {
@@ -500,6 +545,29 @@ describe('validators', function() {
       expect(ctrl.$error.maxlength).toBe(true);
       expect(ctrlNg.$error.maxlength).toBe(true);
     }));
+
+
+    it('should only validate once after compilation when inside ngRepeat', function() {
+      $rootScope.maxlength = 5;
+
+      var element = helper.compileInput(
+         '<div ng-repeat="input in [0]">' +
+           '<input type="text" ng-model="value" maxlength="{{maxlength}}" validation-spy="maxlength" />' +
+         '</div>');
+
+      $rootScope.$digest();
+
+      expect(helper.validationCounter.maxlength).toBe(1);
+
+      element = helper.compileInput(
+         '<div ng-repeat="input in [0]">' +
+           '<input type="text" ng-model="value" ng-maxlength="maxlength" validation-spy="maxlength" />' +
+         '</div>');
+
+      $rootScope.$digest();
+
+      expect(helper.validationCounter.maxlength).toBe(1);
+    });
   });
 
 
@@ -626,5 +694,41 @@ describe('validators', function() {
       expect(ctrl.$error.required).toBe(true);
       expect(ctrlNg.$error.required).toBe(true);
     }));
+
+
+    it('should validate only once after compilation when inside ngRepeat', function() {
+      helper.compileInput(
+         '<div ng-repeat="input in [0]">' +
+           '<input type="text" ng-model="value" required validation-spy="required" />' +
+         '</div>');
+
+      $rootScope.$digest();
+
+      expect(helper.validationCounter.required).toBe(1);
+    });
+
+
+    it('should validate only once after compilation when inside ngRepeat and ngRequired is true', function() {
+      $rootScope.isRequired = true;
+
+      helper.compileInput(
+         '<div ng-repeat="input in [0]">' +
+           '<input type="text" ng-model="value" ng-required="isRequired" validation-spy="required" />' +
+         '</div>');
+
+      expect(helper.validationCounter.required).toBe(1);
+    });
+
+
+    it('should validate only once after compilation when inside ngRepeat and ngRequired is false', function() {
+      $rootScope.isRequired = false;
+
+      helper.compileInput(
+         '<div ng-repeat="input in [0]">' +
+           '<input type="text" ng-model="value" ng-required="isRequired" validation-spy="required" />' +
+         '</div>');
+
+      expect(helper.validationCounter.required).toBe(1);
+    });
   });
 });