From c1bef694f81142712817fca4336870f4195534ba Mon Sep 17 00:00:00 2001 From: Martin Staffa Date: Mon, 29 Oct 2018 17:53:32 +0100 Subject: [PATCH 1/7] 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 --- src/ng/directive/input.js | 99 ++++++--- src/ng/directive/ngModel.js | 4 + src/ng/directive/ngRepeat.js | 4 +- src/ng/directive/validators.js | 105 ++++++--- src/ng/rootScope.js | 3 + test/helpers/testabilityPatch.js | 21 ++ test/ng/directive/inputSpec.js | 325 +++++++++++++++++++++++++++- test/ng/directive/ngModelSpec.js | 2 +- test/ng/directive/validatorsSpec.js | 115 +++++++++- 9 files changed, 618 insertions(+), 60 deletions(-) diff --git a/src/ng/directive/input.js b/src/ng/directive/input.js index 2f75defe1944..2aafb7fe1080 100644 --- a/src/ng/directive/input.js +++ b/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,35 @@ 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); + ctrl.$validate(); + } + minVal = val; }); } 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); + ctrl.$validate(); + } + + maxVal = val; }); } @@ -1709,50 +1720,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); + // TODO(matsko): implement validateLater to reduce number of validations + ctrl.$validate(); + } + minVal = val; }); } 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); + // TODO(matsko): implement validateLater to reduce number of validations + ctrl.$validate(); + } + maxVal = val; }); } 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 (stepVal !== val) { + parsedStepVal = parseNumberAttrVal(val); + ctrl.$validate(); + } + + stepVal = val; }); + } } @@ -1782,6 +1811,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 +1825,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 +1839,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 +1862,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) { + changeFn(val); + } + oldVal = val; + }); } function minChange(val) { @@ -1881,11 +1922,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()); } } } diff --git a/src/ng/directive/ngModel.js b/src/ng/directive/ngModel.js index 5d73c33ceb28..dc8962a00af0 100644 --- a/src/ng/directive/ngModel.js +++ b/src/ng/directive/ngModel.js @@ -562,8 +562,10 @@ 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)) { + // console.log('dont validate yet'); return; } @@ -1050,6 +1052,7 @@ NgModelController.prototype = { * This method is called internally when the bound scope value changes. */ $$setModelValue: function(modelValue) { + // console.log('$$setModelValue', modelValue); this.$modelValue = this.$$rawModelValue = modelValue; this.$$parserValid = undefined; this.$processModelValue(); @@ -1081,6 +1084,7 @@ function setupModelWatcher(ctrl) { // ng-change executes in apply phase // 4. view should be changed back to 'a' ctrl.$$scope.$watch(function ngModelWatch(scope) { + // console.log('ngModelWatch exp'); var modelValue = ctrl.$$ngModelGet(scope); // if scope model value and ngModel value are out of sync diff --git a/src/ng/directive/ngRepeat.js b/src/ng/directive/ngRepeat.js index dacd67688006..0f5cbd46bf30 100644 --- a/src/ng/directive/ngRepeat.js +++ b/src/ng/directive/ngRepeat.js @@ -1,4 +1,4 @@ -'use strict'; + 'use strict'; /* exported ngRepeatDirective */ @@ -531,6 +531,8 @@ var ngRepeatDirective = ['$parse', '$animate', '$compile', function($parse, $ani //watch props $scope.$watchCollection(rhs, function ngRepeatAction(collection) { + // console.log('ngRepeat action') + var index, length, previousNode = $element[0], // node that cloned nodes should be inserted after // initialized to the comment node anchor diff --git a/src/ng/directive/validators.js b/src/ng/directive/validators.js index 1a07a5501015..eb166fc03914 100644 --- a/src/ng/directive/validators.js +++ b/src/ng/directive/validators.js @@ -62,20 +62,26 @@ * * */ -var requiredDirective = function() { +var requiredDirective = 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) { + ctrl.$validate(); + } + + oldVal = val; }); } }; @@ -162,27 +168,43 @@ var requiredDirective = function() { * * */ -var patternDirective = function() { +var patternDirective = function($parse) { return { restrict: 'A', require: '?ngModel', link: function(scope, elm, attr, ctrl) { if (!ctrl) return; - var regexp, patternExp = attr.ngPattern || attr.pattern; - attr.$observe('pattern', function(regex) { - if (isString(regex) && regex.length > 0) { - regex = new RegExp('^' + regex + '$'); - } + var attrVal = attr.pattern; + var patternExp; + + if (attr.ngPattern) { + patternExp = attr.ngPattern; - if (regex && !regex.test) { - throw minErr('ngPattern')('noregexp', - 'Expected {0} to be a RegExp but was {1}. Element: {2}', patternExp, - regex, startingTag(elm)); + // ngPattern might be a scope expressions, 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 unnessecary validations + try { + attrVal = $parse(attr.ngPattern)(scope); + } catch (e) { + if (/^\[\$parse:lexerr\]/.test(e.message)) { + attrVal = attr.ngPattern; + } else { + throw e; + } } + } - regexp = regex || undefined; - ctrl.$validate(); + var regexp = parsePatternAttr(attrVal, patternExp, elm); + + attr.$observe('pattern', function(newVal) { + var oldRegexp = regexp; + + regexp = parsePatternAttr(newVal, patternExp, elm); + + if ((oldRegexp && oldRegexp.toString()) !== (regexp && regexp.toString())) { + ctrl.$validate(); + } }); ctrl.$validators.pattern = function(modelValue, viewValue) { @@ -264,21 +286,25 @@ var patternDirective = function() { * * */ -var maxlengthDirective = function() { +var maxlengthDirective = 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); + ctrl.$validate(); + } + maxlength = value; }); ctrl.$validators.maxlength = function(modelValue, viewValue) { - return (maxlength < 0) || ctrl.$isEmpty(viewValue) || (viewValue.length <= maxlength); + return (maxlengthParsed < 0) || ctrl.$isEmpty(viewValue) || (viewValue.length <= maxlengthParsed); }; } }; @@ -353,21 +379,48 @@ var maxlengthDirective = function() { * * */ -var minlengthDirective = function() { +var minlengthDirective = 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 = toInt(minlength) || 0; + attr.$observe('minlength', function(value) { - minlength = toInt(value) || 0; - ctrl.$validate(); + if (minlength !== value) { + minlengthParsed = toInt(value) || 0; + ctrl.$validate(); + } + minlength = value; + }); 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 (isString(regex) && regex.length > 0) { + regex = new RegExp('^' + regex + '$'); + } + + if (regex && !regex.test) { + throw minErr('ngPattern')('noregexp', + 'Expected {0} to be a RegExp but was {1}. Element: {2}', patternExp, + regex, startingTag(elm)); + } + + return regex || undefined; +} + +function parseLength(val) { + var intVal = toInt(val); + return isNumberNaN(intVal) ? -1 : intVal; +} diff --git a/src/ng/rootScope.js b/src/ng/rootScope.js index 2a1d85ccbb2b..dba0ae12264e 100644 --- a/src/ng/rootScope.js +++ b/src/ng/rootScope.js @@ -395,6 +395,7 @@ function $RootScopeProvider() { * @returns {function()} Returns a deregistration function for this listener. */ $watch: function(watchExp, listener, objectEquality, prettyPrintExpression) { + // console.log('setup watchExpression', watchExp); var get = $parse(watchExp); var fn = isFunction(listener) ? listener : noop; @@ -769,6 +770,7 @@ function $RootScopeProvider() { * */ $digest: function() { + // console.log('digest'); var watch, value, last, fn, get, watchers, dirty, ttl = TTL, @@ -828,6 +830,7 @@ function $RootScopeProvider() { lastDirtyWatch = watch; watch.last = watch.eq ? copy(value, null) : value; fn = watch.fn; + // console.log('call watch fn', watch.exp, fn); fn(value, ((last === initWatchVal) ? value : last), current); if (ttl < 5) { logIdx = 4 - ttl; diff --git a/test/helpers/testabilityPatch.js b/test/helpers/testabilityPatch.js index 64544e586444..37d4ef694bad 100644 --- a/test/helpers/testabilityPatch.js +++ b/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; diff --git a/test/ng/directive/inputSpec.js b/test/ng/directive/inputSpec.js index 79b44c910170..a001c857277e 100644 --- a/test/ng/directive/inputSpec.js +++ b/test/ng/directive/inputSpec.js @@ -839,6 +839,24 @@ describe('input', function() { expect($rootScope.form.alias.$error.min).toBeFalsy(); }); + + it('should only validate once after compilation when inside ngRepeat', function() { + inputElm = helper.compileInput('
' + + '' + + '
'); + $rootScope.$digest(); + + expect(helper.validationCounter.min).toBe(1); + + helper.validationCounter = {}; + + inputElm = helper.compileInput('
' + + '' + + '
'); + $rootScope.$digest(); + + expect(helper.validationCounter.min).toBe(1); + }); }); describe('max', function() { @@ -898,6 +916,24 @@ describe('input', function() { expect($rootScope.form.alias.$error.max).toBeFalsy(); expect($rootScope.form.alias.$valid).toBeTruthy(); }); + + it('should only validate once after compilation when inside ngRepeat', function() { + inputElm = helper.compileInput('
' + + '' + + '
'); + $rootScope.$digest(); + + expect(helper.validationCounter.max).toBe(1); + + helper.validationCounter = {}; + + inputElm = helper.compileInput('
' + + '' + + '
'); + $rootScope.$digest(); + + expect(helper.validationCounter.max).toBe(1); + }); }); }); @@ -1114,6 +1150,24 @@ describe('input', function() { expect($rootScope.form.alias.$error.min).toBeFalsy(); }); + + it('should only validate once after compilation when inside ngRepeat', function() { + inputElm = helper.compileInput('
' + + '' + + '
'); + $rootScope.$digest(); + + expect(helper.validationCounter.min).toBe(1); + + helper.validationCounter = {}; + + inputElm = helper.compileInput('
' + + '' + + '
'); + $rootScope.$digest(); + + expect(helper.validationCounter.min).toBe(1); + }); }); describe('max', function() { @@ -1176,6 +1230,24 @@ describe('input', function() { expect($rootScope.form.alias.$error.max).toBeFalsy(); expect($rootScope.form.alias.$valid).toBeTruthy(); }); + + it('should only validate once after compilation when inside ngRepeat', function() { + inputElm = helper.compileInput('
' + + '' + + '
'); + $rootScope.$digest(); + + expect(helper.validationCounter.max).toBe(1); + + helper.validationCounter = {}; + + inputElm = helper.compileInput('
' + + '' + + '
'); + $rootScope.$digest(); + + expect(helper.validationCounter.max).toBe(1); + }); }); }); @@ -1506,6 +1578,25 @@ describe('input', function() { expect($rootScope.form.alias.$error.min).toBeFalsy(); }); + + it('should only validate once after compilation when inside ngRepeat', function() { + inputElm = helper.compileInput('
' + + '' + + '
'); + $rootScope.$digest(); + + expect(helper.validationCounter.min).toBe(1); + + helper.validationCounter = {}; + + inputElm = helper.compileInput('
' + + '' + + '
'); + $rootScope.$digest(); + + expect(helper.validationCounter.min).toBe(1); + }); + }); describe('max', function() { @@ -1565,6 +1656,24 @@ describe('input', function() { expect($rootScope.form.alias.$error.max).toBeFalsy(); expect($rootScope.form.alias.$valid).toBeTruthy(); }); + + it('should only validate once after compilation when inside ngRepeat', function() { + inputElm = helper.compileInput('
' + + '' + + '
'); + $rootScope.$digest(); + + expect(helper.validationCounter.max).toBe(1); + + helper.validationCounter = {}; + + inputElm = helper.compileInput('
' + + '' + + '
'); + $rootScope.$digest(); + + expect(helper.validationCounter.max).toBe(1); + }); }); @@ -1972,6 +2081,24 @@ describe('input', function() { expect($rootScope.form.alias.$error.min).toBeFalsy(); }); + + it('should only validate once after compilation when inside ngRepeat', function() { + inputElm = helper.compileInput('
' + + '' + + '
'); + $rootScope.$digest(); + + expect(helper.validationCounter.min).toBe(1); + + helper.validationCounter = {}; + + inputElm = helper.compileInput('
' + + '' + + '
'); + $rootScope.$digest(); + + expect(helper.validationCounter.min).toBe(1); + }); }); describe('max', function() { @@ -2019,6 +2146,24 @@ describe('input', function() { expect($rootScope.form.alias.$error.max).toBeFalsy(); expect($rootScope.form.alias.$valid).toBeTruthy(); }); + + it('should only validate once after compilation when inside ngRepeat', function() { + inputElm = helper.compileInput('
' + + '' + + '
'); + $rootScope.$digest(); + + expect(helper.validationCounter.max).toBe(1); + + helper.validationCounter = {}; + + inputElm = helper.compileInput('
' + + '' + + '
'); + $rootScope.$digest(); + + expect(helper.validationCounter.max).toBe(1); + }); }); @@ -2361,6 +2506,28 @@ describe('input', function() { expect($rootScope.form.alias.$error.min).toBeFalsy(); }); + + it('should only validate once after compilation when inside ngRepeat', function() { + $rootScope.minVal = '2000-01-01'; + $rootScope.value = new Date(2010, 1, 1, 0, 0, 0); + + var inputElm = helper.compileInput('
' + + '' + + '
'); + $rootScope.$digest(); + + expect(helper.validationCounter.min).toBe(1); + + helper.validationCounter = {}; + + inputElm = helper.compileInput('
' + + '' + + '
'); + $rootScope.$digest(); + + expect(helper.validationCounter.min).toBe(1); + }); + }); describe('max', function() { @@ -2428,6 +2595,27 @@ describe('input', function() { expect($rootScope.form.alias.$error.max).toBeFalsy(); expect($rootScope.form.alias.$valid).toBeTruthy(); }); + + it('should only validate once after compilation when inside ngRepeat', function() { + $rootScope.maxVal = '2000-01-01'; + $rootScope.value = new Date(2020, 1, 1, 0, 0, 0); + + var inputElm = helper.compileInput('
' + + '' + + '
'); + $rootScope.$digest(); + + expect(helper.validationCounter.max).toBe(1); + + helper.validationCounter = {}; + + inputElm = helper.compileInput('
' + + '' + + '
'); + $rootScope.$digest(); + + expect(helper.validationCounter.max).toBe(1); + }); }); @@ -3063,6 +3251,18 @@ describe('input', function() { $rootScope.$digest(); expect(inputElm).toBeValid(); }); + + it('should only validate once after compilation when inside ngRepeat', function() { + $rootScope.value = 5; + $rootScope.minVal = 3; + var inputElm = helper.compileInput('
' + + '' + + '
'); + $rootScope.$digest(); + + expect(helper.validationCounter.min).toBe(1); + }); + }); describe('ngMin', function() { @@ -3131,6 +3331,17 @@ describe('input', function() { $rootScope.$digest(); expect(inputElm).toBeValid(); }); + + it('should only validate once after compilation when inside ngRepeat', function() { + $rootScope.value = 5; + $rootScope.minVal = 3; + var inputElm = helper.compileInput('
' + + '' + + '
'); + $rootScope.$digest(); + + expect(helper.validationCounter.min).toBe(1); + }); }); @@ -3200,6 +3411,18 @@ describe('input', function() { $rootScope.$digest(); expect(inputElm).toBeValid(); }); + + it('should only validate once after compilation when inside ngRepeat', function() { + $rootScope.value = 5; + $rootScope.maxVal = 3; + var inputElm = helper.compileInput('
' + + '' + + '
'); + $rootScope.$digest(); + + expect(helper.validationCounter.max).toBe(1); + }); + }); describe('ngMax', function() { @@ -3268,6 +3491,17 @@ describe('input', function() { $rootScope.$digest(); expect(inputElm).toBeValid(); }); + + it('should only validate once after compilation when inside ngRepeat', function() { + $rootScope.value = 5; + $rootScope.maxVal = 3; + var inputElm = helper.compileInput('
' + + '' + + '
'); + $rootScope.$digest(); + + expect(helper.validationCounter.max).toBe(1); + }); }); @@ -3364,7 +3598,7 @@ describe('input', function() { expect(inputElm.val()).toBe('10'); expect(inputElm).toBeInvalid(); expect(ngModel.$error.step).toBe(true); - expect($rootScope.value).toBeUndefined(); + expect($rootScope.value).toBe(10); // an initially invalid value should not be changed helper.changeInputValueTo('15'); expect(inputElm).toBeValid(); @@ -3444,6 +3678,17 @@ describe('input', function() { expect($rootScope.value).toBe(1.16); } ); + + it('should validate only once after compilation inside ngRepeat', function() { + $rootScope.step = 10; + $rootScope.value = 20; + var inputElm = helper.compileInput('
' + + '' + + '
'); + + expect(helper.validationCounter.step).toBe(1); + }); + }); }); @@ -3485,6 +3730,16 @@ describe('input', function() { expect(inputElm).toBeValid(); }); + + it('should only validate once after compilation when inside ngRepeat', function() { + $rootScope.value = 'text'; + var inputElm = helper.compileInput('
' + + '' + + '
'); + $rootScope.$digest(); + + expect(helper.validationCounter.required).toBe(1); + }); }); describe('ngRequired', function() { @@ -3534,6 +3789,17 @@ describe('input', function() { expect($rootScope.value).toBeUndefined(); expect($rootScope.form.numberInput.$error.required).toBeFalsy(); }); + + it('should only validate once after compilation when inside ngRepeat', function() { + $rootScope.value = 'text'; + $rootScope.isRequired = true; + var inputElm = helper.compileInput('
' + + '' + + '
'); + $rootScope.$digest(); + + expect(helper.validationCounter.required).toBe(1); + }); }); describe('when the ngRequired expression initially evaluates to false', function() { @@ -3848,6 +4114,17 @@ describe('input', function() { expect(inputElm.val()).toBe('20'); }); + it('should only validate once after compilation when inside ngRepeat', function() { + $rootScope.minVal = 5; + $rootScope.value = 10; + helper.compileInput('
' + + '' + + '
'); + $rootScope.$digest(); + + expect(helper.validationCounter.min).toBe(1); + }); + } else { // input[type=range] will become type=text in browsers that don't support it @@ -3926,6 +4203,16 @@ describe('input', function() { expect(inputElm.val()).toBe('15'); }); + it('should only validate once after compilation when inside ngRepeat', function() { + $rootScope.minVal = 5; + $rootScope.value = 10; + helper.compileInput('
' + + '' + + '
'); + $rootScope.$digest(); + + expect(helper.validationCounter.min).toBe(1); + }); } }); @@ -4006,6 +4293,17 @@ describe('input', function() { expect(inputElm.val()).toBe('0'); }); + it('should only validate once after compilation when inside ngRepeat and the value is valid', function() { + $rootScope.maxVal = 5; + $rootScope.value = 5; + helper.compileInput('
' + + '' + + '
'); + $rootScope.$digest(); + + expect(helper.validationCounter.max).toBe(1); + }); + } else { it('should validate if "range" is not implemented', function() { var inputElm = helper.compileInput(''); @@ -4081,6 +4379,17 @@ describe('input', function() { expect(scope.value).toBe(5); expect(inputElm.val()).toBe('5'); }); + + it('should only validate once after compilation when inside ngRepeat', function() { + $rootScope.maxVal = 5; + $rootScope.value = 10; + helper.compileInput('
' + + '' + + '
'); + $rootScope.$digest(); + + expect(helper.validationCounter.max).toBe(1); + }); } }); @@ -4183,6 +4492,18 @@ describe('input', function() { expect(scope.value).toBe(10); expect(scope.form.alias.$error.step).toBeFalsy(); }); + + it('should only validate once after compilation when inside ngRepeat', function() { + $rootScope.stepVal = 5; + $rootScope.value = 10; + helper.compileInput('
' + + '' + + '
'); + $rootScope.$digest(); + + expect(helper.validationCounter.step).toBe(1); + }); + } else { it('should validate if "range" is not implemented', function() { @@ -4269,7 +4590,7 @@ describe('input', function() { expect(inputElm.val()).toBe('10'); expect(inputElm).toBeInvalid(); expect(ngModel.$error.step).toBe(true); - expect($rootScope.value).toBeUndefined(); + expect($rootScope.value).toBe(10); helper.changeInputValueTo('15'); expect(inputElm).toBeValid(); diff --git a/test/ng/directive/ngModelSpec.js b/test/ng/directive/ngModelSpec.js index 825973bdcf2f..ede31bff426f 100644 --- a/test/ng/directive/ngModelSpec.js +++ b/test/ng/directive/ngModelSpec.js @@ -863,7 +863,6 @@ describe('ngModel', function() { }); }); - describe('view -> model update', function() { it('should always perform validations using the parsed model value', function() { @@ -1246,6 +1245,7 @@ describe('ngModel', function() { dealoc(element); })); + it('should re-evaluate the form validity state once the asynchronous promise has been delivered', inject(function($compile, $rootScope, $q) { diff --git a/test/ng/directive/validatorsSpec.js b/test/ng/directive/validatorsSpec.js index 9b152da7f386..1a2b9e5b85d1 100644 --- a/test/ng/directive/validatorsSpec.js +++ b/test/ng/directive/validatorsSpec.js @@ -230,6 +230,31 @@ 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( + '
' + + '' + + '
'); + + $rootScope.$digest(); + + expect(helper.validationCounter.pattern).toBe(1); + + helper.validationCounter = {}; + + helper.compileInput( + '
' + + '' + + '
'); + + $rootScope.$digest(); + + expect(helper.validationCounter.pattern).toBe(1); + }); }); @@ -312,9 +337,33 @@ 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( + '
' + + '' + + '
'); + + $rootScope.$digest(); + + expect(helper.validationCounter.minlength).toBe(1); + + helper.validationCounter = {}; + + element = helper.compileInput( + '
' + + '' + + '
'); + + $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 +549,32 @@ describe('validators', function() { expect(ctrl.$error.maxlength).toBe(true); expect(ctrlNg.$error.maxlength).toBe(true); })); + + + it('does not validate twice after compilation when inside ngRepeat', function() { + $rootScope.inputs = ['Jamie']; + $rootScope.maxlength = 5; + + var element = helper.compileInput( + '
' + + '' + + '
'); + + $rootScope.$digest(); + + expect(helper.validationCounter.maxlength).toBe(1); + + helper.validationCounter = {}; + + element = helper.compileInput( + '
' + + '' + + '
'); + + $rootScope.$digest(); + + expect(helper.validationCounter.maxlength).toBe(1); + }); }); @@ -626,5 +701,43 @@ 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( + '
' + + '' + + '
'); + + $rootScope.$digest(); + + expect(helper.validationCounter.required).toBe(1); + + helper.validationCounter = {}; + }); + + + it('should validate only once after compilation when inside ngRepeat and ngRequired is true', function() { + $rootScope.isRequired = true; + + helper.compileInput( + '
' + + '' + + '
'); + + 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( + '
' + + '' + + '
'); + + expect(helper.validationCounter.required).toBe(1); + }); }); }); From 0d826b879926a4d203962723a50b4f679db25a21 Mon Sep 17 00:00:00 2001 From: Martin Staffa Date: Mon, 19 Nov 2018 17:18:44 +0100 Subject: [PATCH 2/7] fixup! perf(input): prevent multiple validations on compilation --- src/ng/directive/input.js | 13 +++--- src/ng/directive/validators.js | 61 +++++++++++++++++------------ src/ng/rootScope.js | 3 -- test/ng/directive/ngModelSpec.js | 1 - test/ng/directive/validatorsSpec.js | 11 +++--- 5 files changed, 46 insertions(+), 43 deletions(-) diff --git a/src/ng/directive/input.js b/src/ng/directive/input.js index 2aafb7fe1080..82b64f5ff80b 100644 --- a/src/ng/directive/input.js +++ b/src/ng/directive/input.js @@ -1549,9 +1549,9 @@ function createDateInputType(type, regexp, parseDate, format) { attr.$observe('min', function(val) { if (val !== minVal) { parsedMinVal = parseObservedDateValue(val); + minVal = val; ctrl.$validate(); } - minVal = val; }); } @@ -1565,10 +1565,9 @@ function createDateInputType(type, regexp, parseDate, format) { attr.$observe('max', function(val) { if (val !== maxVal) { parsedMaxVal = parseObservedDateValue(val); + maxVal = val; ctrl.$validate(); } - - maxVal = val; }); } @@ -1738,10 +1737,10 @@ function numberInputType(scope, element, attr, ctrl, $sniffer, $browser, $filter attr.$observe('min', function(val) { if (val !== minVal) { parsedMinVal = parseNumberAttrVal(val); + minVal = val; // TODO(matsko): implement validateLater to reduce number of validations ctrl.$validate(); } - minVal = val; }); } @@ -1756,10 +1755,10 @@ function numberInputType(scope, element, attr, ctrl, $sniffer, $browser, $filter attr.$observe('max', function(val) { if (val !== maxVal) { parsedMaxVal = parseNumberAttrVal(val); + maxVal = val; // TODO(matsko): implement validateLater to reduce number of validations ctrl.$validate(); } - maxVal = val; }); } @@ -1776,10 +1775,10 @@ function numberInputType(scope, element, attr, ctrl, $sniffer, $browser, $filter // TODO(matsko): implement validateLater to reduce number of validations if (stepVal !== val) { parsedStepVal = parseNumberAttrVal(val); + stepVal = val; ctrl.$validate(); } - stepVal = val; }); } @@ -1865,9 +1864,9 @@ function rangeInputType(scope, element, attr, ctrl, $sniffer, $browser) { var oldVal = attr[htmlAttrName]; attr.$observe(htmlAttrName, function wrappedObserver(val) { if (val !== oldVal) { + oldVal = val; changeFn(val); } - oldVal = val; }); } diff --git a/src/ng/directive/validators.js b/src/ng/directive/validators.js index eb166fc03914..400a994e84d8 100644 --- a/src/ng/directive/validators.js +++ b/src/ng/directive/validators.js @@ -78,10 +78,9 @@ var requiredDirective = function($parse) { attr.$observe('required', function(val) { if (oldVal !== val) { + oldVal = val; ctrl.$validate(); } - - oldVal = val; }); } }; @@ -172,46 +171,55 @@ var patternDirective = function($parse) { return { restrict: 'A', require: '?ngModel', - link: function(scope, elm, attr, ctrl) { - if (!ctrl) return; - - var attrVal = attr.pattern; + compile: function(tElm, tAttr) { var patternExp; + var parseFn; - if (attr.ngPattern) { - patternExp = attr.ngPattern; + if (tAttr.ngPattern) { + patternExp = tAttr.ngPattern; // ngPattern might be a scope expressions, 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 unnessecary validations + // in the observer to avoid unnecessary validations try { - attrVal = $parse(attr.ngPattern)(scope); + parseFn = $parse(tAttr.ngPattern); } catch (e) { if (/^\[\$parse:lexerr\]/.test(e.message)) { - attrVal = attr.ngPattern; + parseFn = function() { return tAttr.ngPattern; }; } else { throw e; } } } - var regexp = parsePatternAttr(attrVal, patternExp, elm); + return function(scope, elm, attr, ctrl) { + if (!ctrl) return; - attr.$observe('pattern', function(newVal) { - var oldRegexp = regexp; + var attrVal = attr.pattern; - regexp = parsePatternAttr(newVal, patternExp, elm); - - if ((oldRegexp && oldRegexp.toString()) !== (regexp && regexp.toString())) { - ctrl.$validate(); + if (attr.ngPattern) { + attrVal = parseFn(scope); } - }); - 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); + var regexp = parsePatternAttr(attrVal, patternExp, elm); + + attr.$observe('pattern', function(newVal) { + var oldRegexp = regexp; + + 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); + }; }; } + }; }; @@ -299,9 +307,9 @@ var maxlengthDirective = function($parse) { attr.$observe('maxlength', function(value) { if (maxlength !== value) { maxlengthParsed = parseLength(value); + maxlength = value; ctrl.$validate(); } - maxlength = value; }); ctrl.$validators.maxlength = function(modelValue, viewValue) { return (maxlengthParsed < 0) || ctrl.$isEmpty(viewValue) || (viewValue.length <= maxlengthParsed); @@ -392,9 +400,9 @@ var minlengthDirective = function($parse) { attr.$observe('minlength', function(value) { if (minlength !== value) { minlengthParsed = toInt(value) || 0; + minlength = value; ctrl.$validate(); } - minlength = value; }); ctrl.$validators.minlength = function(modelValue, viewValue) { @@ -406,18 +414,19 @@ var minlengthDirective = function($parse) { function parsePatternAttr(regex, patternExp, elm) { + if (!regex) return undefined; if (isString(regex) && regex.length > 0) { regex = new RegExp('^' + regex + '$'); } - if (regex && !regex.test) { + if (!(regex instanceof RegExp)) { throw minErr('ngPattern')('noregexp', 'Expected {0} to be a RegExp but was {1}. Element: {2}', patternExp, regex, startingTag(elm)); } - return regex || undefined; + return regex; } function parseLength(val) { diff --git a/src/ng/rootScope.js b/src/ng/rootScope.js index dba0ae12264e..2a1d85ccbb2b 100644 --- a/src/ng/rootScope.js +++ b/src/ng/rootScope.js @@ -395,7 +395,6 @@ function $RootScopeProvider() { * @returns {function()} Returns a deregistration function for this listener. */ $watch: function(watchExp, listener, objectEquality, prettyPrintExpression) { - // console.log('setup watchExpression', watchExp); var get = $parse(watchExp); var fn = isFunction(listener) ? listener : noop; @@ -770,7 +769,6 @@ function $RootScopeProvider() { * */ $digest: function() { - // console.log('digest'); var watch, value, last, fn, get, watchers, dirty, ttl = TTL, @@ -830,7 +828,6 @@ function $RootScopeProvider() { lastDirtyWatch = watch; watch.last = watch.eq ? copy(value, null) : value; fn = watch.fn; - // console.log('call watch fn', watch.exp, fn); fn(value, ((last === initWatchVal) ? value : last), current); if (ttl < 5) { logIdx = 4 - ttl; diff --git a/test/ng/directive/ngModelSpec.js b/test/ng/directive/ngModelSpec.js index ede31bff426f..f8eda1fe76ff 100644 --- a/test/ng/directive/ngModelSpec.js +++ b/test/ng/directive/ngModelSpec.js @@ -1245,7 +1245,6 @@ describe('ngModel', function() { dealoc(element); })); - it('should re-evaluate the form validity state once the asynchronous promise has been delivered', inject(function($compile, $rootScope, $q) { diff --git a/test/ng/directive/validatorsSpec.js b/test/ng/directive/validatorsSpec.js index 1a2b9e5b85d1..6bc52013ab46 100644 --- a/test/ng/directive/validatorsSpec.js +++ b/test/ng/directive/validatorsSpec.js @@ -551,13 +551,12 @@ describe('validators', function() { })); - it('does not validate twice after compilation when inside ngRepeat', function() { - $rootScope.inputs = ['Jamie']; + it('should only validate once after compilation when inside ngRepeat', function() { $rootScope.maxlength = 5; var element = helper.compileInput( - '
' + - '' + + '
' + + '' + '
'); $rootScope.$digest(); @@ -567,8 +566,8 @@ describe('validators', function() { helper.validationCounter = {}; element = helper.compileInput( - '
' + - '' + + '
' + + '' + '
'); $rootScope.$digest(); From f2e1bd6c2eddd73a5e5d53ab938f63397bb98b98 Mon Sep 17 00:00:00 2001 From: Martin Staffa Date: Tue, 20 Nov 2018 11:03:34 +0100 Subject: [PATCH 3/7] fixup! perf(input): prevent multiple validations on compilation --- src/ng/directive/ngModel.js | 3 --- 1 file changed, 3 deletions(-) diff --git a/src/ng/directive/ngModel.js b/src/ng/directive/ngModel.js index dc8962a00af0..951d909342bc 100644 --- a/src/ng/directive/ngModel.js +++ b/src/ng/directive/ngModel.js @@ -565,7 +565,6 @@ NgModelController.prototype = { // ignore $validate before model is initialized if (isNumberNaN(this.$modelValue)) { - // console.log('dont validate yet'); return; } @@ -1052,7 +1051,6 @@ NgModelController.prototype = { * This method is called internally when the bound scope value changes. */ $$setModelValue: function(modelValue) { - // console.log('$$setModelValue', modelValue); this.$modelValue = this.$$rawModelValue = modelValue; this.$$parserValid = undefined; this.$processModelValue(); @@ -1084,7 +1082,6 @@ function setupModelWatcher(ctrl) { // ng-change executes in apply phase // 4. view should be changed back to 'a' ctrl.$$scope.$watch(function ngModelWatch(scope) { - // console.log('ngModelWatch exp'); var modelValue = ctrl.$$ngModelGet(scope); // if scope model value and ngModel value are out of sync From b4d7b61549d8cc557b4dd8aaf443c2a61bf96d4d Mon Sep 17 00:00:00 2001 From: Martin Staffa Date: Wed, 21 Nov 2018 16:48:55 +0100 Subject: [PATCH 4/7] fixup! perf(input): prevent multiple validations on compilation --- src/ng/directive/input.js | 2 +- src/ng/directive/ngRepeat.js | 4 +--- src/ng/directive/validators.js | 8 ++++---- 3 files changed, 6 insertions(+), 8 deletions(-) diff --git a/src/ng/directive/input.js b/src/ng/directive/input.js index 82b64f5ff80b..0a9eacd9fd8b 100644 --- a/src/ng/directive/input.js +++ b/src/ng/directive/input.js @@ -1773,7 +1773,7 @@ function numberInputType(scope, element, attr, ctrl, $sniffer, $browser, $filter attr.$observe('step', function(val) { // TODO(matsko): implement validateLater to reduce number of validations - if (stepVal !== val) { + if (val !== stepVal) { parsedStepVal = parseNumberAttrVal(val); stepVal = val; ctrl.$validate(); diff --git a/src/ng/directive/ngRepeat.js b/src/ng/directive/ngRepeat.js index 0f5cbd46bf30..dacd67688006 100644 --- a/src/ng/directive/ngRepeat.js +++ b/src/ng/directive/ngRepeat.js @@ -1,4 +1,4 @@ - 'use strict'; +'use strict'; /* exported ngRepeatDirective */ @@ -531,8 +531,6 @@ var ngRepeatDirective = ['$parse', '$animate', '$compile', function($parse, $ani //watch props $scope.$watchCollection(rhs, function ngRepeatAction(collection) { - // console.log('ngRepeat action') - var index, length, previousNode = $element[0], // node that cloned nodes should be inserted after // initialized to the comment node anchor diff --git a/src/ng/directive/validators.js b/src/ng/directive/validators.js index 400a994e84d8..b297713bdc47 100644 --- a/src/ng/directive/validators.js +++ b/src/ng/directive/validators.js @@ -178,7 +178,7 @@ var patternDirective = function($parse) { if (tAttr.ngPattern) { patternExp = tAttr.ngPattern; - // ngPattern might be a scope expressions, or an inlined regex, which is not parsable. + // 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 try { @@ -395,11 +395,11 @@ var minlengthDirective = function($parse) { if (!ctrl) return; var minlength = attr.minlength || $parse(attr.ngMinlength)(scope); - var minlengthParsed = toInt(minlength) || 0; + var minlengthParsed = parseLength(minlength) || -1; attr.$observe('minlength', function(value) { if (minlength !== value) { - minlengthParsed = toInt(value) || 0; + minlengthParsed = parseLength(value) || -1; minlength = value; ctrl.$validate(); } @@ -416,7 +416,7 @@ var minlengthDirective = function($parse) { function parsePatternAttr(regex, patternExp, elm) { if (!regex) return undefined; - if (isString(regex) && regex.length > 0) { + if (isString(regex)) { regex = new RegExp('^' + regex + '$'); } From b0693615fe42dc58351d47f1fcf27af2e2b513cc Mon Sep 17 00:00:00 2001 From: Martin Staffa Date: Fri, 23 Nov 2018 14:23:37 +0100 Subject: [PATCH 5/7] fixup! perf(input): prevent multiple validations on compilation --- src/ng/directive/validators.js | 14 ++++++-------- test/ng/directive/inputSpec.js | 20 -------------------- test/ng/directive/validatorsSpec.js | 8 -------- 3 files changed, 6 insertions(+), 36 deletions(-) diff --git a/src/ng/directive/validators.js b/src/ng/directive/validators.js index b297713bdc47..76d6b9160bb1 100644 --- a/src/ng/directive/validators.js +++ b/src/ng/directive/validators.js @@ -181,14 +181,10 @@ var patternDirective = function($parse) { // 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 - try { + if (tAttr.ngPattern.charAt(0) === '/' && REGEX_STRING_REGEXP.test(tAttr.ngPattern)) { + parseFn = function() { return tAttr.ngPattern; }; + } else { parseFn = $parse(tAttr.ngPattern); - } catch (e) { - if (/^\[\$parse:lexerr\]/.test(e.message)) { - parseFn = function() { return tAttr.ngPattern; }; - } else { - throw e; - } } } @@ -199,6 +195,8 @@ var patternDirective = function($parse) { if (attr.ngPattern) { attrVal = parseFn(scope); + } else { + patternExp = attr.pattern; } var regexp = parsePatternAttr(attrVal, patternExp, elm); @@ -420,7 +418,7 @@ function parsePatternAttr(regex, patternExp, elm) { regex = new RegExp('^' + regex + '$'); } - if (!(regex instanceof RegExp)) { + if (!regex.test) { throw minErr('ngPattern')('noregexp', 'Expected {0} to be a RegExp but was {1}. Element: {2}', patternExp, regex, startingTag(elm)); diff --git a/test/ng/directive/inputSpec.js b/test/ng/directive/inputSpec.js index a001c857277e..e7159cba9ba7 100644 --- a/test/ng/directive/inputSpec.js +++ b/test/ng/directive/inputSpec.js @@ -848,8 +848,6 @@ describe('input', function() { expect(helper.validationCounter.min).toBe(1); - helper.validationCounter = {}; - inputElm = helper.compileInput('
' + '' + '
'); @@ -925,8 +923,6 @@ describe('input', function() { expect(helper.validationCounter.max).toBe(1); - helper.validationCounter = {}; - inputElm = helper.compileInput('
' + '' + '
'); @@ -1159,8 +1155,6 @@ describe('input', function() { expect(helper.validationCounter.min).toBe(1); - helper.validationCounter = {}; - inputElm = helper.compileInput('
' + '' + '
'); @@ -1239,8 +1233,6 @@ describe('input', function() { expect(helper.validationCounter.max).toBe(1); - helper.validationCounter = {}; - inputElm = helper.compileInput('
' + '' + '
'); @@ -1587,8 +1579,6 @@ describe('input', function() { expect(helper.validationCounter.min).toBe(1); - helper.validationCounter = {}; - inputElm = helper.compileInput('
' + '' + '
'); @@ -1665,8 +1655,6 @@ describe('input', function() { expect(helper.validationCounter.max).toBe(1); - helper.validationCounter = {}; - inputElm = helper.compileInput('
' + '' + '
'); @@ -2090,8 +2078,6 @@ describe('input', function() { expect(helper.validationCounter.min).toBe(1); - helper.validationCounter = {}; - inputElm = helper.compileInput('
' + '' + '
'); @@ -2155,8 +2141,6 @@ describe('input', function() { expect(helper.validationCounter.max).toBe(1); - helper.validationCounter = {}; - inputElm = helper.compileInput('
' + '' + '
'); @@ -2518,8 +2502,6 @@ describe('input', function() { expect(helper.validationCounter.min).toBe(1); - helper.validationCounter = {}; - inputElm = helper.compileInput('
' + '' + '
'); @@ -2607,8 +2589,6 @@ describe('input', function() { expect(helper.validationCounter.max).toBe(1); - helper.validationCounter = {}; - inputElm = helper.compileInput('
' + '' + '
'); diff --git a/test/ng/directive/validatorsSpec.js b/test/ng/directive/validatorsSpec.js index 6bc52013ab46..2d89ce8abfce 100644 --- a/test/ng/directive/validatorsSpec.js +++ b/test/ng/directive/validatorsSpec.js @@ -244,8 +244,6 @@ describe('validators', function() { expect(helper.validationCounter.pattern).toBe(1); - helper.validationCounter = {}; - helper.compileInput( '
' + '' + @@ -351,8 +349,6 @@ describe('validators', function() { expect(helper.validationCounter.minlength).toBe(1); - helper.validationCounter = {}; - element = helper.compileInput( '
' + '' + @@ -563,8 +559,6 @@ describe('validators', function() { expect(helper.validationCounter.maxlength).toBe(1); - helper.validationCounter = {}; - element = helper.compileInput( '
' + '' + @@ -711,8 +705,6 @@ describe('validators', function() { $rootScope.$digest(); expect(helper.validationCounter.required).toBe(1); - - helper.validationCounter = {}; }); From 57b45e1ffc12e12a5b36fd657173a8ffe621ffe7 Mon Sep 17 00:00:00 2001 From: Martin Staffa Date: Thu, 29 Nov 2018 12:46:37 +0100 Subject: [PATCH 6/7] fixup! perf(input): prevent multiple validations on compilation --- src/ng/directive/validators.js | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/src/ng/directive/validators.js b/src/ng/directive/validators.js index 76d6b9160bb1..145a671877ed 100644 --- a/src/ng/directive/validators.js +++ b/src/ng/directive/validators.js @@ -62,7 +62,7 @@ * * */ -var requiredDirective = function($parse) { +var requiredDirective = ['$parse', function($parse) { return { restrict: 'A', require: '?ngModel', @@ -84,7 +84,7 @@ var requiredDirective = function($parse) { }); } }; -}; +}]; /** * @ngdoc directive @@ -292,7 +292,7 @@ var patternDirective = function($parse) { * * */ -var maxlengthDirective = function($parse) { +var maxlengthDirective = ['$parse', function($parse) { return { restrict: 'A', require: '?ngModel', @@ -314,7 +314,7 @@ var maxlengthDirective = function($parse) { }; } }; -}; +}]; /** * @ngdoc directive @@ -385,7 +385,7 @@ var maxlengthDirective = function($parse) { * * */ -var minlengthDirective = function($parse) { +var minlengthDirective = ['$parse', function($parse) { return { restrict: 'A', require: '?ngModel', @@ -408,7 +408,7 @@ var minlengthDirective = function($parse) { }; } }; -}; +}]; function parsePatternAttr(regex, patternExp, elm) { From 4d820204fa9621ed0de4ca3a6d4f3912731e765e Mon Sep 17 00:00:00 2001 From: Martin Staffa Date: Mon, 3 Dec 2018 08:50:49 +0100 Subject: [PATCH 7/7] fixup! perf(input): prevent multiple validations on compilation --- src/ng/directive/validators.js | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/ng/directive/validators.js b/src/ng/directive/validators.js index 145a671877ed..85682fb07ea8 100644 --- a/src/ng/directive/validators.js +++ b/src/ng/directive/validators.js @@ -167,7 +167,7 @@ var requiredDirective = ['$parse', function($parse) { * * */ -var patternDirective = function($parse) { +var patternDirective = ['$parse', function($parse) { return { restrict: 'A', require: '?ngModel', @@ -219,7 +219,7 @@ var patternDirective = function($parse) { } }; -}; +}]; /** * @ngdoc directive