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

Commit

Permalink
perf(input): prevent multiple validations on initialization
Browse files Browse the repository at this point in the history
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
  • Loading branch information
Narretz committed Dec 5, 2018
1 parent d855b74 commit 0637a21
Show file tree
Hide file tree
Showing 7 changed files with 594 additions and 68 deletions.
98 changes: 69 additions & 29 deletions src/ng/directive/input.js
Expand Up @@ -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);

Expand Down Expand Up @@ -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();
}
});
}

Expand Down Expand Up @@ -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();
}

});

}
}

Expand Down Expand Up @@ -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; } :
Expand All @@ -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; } :
Expand All @@ -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
Expand All @@ -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) {
Expand Down Expand Up @@ -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());
}
}
}
Expand Down
1 change: 1 addition & 0 deletions src/ng/directive/ngModel.js
Expand Up @@ -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;
Expand Down

0 comments on commit 0637a21

Please sign in to comment.