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

Commit 4ca5cf2

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 4ca5cf2

File tree

9 files changed

+611
-59
lines changed

9 files changed

+611
-59
lines changed

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

src/ng/directive/validators.js

Lines changed: 79 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -62,20 +62,26 @@
6262
* </file>
6363
* </example>
6464
*/
65-
var requiredDirective = function() {
65+
var requiredDirective = function($parse) {
6666
return {
6767
restrict: 'A',
6868
require: '?ngModel',
6969
link: function(scope, elm, attr, ctrl) {
7070
if (!ctrl) return;
71+
var oldVal = attr.required || $parse(attr.ngRequired)(scope);
72+
7173
attr.required = true; // force truthy in case we are on non input element
7274

7375
ctrl.$validators.required = function(modelValue, viewValue) {
7476
return !attr.required || !ctrl.$isEmpty(viewValue);
7577
};
7678

77-
attr.$observe('required', function() {
78-
ctrl.$validate();
79+
attr.$observe('required', function(val) {
80+
if (oldVal !== val) {
81+
ctrl.$validate();
82+
}
83+
84+
oldVal = val;
7985
});
8086
}
8187
};
@@ -162,27 +168,43 @@ var requiredDirective = function() {
162168
* </file>
163169
* </example>
164170
*/
165-
var patternDirective = function() {
171+
var patternDirective = function($parse) {
166172
return {
167173
restrict: 'A',
168174
require: '?ngModel',
169175
link: function(scope, elm, attr, ctrl) {
170176
if (!ctrl) return;
171177

172-
var regexp, patternExp = attr.ngPattern || attr.pattern;
173-
attr.$observe('pattern', function(regex) {
174-
if (isString(regex) && regex.length > 0) {
175-
regex = new RegExp('^' + regex + '$');
176-
}
178+
var attrVal = attr.pattern;
179+
var patternExp;
180+
181+
if (attr.ngPattern) {
182+
patternExp = attr.ngPattern;
177183

178-
if (regex && !regex.test) {
179-
throw minErr('ngPattern')('noregexp',
180-
'Expected {0} to be a RegExp but was {1}. Element: {2}', patternExp,
181-
regex, startingTag(elm));
184+
// ngPattern might be a scope expressions, or an inlined regex, which is not parsable.
185+
// We get value of the attribute here, so we can compare the old and the new value
186+
// in the observer to avoid unnessecary validations
187+
try {
188+
attrVal = $parse(attr.ngPattern)(scope);
189+
} catch (e) {
190+
if (/^\[\$parse:lexerr\]/.test(e.message)) {
191+
attrVal = attr.ngPattern;
192+
} else {
193+
throw e;
194+
}
182195
}
196+
}
183197

184-
regexp = regex || undefined;
185-
ctrl.$validate();
198+
var regexp = parsePatternAttr(attrVal, patternExp, elm);
199+
200+
attr.$observe('pattern', function(newVal) {
201+
var oldRegexp = regexp;
202+
203+
regexp = parsePatternAttr(newVal, patternExp, elm);
204+
205+
if ((oldRegexp && oldRegexp.toString()) !== (regexp && regexp.toString())) {
206+
ctrl.$validate();
207+
}
186208
});
187209

188210
ctrl.$validators.pattern = function(modelValue, viewValue) {
@@ -264,21 +286,25 @@ var patternDirective = function() {
264286
* </file>
265287
* </example>
266288
*/
267-
var maxlengthDirective = function() {
289+
var maxlengthDirective = function($parse) {
268290
return {
269291
restrict: 'A',
270292
require: '?ngModel',
271293
link: function(scope, elm, attr, ctrl) {
272294
if (!ctrl) return;
273295

274-
var maxlength = -1;
296+
var maxlength = attr.maxlength || $parse(attr.ngMaxlength)(scope);
297+
var maxlengthParsed = parseLength(maxlength);
298+
275299
attr.$observe('maxlength', function(value) {
276-
var intVal = toInt(value);
277-
maxlength = isNumberNaN(intVal) ? -1 : intVal;
278-
ctrl.$validate();
300+
if (maxlength !== value) {
301+
maxlengthParsed = parseLength(value);
302+
ctrl.$validate();
303+
}
304+
maxlength = value;
279305
});
280306
ctrl.$validators.maxlength = function(modelValue, viewValue) {
281-
return (maxlength < 0) || ctrl.$isEmpty(viewValue) || (viewValue.length <= maxlength);
307+
return (maxlengthParsed < 0) || ctrl.$isEmpty(viewValue) || (viewValue.length <= maxlengthParsed);
282308
};
283309
}
284310
};
@@ -353,21 +379,48 @@ var maxlengthDirective = function() {
353379
* </file>
354380
* </example>
355381
*/
356-
var minlengthDirective = function() {
382+
var minlengthDirective = function($parse) {
357383
return {
358384
restrict: 'A',
359385
require: '?ngModel',
360386
link: function(scope, elm, attr, ctrl) {
361387
if (!ctrl) return;
362388

363-
var minlength = 0;
389+
var minlength = attr.minlength || $parse(attr.ngMinlength)(scope);
390+
var minlengthParsed = toInt(minlength) || 0;
391+
364392
attr.$observe('minlength', function(value) {
365-
minlength = toInt(value) || 0;
366-
ctrl.$validate();
393+
if (minlength !== value) {
394+
minlengthParsed = toInt(value) || 0;
395+
ctrl.$validate();
396+
}
397+
minlength = value;
398+
367399
});
368400
ctrl.$validators.minlength = function(modelValue, viewValue) {
369-
return ctrl.$isEmpty(viewValue) || viewValue.length >= minlength;
401+
return ctrl.$isEmpty(viewValue) || viewValue.length >= minlengthParsed;
370402
};
371403
}
372404
};
373405
};
406+
407+
408+
function parsePatternAttr(regex, patternExp, elm) {
409+
410+
if (isString(regex) && regex.length > 0) {
411+
regex = new RegExp('^' + regex + '$');
412+
}
413+
414+
if (regex && !regex.test) {
415+
throw minErr('ngPattern')('noregexp',
416+
'Expected {0} to be a RegExp but was {1}. Element: {2}', patternExp,
417+
regex, startingTag(elm));
418+
}
419+
420+
return regex || undefined;
421+
}
422+
423+
function parseLength(val) {
424+
var intVal = toInt(val);
425+
return isNumberNaN(intVal) ? -1 : intVal;
426+
}

0 commit comments

Comments
 (0)