From 26ca5746eb40eb1caa66f8dcd51646a40b40291c Mon Sep 17 00:00:00 2001 From: Shahar Talmi Date: Wed, 3 Sep 2014 17:20:32 +0300 Subject: [PATCH 1/2] perf(ngModel): run validators only if rendered value changed --- src/ng/directive/input.js | 3 +-- test/ng/directive/inputSpec.js | 11 +++++++++++ 2 files changed, 12 insertions(+), 2 deletions(-) diff --git a/src/ng/directive/input.js b/src/ng/directive/input.js index abc689193ee7..864b7bd782c3 100644 --- a/src/ng/directive/input.js +++ b/src/ng/directive/input.js @@ -2133,10 +2133,9 @@ var NgModelController = ['$scope', '$exceptionHandler', '$attrs', '$element', '$ viewValue = formatters[idx](viewValue); } - ctrl.$$runValidators(modelValue, viewValue); - if (ctrl.$viewValue !== viewValue) { ctrl.$viewValue = ctrl.$$lastCommittedViewValue = viewValue; + ctrl.$$runValidators(modelValue, viewValue); ctrl.$render(); } } diff --git a/test/ng/directive/inputSpec.js b/test/ng/directive/inputSpec.js index 3c5e6d437cb1..344debca4d0e 100644 --- a/test/ng/directive/inputSpec.js +++ b/test/ng/directive/inputSpec.js @@ -516,6 +516,17 @@ describe('NgModelController', function() { expect(ctrl.$pending).toBeUndefined(); })); + it('should not run validators in case view value is not re-rendered', function() { + ctrl.$formatters.push(function(value) { + return 'nochange'; + }); + + ctrl.$validators.spyValidator = jasmine.createSpy('spyValidator'); + scope.$apply('value = "first"'); + scope.$apply('value = "second"'); + expect(ctrl.$validators.spyValidator).toHaveBeenCalledOnce(); + }); + it('should throw an error when a promise is not returned for an asynchronous validator', inject(function($q) { ctrl.$asyncValidators.async = function(value) { return true; From 9b93cd5936ad16bf0c01bc2ff3a785833002a45a Mon Sep 17 00:00:00 2001 From: Shahar Talmi Date: Sat, 30 Aug 2014 12:05:32 +0300 Subject: [PATCH 2/2] fix(ngModel): make async validators play nicely with $parsers when an async validator resolves, we decrease the `pendingCount` only if the `$viewValue` hasn't changed in the mean time. before this commit the view value after resolve was compared with the model value before resolve instead of the view value before resolve. this caused it never to resolve when those two are different (when `$parsers` are defined) --- src/ng/directive/input.js | 18 +++++++++--------- test/ng/directive/inputSpec.js | 25 +++++++++++++++++++++++++ 2 files changed, 34 insertions(+), 9 deletions(-) diff --git a/src/ng/directive/input.js b/src/ng/directive/input.js index 864b7bd782c3..f0e8170f6f8a 100644 --- a/src/ng/directive/input.js +++ b/src/ng/directive/input.js @@ -1709,7 +1709,7 @@ var NgModelController = ['$scope', '$exceptionHandler', '$attrs', '$element', '$ $animate.removeClass($element, PENDING_CLASS); }; - this.$$setPending = function(validationErrorKey, promise, currentValue) { + this.$$setPending = function(validationErrorKey, promise, modelValue) { ctrl.$pending = ctrl.$pending || {}; if (angular.isUndefined(ctrl.$pending[validationErrorKey])) { ctrl.$pending[validationErrorKey] = true; @@ -1725,7 +1725,7 @@ var NgModelController = ['$scope', '$exceptionHandler', '$attrs', '$element', '$ //Special-case for (undefined|null|false|NaN) values to avoid //having to compare each of them with each other - currentValue = currentValue || ''; + var currentValue = ctrl.$viewValue || ''; promise.then(resolve(true), resolve(false)); function resolve(bool) { @@ -1737,7 +1737,7 @@ var NgModelController = ['$scope', '$exceptionHandler', '$attrs', '$element', '$ ctrl.$setValidity(validationErrorKey, bool); if (pendingCount === 0) { ctrl.$$clearPending(); - ctrl.$$updateValidModelValue(value); + ctrl.$$updateValidModelValue(modelValue); ctrl.$$writeModelToScope(); } } @@ -1924,14 +1924,14 @@ var NgModelController = ['$scope', '$exceptionHandler', '$attrs', '$element', '$ } var prev = ctrl.$modelValue; - ctrl.$$runValidators(ctrl.$$invalidModelValue || ctrl.$modelValue, ctrl.$viewValue); + ctrl.$$runValidators(ctrl.$$invalidModelValue || ctrl.$modelValue); if (prev !== ctrl.$modelValue) { ctrl.$$writeModelToScope(); } }; - this.$$runValidators = function(modelValue, viewValue) { - // this is called in the event if incase the input value changes + this.$$runValidators = function(modelValue) { + // this is called in the event if in case the input value changes // while a former asynchronous validator is still doing its thing if (ctrl.$pending) { ctrl.$$clearPending(); @@ -1956,7 +1956,7 @@ var NgModelController = ['$scope', '$exceptionHandler', '$attrs', '$element', '$ function validate(validators, callback) { var status = true; forEach(validators, function(fn, name) { - var result = fn(modelValue, viewValue); + var result = fn(modelValue, ctrl.$viewValue); callback(name, result); status = status && result; }); @@ -2016,7 +2016,7 @@ var NgModelController = ['$scope', '$exceptionHandler', '$attrs', '$element', '$ } else if (ctrl.$modelValue !== modelValue && (isUndefined(ctrl.$$invalidModelValue) || ctrl.$$invalidModelValue != modelValue)) { ctrl.$setValidity(parserName, true); - ctrl.$$runValidators(modelValue, viewValue); + ctrl.$$runValidators(modelValue); ctrl.$$writeModelToScope(); } }; @@ -2135,7 +2135,7 @@ var NgModelController = ['$scope', '$exceptionHandler', '$attrs', '$element', '$ if (ctrl.$viewValue !== viewValue) { ctrl.$viewValue = ctrl.$$lastCommittedViewValue = viewValue; - ctrl.$$runValidators(modelValue, viewValue); + ctrl.$$runValidators(modelValue); ctrl.$render(); } } diff --git a/test/ng/directive/inputSpec.js b/test/ng/directive/inputSpec.js index 344debca4d0e..b34badef2da7 100644 --- a/test/ng/directive/inputSpec.js +++ b/test/ng/directive/inputSpec.js @@ -527,6 +527,31 @@ describe('NgModelController', function() { expect(ctrl.$validators.spyValidator).toHaveBeenCalledOnce(); }); + it('should render a validator asynchronously when parser is defined', inject(function($q) { + var defer; + ctrl.$asyncValidators.promiseValidator = function(value) { + defer = $q.defer(); + return defer.promise; + }; + ctrl.$parsers.push(function(value) { + return value + '-a'; + }); + + ctrl.$setViewValue(''); + + expect(ctrl.$valid).toBeUndefined(); + expect(ctrl.$invalid).toBeUndefined(); + expect(ctrl.$pending.promiseValidator).toBe(true); + + defer.resolve(); + scope.$digest(); + + expect(ctrl.$valid).toBe(true); + expect(ctrl.$invalid).toBe(false); + expect(ctrl.$pending).toBeUndefined(); + expect(ctrl.$modelValue).toBe('-a'); + })); + it('should throw an error when a promise is not returned for an asynchronous validator', inject(function($q) { ctrl.$asyncValidators.async = function(value) { return true;