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

Commit 985dc51

Browse files
NarretzNarretz
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 985dc51

File tree

10 files changed

+615
-59
lines changed

10 files changed

+615
-59
lines changed

‎src/ng/compile.js

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2241,6 +2241,7 @@ function $CompileProvider($provide, $$sanitizeUriProvider) {
22412241
// fire observers
22422242
var $$observers = this.$$observers;
22432243
if ($$observers) {
2244+
// console.log('call observers', observer);
22442245
forEach($$observers[observer], function(fn) {
22452246
try {
22462247
fn(value);
@@ -2272,6 +2273,7 @@ function $CompileProvider($provide, $$sanitizeUriProvider) {
22722273
* @returns {function()} Returns a deregistration function for this observer.
22732274
*/
22742275
$observe: function(key, fn) {
2276+
// console.log('$observe', key);
22752277
var attrs = this,
22762278
$$observers = (attrs.$$observers || (attrs.$$observers = createMap())),
22772279
listeners = ($$observers[key] || ($$observers[key] = []));
@@ -2280,6 +2282,7 @@ function $CompileProvider($provide, $$sanitizeUriProvider) {
22802282
$rootScope.$evalAsync(function() {
22812283
if (!listeners.$$inter && attrs.hasOwnProperty(key) && !isUndefined(attrs[key])) {
22822284
// no one registered attribute interpolation function, so lets call it manually
2285+
// console.log('call initial fn for', key);
22832286
fn(attrs[key]);
22842287
}
22852288
});
@@ -3904,6 +3907,7 @@ function $CompileProvider($provide, $$sanitizeUriProvider) {
39043907
if (name === 'class' && newValue !== oldValue) {
39053908
attr.$updateClass(newValue, oldValue);
39063909
} else {
3910+
// console.log('call set for', name)
39073911
attr.$set(name, newValue);
39083912
}
39093913
});

‎src/ng/directive/input.js

Lines changed: 64 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

@@ -1827,7 +1856,13 @@ function rangeInputType(scope, element, attr, ctrl, $sniffer, $browser) {
18271856
// attribute value when the input is first rendered, so that the browser can adjust the
18281857
// input value based on the min/max value
18291858
element.attr(htmlAttrName, attr[htmlAttrName]);
1830-
attr.$observe(htmlAttrName, changeFn);
1859+
var oldVal = attr.htmlAttrName;
1860+
attr.$observe(htmlAttrName, function wrappedObserver(val) {
1861+
if (val !== oldVal) {
1862+
changeFn(val);
1863+
}
1864+
oldVal = val;
1865+
});
18311866
}
18321867

18331868
function minChange(val) {
@@ -1881,11 +1916,11 @@ function rangeInputType(scope, element, attr, ctrl, $sniffer, $browser) {
18811916
}
18821917

18831918
// 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 {
1919+
if (!supportsRange) {
18871920
// TODO(matsko): implement validateLater to reduce number of validations
18881921
ctrl.$validate();
1922+
} else if (ctrl.$viewValue !== element.val()) {
1923+
ctrl.$setViewValue(element.val());
18891924
}
18901925
}
18911926
}

‎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)