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

Commit

Permalink
fix(ngModel): treat undefined parse responses as parse errors
Browse files Browse the repository at this point in the history 
With this commit, ngModel will now handle parsing first and then validation
afterwards once the parsing is successful. If any parser along the way returns
`undefined` then ngModel will break the chain of parsing and register a
a parser error represented by the type of input that is being collected
(e.g. number, date, datetime, url, etc...). If a parser fails for a standard
text input field then an error of `parse` will be placed on `model.$error`.

BREAKING CHANGE

Any parser code from before that returned an `undefined` value
(or nothing at all) will now cause a parser failure. When this occurs
none of the validators present in `$validators` will run until the parser
error is gone.
  • Loading branch information
@matsko
matsko committed Aug 26, 2014
1 parent 0e44ac2 commit db044c4
Show file tree
Hide file tree
Showing 3 changed files with 264 additions and 104 deletions.
11 changes: 8 additions & 3 deletions src/ng/directive/form.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,8 @@ var nullFormCtrl = {
$setValidity: noop,
$setDirty: noop,
$setPristine: noop,
$setSubmitted: noop
$setSubmitted: noop,
$$clearControlValidity: noop
},
SUBMITTED_CLASS = 'ng-submitted';

Expand Down Expand Up @@ -144,11 +145,15 @@ function FormController(element, attrs, $scope, $animate) {
if (control.$name && form[control.$name] === control) {
delete form[control.$name];
}

form.$$clearControlValidity(control);
arrayRemove(controls, control);
};

form.$$clearControlValidity = function(control) {
forEach(errors, function(queue, validationToken) {
form.$setValidity(validationToken, true, control);
});

arrayRemove(controls, control);
};

/**
Expand Down
166 changes: 75 additions & 91 deletions src/ng/directive/input.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ var MONTH_REGEXP = /^(\d{4})-(\d\d)$/;
var TIME_REGEXP = /^(\d\d):(\d\d)(?::(\d\d))?$/;
var DEFAULT_REGEXP = /(\s+|^)default(\s+|$)/;

var $ngModelMinErr = new minErr('ngModel');
var inputType = {

/**
Expand Down Expand Up @@ -885,13 +886,6 @@ var inputType = {
'file': noop
};

// A helper function to call $setValidity and return the value / undefined,
// a pattern that is repeated a lot in the input validation logic.
function validate(ctrl, validatorName, validity, value){
ctrl.$setValidity(validatorName, validity);
return validity ? value : undefined;
}

function testFlags(validity, flags) {
var i, flag;
if (flags) {
Expand All @@ -905,25 +899,6 @@ function testFlags(validity, flags) {
return false;
}

// Pass validity so that behaviour can be mocked easier.
function addNativeHtml5Validators(ctrl, validatorName, badFlags, ignoreFlags, validity) {
if (isObject(validity)) {
ctrl.$$hasNativeValidators = true;
var validator = function(value) {
// Don't overwrite previous validation, don't consider valueMissing to apply (ng-required can
// perform the required validation)
if (!ctrl.$error[validatorName] &&
!testFlags(validity, ignoreFlags) &&
testFlags(validity, badFlags)) {
ctrl.$setValidity(validatorName, false);
return;
}
return value;
};
ctrl.$parsers.push(validator);
}
}

function textInputType(scope, element, attr, ctrl, $sniffer, $browser) {
var validity = element.prop(VALIDITY_STATE_PROPERTY);
var placeholder = element[0].placeholder, noevent = {};
Expand Down Expand Up @@ -1074,25 +1049,20 @@ function createDateParser(regexp, mapping) {

function createDateInputType(type, regexp, parseDate, format) {
return function dynamicDateInputType(scope, element, attr, ctrl, $sniffer, $browser, $filter) {
badInputChecker(scope, element, attr, ctrl);
textInputType(scope, element, attr, ctrl, $sniffer, $browser);
var timezone = ctrl && ctrl.$options && ctrl.$options.timezone;

ctrl.$$parserName = type;
ctrl.$parsers.push(function(value) {
if(ctrl.$isEmpty(value)) {
ctrl.$setValidity(type, true);
return null;
}

if(regexp.test(value)) {
ctrl.$setValidity(type, true);
if (ctrl.$isEmpty(value)) return null;
if (regexp.test(value)) {
var parsedDate = parseDate(value);
if (timezone === 'UTC') {
parsedDate.setMinutes(parsedDate.getMinutes() - parsedDate.getTimezoneOffset());
}
return parsedDate;
}

ctrl.$setValidity(type, false);
return undefined;
});

Expand All @@ -1104,90 +1074,80 @@ function createDateInputType(type, regexp, parseDate, format) {
});

if(attr.min) {
var minValidator = function(value) {
var valid = ctrl.$isEmpty(value) ||
(parseDate(value) >= parseDate(attr.min));
ctrl.$setValidity('min', valid);
return valid ? value : undefined;
};

ctrl.$parsers.push(minValidator);
ctrl.$formatters.push(minValidator);
ctrl.$validators.min = function(value) {
return ctrl.$isEmpty(value) || isUndefined(attr.min) || parseDate(value) >= parseDate(attr.min);
};
}

if(attr.max) {
var maxValidator = function(value) {
var valid = ctrl.$isEmpty(value) ||
(parseDate(value) <= parseDate(attr.max));
ctrl.$setValidity('max', valid);
return valid ? value : undefined;
};

ctrl.$parsers.push(maxValidator);
ctrl.$formatters.push(maxValidator);
ctrl.$validators.max = function(value) {
return ctrl.$isEmpty(value) || isUndefined(attr.max) || parseDate(value) <= parseDate(attr.max);
};
}
};
}

var numberBadFlags = ['badInput'];
function badInputChecker(scope, element, attr, ctrl) {
var node = element[0];
var nativeValidation = ctrl.$$hasNativeValidators = isObject(node.validity);
if (nativeValidation) {
ctrl.$parsers.push(function(value) {
var validity = element.prop(VALIDITY_STATE_PROPERTY) || {};
return validity.badInput || validity.typeMismatch ? undefined : value;
});
}
}

function numberInputType(scope, element, attr, ctrl, $sniffer, $browser) {
badInputChecker(scope, element, attr, ctrl);
textInputType(scope, element, attr, ctrl, $sniffer, $browser);

ctrl.$$parserName = 'number';
ctrl.$parsers.push(function(value) {
var empty = ctrl.$isEmpty(value);
if (empty || NUMBER_REGEXP.test(value)) {
ctrl.$setValidity('number', true);
return value === '' ? null : (empty ? value : parseFloat(value));
} else {
ctrl.$setValidity('number', false);
return undefined;
}
if(ctrl.$isEmpty(value)) return null;
if(NUMBER_REGEXP.test(value)) return parseFloat(value);
return undefined;
});

addNativeHtml5Validators(ctrl, 'number', numberBadFlags, null, ctrl.$$validityState);

ctrl.$formatters.push(function(value) {
return ctrl.$isEmpty(value) ? '' : '' + value;
if (!ctrl.$isEmpty(value)) {
if (!isNumber(value)) {
throw $ngModelMinErr('numfmt', 'Expected `{0}` to be a number', value);
}
value = value.toString();
}
return value;
});

if (attr.min) {
var minValidator = function(value) {
var min = parseFloat(attr.min);
return validate(ctrl, 'min', ctrl.$isEmpty(value) || value >= min, value);
ctrl.$validators.min = function(value) {
return ctrl.$isEmpty(value) || isUndefined(attr.min) || value >= parseFloat(attr.min);
};

ctrl.$parsers.push(minValidator);
ctrl.$formatters.push(minValidator);
}

if (attr.max) {
var maxValidator = function(value) {
var max = parseFloat(attr.max);
return validate(ctrl, 'max', ctrl.$isEmpty(value) || value <= max, value);
ctrl.$validators.max = function(value) {
return ctrl.$isEmpty(value) || isUndefined(attr.max) || value <= parseFloat(attr.max);
};

ctrl.$parsers.push(maxValidator);
ctrl.$formatters.push(maxValidator);
}

ctrl.$formatters.push(function(value) {
return validate(ctrl, 'number', ctrl.$isEmpty(value) || isNumber(value), value);
});
}

function urlInputType(scope, element, attr, ctrl, $sniffer, $browser) {
badInputChecker(scope, element, attr, ctrl);
textInputType(scope, element, attr, ctrl, $sniffer, $browser);

ctrl.$$parserName = 'url';
ctrl.$validators.url = function(modelValue, viewValue) {
var value = modelValue || viewValue;
return ctrl.$isEmpty(value) || URL_REGEXP.test(value);
};
}

function emailInputType(scope, element, attr, ctrl, $sniffer, $browser) {
badInputChecker(scope, element, attr, ctrl);
textInputType(scope, element, attr, ctrl, $sniffer, $browser);

ctrl.$$parserName = 'email';
ctrl.$validators.email = function(modelValue, viewValue) {
var value = modelValue || viewValue;
return ctrl.$isEmpty(value) || EMAIL_REGEXP.test(value);
Expand Down Expand Up @@ -1223,7 +1183,7 @@ function parseConstantExpr($parse, context, name, expression, fallback) {
if (isDefined(expression)) {
parseFn = $parse(expression);
if (!parseFn.constant) {
throw new minErr('ngModel')('constexpr', 'Expected constant expression for `{0}`, but saw ' +
throw minErr('ngModel')('constexpr', 'Expected constant expression for `{0}`, but saw ' +
'`{1}`.', name, expression);
}
return parseFn(context);
Expand Down Expand Up @@ -1598,7 +1558,7 @@ var NgModelController = ['$scope', '$exceptionHandler', '$attrs', '$element', '$
ctrl = this;

if (!ngModelSet) {
throw minErr('ngModel')('nonassign', "Expression '{0}' is non-assignable. Element: {1}",
throw $ngModelMinErr('nonassign', "Expression '{0}' is non-assignable. Element: {1}",
$attr.ngModel, startingTag($element));
}

Expand Down Expand Up @@ -1663,6 +1623,19 @@ var NgModelController = ['$scope', '$exceptionHandler', '$attrs', '$element', '$
$animate.addClass($element, (isValid ? VALID_CLASS : INVALID_CLASS) + validationErrorKey);
}

this.$$clearValidity = function() {
forEach(ctrl.$error, function(val, key) {
var validationKey = snake_case(key, '-');
$animate.removeClass($element, VALID_CLASS + validationKey);
$animate.removeClass($element, INVALID_CLASS + validationKey);
});

invalidCount = 0;
$error = ctrl.$error = {};

parentForm.$$clearControlValidity(ctrl);
};

/**
* @ngdoc method
* @name ngModel.NgModelController#$setValidity
Expand Down Expand Up @@ -1694,7 +1667,7 @@ var NgModelController = ['$scope', '$exceptionHandler', '$attrs', '$element', '$
ctrl.$valid = true;
ctrl.$invalid = false;
}
} else {
} else if(!$error[validationErrorKey]) {
toggleValidCss(false);
ctrl.$invalid = true;
ctrl.$valid = false;
Expand Down Expand Up @@ -1883,16 +1856,27 @@ var NgModelController = ['$scope', '$exceptionHandler', '$attrs', '$element', '$
parentForm.$setDirty();
}

var modelValue = viewValue;
forEach(ctrl.$parsers, function(fn) {
modelValue = fn(modelValue);
});
var hasBadInput, modelValue = viewValue;
for(var i = 0; i < ctrl.$parsers.length; i++) {
modelValue = ctrl.$parsers[i](modelValue);
if(isUndefined(modelValue)) {
hasBadInput = true;
break;
}
}

if (ctrl.$modelValue !== modelValue &&
(isUndefined(ctrl.$$invalidModelValue) || ctrl.$$invalidModelValue != modelValue)) {
var parserName = ctrl.$$parserName || 'parse';
if (hasBadInput) {
ctrl.$$invalidModelValue = ctrl.$modelValue = undefined;
ctrl.$$clearValidity();
ctrl.$setValidity(parserName, false);
} else if (ctrl.$modelValue !== modelValue &&
(isUndefined(ctrl.$$invalidModelValue) || ctrl.$$invalidModelValue != modelValue)) {
ctrl.$setValidity(parserName, true);
ctrl.$$runValidators(modelValue, viewValue);
ctrl.$$writeModelToScope();
}

ctrl.$$writeModelToScope();
};

this.$$writeModelToScope = function() {
Expand Down
Loading

1 comment on commit db044c4

@IgorMinar
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

a breaking change not documented in the commit message is that now we expect all values for input[number] flowing from model to view to be of type number otherwise ngModel:numfmt error is thrown.

This error points out a programming error when application's model deals with strings and numbers interchangeably. This must be avoided to prevent silent errors throughout the app.

Please sign in to comment.