Skip to content

Commit 1a1d0fb

Browse files
committed
fix(ngModel): treat synchronous validators as boolean always
Change synchronous validators to convert the return to boolean value. Prevent unexpected behavior when returning `undefined`. Closes angular#14734 BREAKING CHANGE: Previously, only a literal `false` return would resolve as the synchronous validator failing. Now, all traditionally false JavaScript values are treated as failing the validator, as one would naturally expect. To migrate, make sure your synchronous validators are returning either a literal `true` or a literal `false` value. For most code, we expect this to already be the case. Only a very small subset of projects will affected. Namely, anyone using `undefined` or any falsy value as a return will now see their validation failing, whereas previously falsy values other than `undefined` would have been seen as passing and `undefined` would have been seen as pending.
1 parent a812954 commit 1a1d0fb

File tree

2 files changed

+2
-39
lines changed

2 files changed

+2
-39
lines changed

src/ng/directive/ngModel.js

+1-2
Original file line numberDiff line numberDiff line change
@@ -603,8 +603,7 @@ NgModelController.prototype = {
603603
function processSyncValidators() {
604604
var syncValidatorsValid = true;
605605
forEach(that.$validators, function(validator, name) {
606-
var result = validator(modelValue, viewValue);
607-
// var result = Boolean(validator(modelValue, viewValue));
606+
var result = Boolean(validator(modelValue, viewValue));
608607
syncValidatorsValid = syncValidatorsValid && result;
609608
setValidity(name, result);
610609
});

test/ng/directive/ngModelSpec.js

+1-37
Original file line numberDiff line numberDiff line change
@@ -847,43 +847,7 @@ describe('ngModel', function() {
847847
expect(ctrl.$valid).toBe(true);
848848
});
849849

850-
// See also the test on L886.
851-
fit('should demonstrate how synchronous validators work now', function() {
852-
var curry = function(v) {
853-
return function() {
854-
return v;
855-
};
856-
};
857-
858-
var tester = function(v, e) {
859-
ctrl.$modelValue = undefined;
860-
ctrl.$validators.a = curry(v);
861-
862-
ctrl.$validate();
863-
expect(ctrl.$valid).toBe(e);
864-
};
865-
866-
// False tests
867-
tester(false, false);
868-
tester(undefined, undefined);
869-
tester(null, true);
870-
tester(0, true);
871-
tester(NaN, true);
872-
tester('', true);
873-
874-
// True tests
875-
tester(true, true);
876-
tester(1, true);
877-
tester('0', true);
878-
tester('false', true);
879-
tester([], true);
880-
tester({}, true);
881-
});
882-
883-
// Flip this test with the one above (L851) as well as line L606 with L607 in `ngModel.js`
884-
// The differences between the two would likely being a breaking change
885-
// Though the feature it is breaking was undocumented.
886-
xit('should demonstrate how synchronous validators will work after this change', function() {
850+
it('should treat all responses as boolean for synchronous validators', function() {
887851
var curry = function(v) {
888852
return function() {
889853
return v;

0 commit comments

Comments
 (0)