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

Commit f2aa98a

Browse files
committed
perf(input): prevent multiple validations on compilation
This commit updates in-built validators with observers to prevent multiple calls to $validate that could happen after compilation 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
1 parent b4e409b commit f2aa98a

File tree

9 files changed

+618
-60
lines changed

9 files changed

+618
-60
lines changed

src/ng/directive/input.js

Lines changed: 70 additions & 29 deletions
Original file line numberDiff line numberDiff line change
@@ -1497,7 +1497,7 @@ function createDateParser(regexp, mapping) {
14971497
}
14981498

14991499
function createDateInputType(type, regexp, parseDate, format) {
1500-
return function dynamicDateInputType(scope, element, attr, ctrl, $sniffer, $browser, $filter) {
1500+
return function dynamicDateInputType(scope, element, attr, ctrl, $sniffer, $browser, $filter, $parse) {
15011501
badInputChecker(scope, element, attr, ctrl, type);
15021502
baseInputType(scope, element, attr, ctrl, $sniffer, $browser);
15031503

@@ -1540,24 +1540,35 @@ function createDateInputType(type, regexp, parseDate, format) {
15401540
});
15411541

15421542
if (isDefined(attr.min) || attr.ngMin) {
1543-
var minVal;
1543+
var minVal = attr.min || $parse(attr.ngMin)(scope);
1544+
var parsedMinVal = parseObservedDateValue(minVal);
1545+
15441546
ctrl.$validators.min = function(value) {
1545-
return !isValidDate(value) || isUndefined(minVal) || parseDate(value) >= minVal;
1547+
return !isValidDate(value) || isUndefined(parsedMinVal) || parseDate(value) >= parsedMinVal;
15461548
};
15471549
attr.$observe('min', function(val) {
1548-
minVal = parseObservedDateValue(val);
1549-
ctrl.$validate();
1550+
if (val !== minVal) {
1551+
parsedMinVal = parseObservedDateValue(val);
1552+
ctrl.$validate();
1553+
}
1554+
minVal = val;
15501555
});
15511556
}
15521557

15531558
if (isDefined(attr.max) || attr.ngMax) {
1554-
var maxVal;
1559+
var maxVal = attr.max || $parse(attr.ngMax)(scope);
1560+
var parsedMaxVal = parseObservedDateValue(maxVal);
1561+
15551562
ctrl.$validators.max = function(value) {
1556-
return !isValidDate(value) || isUndefined(maxVal) || parseDate(value) <= maxVal;
1563+
return !isValidDate(value) || isUndefined(parsedMaxVal) || parseDate(value) <= parsedMaxVal;
15571564
};
15581565
attr.$observe('max', function(val) {
1559-
maxVal = parseObservedDateValue(val);
1560-
ctrl.$validate();
1566+
if (val !== maxVal) {
1567+
parsedMaxVal = parseObservedDateValue(val);
1568+
ctrl.$validate();
1569+
}
1570+
1571+
maxVal = val;
15611572
});
15621573
}
15631574

@@ -1709,50 +1720,68 @@ function isValidForStep(viewValue, stepBase, step) {
17091720
return (value - stepBase) % step === 0;
17101721
}
17111722

1712-
function numberInputType(scope, element, attr, ctrl, $sniffer, $browser) {
1723+
function numberInputType(scope, element, attr, ctrl, $sniffer, $browser, $filter, $parse) {
17131724
badInputChecker(scope, element, attr, ctrl, 'number');
17141725
numberFormatterParser(ctrl);
17151726
baseInputType(scope, element, attr, ctrl, $sniffer, $browser);
17161727

1717-
var minVal;
1718-
var maxVal;
1728+
var parsedMinVal;
17191729

17201730
if (isDefined(attr.min) || attr.ngMin) {
1731+
var minVal = attr.min || $parse(attr.ngMin)(scope);
1732+
parsedMinVal = parseNumberAttrVal(minVal);
1733+
17211734
ctrl.$validators.min = function(modelValue, viewValue) {
1722-
return ctrl.$isEmpty(viewValue) || isUndefined(minVal) || viewValue >= minVal;
1735+
return ctrl.$isEmpty(viewValue) || isUndefined(parsedMinVal) || viewValue >= parsedMinVal;
17231736
};
17241737

17251738
attr.$observe('min', function(val) {
1726-
minVal = parseNumberAttrVal(val);
1727-
// TODO(matsko): implement validateLater to reduce number of validations
1728-
ctrl.$validate();
1739+
if (val !== minVal) {
1740+
parsedMinVal = parseNumberAttrVal(val);
1741+
// TODO(matsko): implement validateLater to reduce number of validations
1742+
ctrl.$validate();
1743+
}
1744+
minVal = val;
17291745
});
17301746
}
17311747

17321748
if (isDefined(attr.max) || attr.ngMax) {
1749+
var maxVal = attr.max || $parse(attr.ngMax)(scope);
1750+
var parsedMaxVal = parseNumberAttrVal(maxVal);
1751+
17331752
ctrl.$validators.max = function(modelValue, viewValue) {
1734-
return ctrl.$isEmpty(viewValue) || isUndefined(maxVal) || viewValue <= maxVal;
1753+
return ctrl.$isEmpty(viewValue) || isUndefined(parsedMaxVal) || viewValue <= parsedMaxVal;
17351754
};
17361755

17371756
attr.$observe('max', function(val) {
1738-
maxVal = parseNumberAttrVal(val);
1739-
// TODO(matsko): implement validateLater to reduce number of validations
1740-
ctrl.$validate();
1757+
if (val !== maxVal) {
1758+
parsedMaxVal = parseNumberAttrVal(val);
1759+
// TODO(matsko): implement validateLater to reduce number of validations
1760+
ctrl.$validate();
1761+
}
1762+
maxVal = val;
17411763
});
17421764
}
17431765

17441766
if (isDefined(attr.step) || attr.ngStep) {
1745-
var stepVal;
1767+
var stepVal = attr.step || $parse(attr.ngStep)(scope);
1768+
var parsedStepVal = parseNumberAttrVal(stepVal);
1769+
17461770
ctrl.$validators.step = function(modelValue, viewValue) {
1747-
return ctrl.$isEmpty(viewValue) || isUndefined(stepVal) ||
1748-
isValidForStep(viewValue, minVal || 0, stepVal);
1771+
return ctrl.$isEmpty(viewValue) || isUndefined(parsedStepVal) ||
1772+
isValidForStep(viewValue, parsedMinVal || 0, parsedStepVal);
17491773
};
17501774

17511775
attr.$observe('step', function(val) {
1752-
stepVal = parseNumberAttrVal(val);
17531776
// TODO(matsko): implement validateLater to reduce number of validations
1754-
ctrl.$validate();
1777+
if (stepVal !== val) {
1778+
parsedStepVal = parseNumberAttrVal(val);
1779+
ctrl.$validate();
1780+
}
1781+
1782+
stepVal = val;
17551783
});
1784+
17561785
}
17571786
}
17581787

@@ -1782,6 +1811,8 @@ function rangeInputType(scope, element, attr, ctrl, $sniffer, $browser) {
17821811
originalRender;
17831812

17841813
if (hasMinAttr) {
1814+
minVal = parseNumberAttrVal(attr.min);
1815+
17851816
ctrl.$validators.min = supportsRange ?
17861817
// Since all browsers set the input to a valid value, we don't need to check validity
17871818
function noopMinValidator() { return true; } :
@@ -1794,6 +1825,8 @@ function rangeInputType(scope, element, attr, ctrl, $sniffer, $browser) {
17941825
}
17951826

17961827
if (hasMaxAttr) {
1828+
maxVal = parseNumberAttrVal(attr.max);
1829+
17971830
ctrl.$validators.max = supportsRange ?
17981831
// Since all browsers set the input to a valid value, we don't need to check validity
17991832
function noopMaxValidator() { return true; } :
@@ -1806,6 +1839,8 @@ function rangeInputType(scope, element, attr, ctrl, $sniffer, $browser) {
18061839
}
18071840

18081841
if (hasStepAttr) {
1842+
stepVal = parseNumberAttrVal(attr.step);
1843+
18091844
ctrl.$validators.step = supportsRange ?
18101845
function nativeStepValidator() {
18111846
// Currently, only FF implements the spec on step change correctly (i.e. adjusting the
@@ -1827,7 +1862,13 @@ function rangeInputType(scope, element, attr, ctrl, $sniffer, $browser) {
18271862
// attribute value when the input is first rendered, so that the browser can adjust the
18281863
// input value based on the min/max value
18291864
element.attr(htmlAttrName, attr[htmlAttrName]);
1830-
attr.$observe(htmlAttrName, changeFn);
1865+
var oldVal = attr[htmlAttrName];
1866+
attr.$observe(htmlAttrName, function wrappedObserver(val) {
1867+
if (val !== oldVal) {
1868+
changeFn(val);
1869+
}
1870+
oldVal = val;
1871+
});
18311872
}
18321873

18331874
function minChange(val) {
@@ -1881,11 +1922,11 @@ function rangeInputType(scope, element, attr, ctrl, $sniffer, $browser) {
18811922
}
18821923

18831924
// Some browsers don't adjust the input value correctly, but set the stepMismatch error
1884-
if (supportsRange && ctrl.$viewValue !== element.val()) {
1885-
ctrl.$setViewValue(element.val());
1886-
} else {
1925+
if (!supportsRange) {
18871926
// TODO(matsko): implement validateLater to reduce number of validations
18881927
ctrl.$validate();
1928+
} else if (ctrl.$viewValue !== element.val()) {
1929+
ctrl.$setViewValue(element.val());
18891930
}
18901931
}
18911932
}

src/ng/directive/ngModel.js

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -562,8 +562,10 @@ NgModelController.prototype = {
562562
* `$modelValue`, i.e. either the last parsed value or the last value set from the scope.
563563
*/
564564
$validate: function() {
565+
565566
// ignore $validate before model is initialized
566567
if (isNumberNaN(this.$modelValue)) {
568+
// console.log('dont validate yet');
567569
return;
568570
}
569571

@@ -1050,6 +1052,7 @@ NgModelController.prototype = {
10501052
* This method is called internally when the bound scope value changes.
10511053
*/
10521054
$$setModelValue: function(modelValue) {
1055+
// console.log('$$setModelValue', modelValue);
10531056
this.$modelValue = this.$$rawModelValue = modelValue;
10541057
this.$$parserValid = undefined;
10551058
this.$processModelValue();
@@ -1081,6 +1084,7 @@ function setupModelWatcher(ctrl) {
10811084
// ng-change executes in apply phase
10821085
// 4. view should be changed back to 'a'
10831086
ctrl.$$scope.$watch(function ngModelWatch(scope) {
1087+
// console.log('ngModelWatch exp');
10841088
var modelValue = ctrl.$$ngModelGet(scope);
10851089

10861090
// if scope model value and ngModel value are out of sync

src/ng/directive/ngRepeat.js

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
'use strict';
1+
'use strict';
22

33
/* exported ngRepeatDirective */
44

@@ -531,6 +531,8 @@ var ngRepeatDirective = ['$parse', '$animate', '$compile', function($parse, $ani
531531

532532
//watch props
533533
$scope.$watchCollection(rhs, function ngRepeatAction(collection) {
534+
// console.log('ngRepeat action')
535+
534536
var index, length,
535537
previousNode = $element[0], // node that cloned nodes should be inserted after
536538
// initialized to the comment node anchor

0 commit comments

Comments
 (0)