Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Clearer errors for common pitfalls #108

Merged
merged 1 commit into from
Jul 31, 2017
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 6 additions & 6 deletions src/ValidationError.js
Original file line number Diff line number Diff line change
@@ -1,8 +1,9 @@
import printValue from './util/printValue';

let strReg = /\$\{\s*(\w+)\s*\}/g;

let replace = str =>
params => str.replace(strReg, (_, key) => String(params[key]))
params => str.replace(strReg, (_, key) => printValue(params[key]))


export default function ValidationError(errors, value, field, type) {
Expand All @@ -13,16 +14,16 @@ export default function ValidationError(errors, value, field, type) {
this.errors = []
this.inner = []

if ( errors )
if (errors)
[].concat(errors).forEach(err => {
this.errors = this.errors.concat(err.errors || err)

if ( err.inner )
if (err.inner)
this.inner = this.inner.concat(err.inner.length ? err.inner : err)
})

this.message = this.errors.length > 1
? `${this.errors.length } errors occurred`
? `${this.errors.length} errors occurred`
: this.errors[0]

if (Error.captureStackTrace)
Expand All @@ -43,8 +44,7 @@ ValidationError.formatError = function(message, params) {

let fn = ({ path, label, ...params }) => {
params.path = label || path || 'this'

return message(params)
return typeof message === 'function' ? message(params) : message
}

return arguments.length === 1 ? fn : fn(params)
Expand Down
8 changes: 7 additions & 1 deletion src/array.js
Original file line number Diff line number Diff line change
Expand Up @@ -61,6 +61,9 @@ inherits(ArraySchema, MixedSchema, {
let endEarly = this._option('abortEarly', options)
let recursive = this._option('recursive', options)

let originalValue = options.originalValue != null ?
options.originalValue : _value

return MixedSchema.prototype._validate
.call(this, _value, options)
.catch(propagateErrors(endEarly, errors))
Expand All @@ -70,6 +73,8 @@ inherits(ArraySchema, MixedSchema, {
return value
}

originalValue = originalValue || value

let validations = value.map((item, idx) => {
var path = makePath`${options.path}[${idx}]`

Expand All @@ -78,7 +83,8 @@ inherits(ArraySchema, MixedSchema, {
...options,
path,
strict: true,
parent: value
parent: value,
originalValue: originalValue[idx]
};

if (subType.validate)
Expand Down
14 changes: 13 additions & 1 deletion src/locale.js
Original file line number Diff line number Diff line change
@@ -1,13 +1,25 @@
import printValue from './util/printValue';
import { getLocale } from './customLocale'

const customLocale = getLocale()

export let mixed = {
default: '${path} is invalid',
notType: '${path} must be a `${type}` type, got: "${value}" instead',
required: '${path} is a required field',
oneOf: '${path} must be one the following values: ${values}',
notOneOf: '${path} must not be one the following values: ${values}',
notType: ({ path, type, value, originalValue }) => {
let isCast = originalValue != null && originalValue !== value
let msg = `${path} must be a \`${type}\` type, ` +
`but the final value was: \`${printValue(value, true)}\`` + (isCast ?
` (cast from the value \`${printValue(originalValue, true)}\`).` : '.')

if (value === null) {
msg += `\n If "null" is intended as an empty value be sure to mark the schema as \`.nullable()\``
}

return msg;
},
...customLocale.mixed,
}

Expand Down
22 changes: 8 additions & 14 deletions src/mixed.js
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ import merge from './util/merge';
import isAbsent from './util/isAbsent';
import cloneDeep from './util/clone';
import createValidation from './util/createValidation';
import printValue from './util/printValue';
import BadSet from './util/set';
import Ref from './Reference';

Expand Down Expand Up @@ -140,14 +141,14 @@ SchemaType.prototype = {
options.assert !== false &&
resolvedSchema.isType(result) !== true
) {
let formattedValue = JSON.stringify(value);
let formattedResult = JSON.stringify(result);
let formattedValue = printValue(value);
let formattedResult = printValue(result);
throw new TypeError(
`The value of ${options.path || 'field'} could not be cast to a value ` +
`that satisfies the schema type: "${resolvedSchema._type}". \n\n` +
`attempted value: ${JSON.stringify(value)} \n` +
`attempted value: ${formattedValue} \n` +
((formattedResult !== formattedValue)
? `result of cast: ${JSON.stringify(result)}` : '')
? `result of cast: ${formattedResult}` : '')
);
}

Expand All @@ -173,6 +174,8 @@ SchemaType.prototype = {

_validate(_value, options = {}) {
let value = _value;
let originalValue = options.originalValue != null ?
options.originalValue : _value

let isStrict = this._option('strict', options)
let endEarly = this._option('abortEarly', options)
Expand All @@ -181,11 +184,10 @@ SchemaType.prototype = {
let label = this._label

if (!isStrict) {

value = this._cast(value, { assert: false, ...options })
}
// value is cast, we can check if it meets type requirements
let validationParams = { value, path, schema: this, options, label }
let validationParams = { value, path, schema: this, options, label, originalValue }
let initialTests = []

if (this._typeError)
Expand Down Expand Up @@ -429,11 +431,3 @@ Object.keys(aliases).forEach(method => {
)
})

function nodeify(promise, cb){
if (typeof cb !== 'function') return promise

promise.then(
val => cb(null, val),
err => cb(err)
)
}
24 changes: 17 additions & 7 deletions src/object.js
Original file line number Diff line number Diff line change
Expand Up @@ -121,13 +121,15 @@ inherits(ObjectSchema, MixedSchema, {
},

_validate(_value, opts = {}) {
var errors = []
, endEarly, recursive;
let endEarly, recursive;
let errors = []
let originalValue = opts.originalValue != null ?
opts.originalValue : _value

endEarly = this._option('abortEarly', opts)
recursive = this._option('recursive', opts)

opts = {...opts, __validating: true };
opts = { ...opts, __validating: true, originalValue };

return MixedSchema.prototype._validate
.call(this, _value, opts)
Expand All @@ -138,14 +140,22 @@ inherits(ObjectSchema, MixedSchema, {
return value
}

originalValue = originalValue || value

let validations = this._nodes.map(key => {
var path = makePath`${opts.path}.${key}`
, field = this.fields[key]
, innerOptions = { ...opts, path, parent: value };
let path = makePath`${opts.path}.${key}`
let field = this.fields[key]

let innerOptions = {
...opts,
path,
parent: value,
originalValue: originalValue[key],
};

if (field) {
// inner fields are always strict:
// 1. this isn't strict so we just cast the value leaving nested values already cast
// 1. this isn't strict so the casting will also have cast inner values
// 2. this is strict in which case the nested values weren't cast either
innerOptions.strict = true;

Expand Down
22 changes: 15 additions & 7 deletions src/util/createValidation.js
Original file line number Diff line number Diff line change
Expand Up @@ -8,30 +8,38 @@ function resolveParams(oldParams, newParams, resolve) {
return mapValues({ ...oldParams, ...newParams }, resolve)
}

function createErrorFactory({ value, label, resolve, ...opts}) {
function createErrorFactory({ value, label, resolve, originalValue, ...opts}) {
return function createError({ path = opts.path, message = opts.message, type = opts.name, params } = {}) {
params = { path, value, label, ...resolveParams(opts.params, params, resolve) };
params = {
path,
value,
originalValue,
label,
...resolveParams(opts.params, params, resolve)
};

return Object.assign(new ValidationError(
typeof message ==='string' ? formatError(message, params) : message
return Object.assign(
new ValidationError(
formatError(message, params)
, value
, path
, type)
, type
)
, { params })
}
}

export default function createValidation(options) {
let { name, message, test, params } = options

function validate({ value, path, label, options, ...rest }) {
function validate({ value, path, label, options, originalValue, ...rest }) {
let parent = options.parent;
var resolve = (value) => Ref.isRef(value)
? value.getValue(parent, options.context)
: value

var createError = createErrorFactory({
message, path, value, params
message, path, value, originalValue, params
, label, resolve, name
})

Expand Down
67 changes: 67 additions & 0 deletions src/util/printValue.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,67 @@
import isFunction from 'lodash/isFunction';
import isMap from 'lodash/isMap';
import isSet from 'lodash/isSet';
import isWeakMap from 'lodash/isWeakMap';
import isWeakSet from 'lodash/isWeakSet';
import isSymbol from 'lodash/isSymbol';

const toString = Object.prototype.toString;
const toISOString = Date.prototype.toISOString;
const errorToString = Error.prototype.toString;
const regExpToString = RegExp.prototype.toString;
const symbolToString = Symbol.prototype.toString;

const SYMBOL_REGEXP = /^Symbol\((.*)\)(.*)$/;
const NEWLINE_REGEXP = /\n/gi;

const getSymbols = Object.getOwnPropertySymbols || (obj => []);


function printNumber(val) {
if (val != +val) return 'NaN';
const isNegativeZero = val === 0 && 1 / val < 0;
return isNegativeZero ? '-0' : '' + val;
}

function printFunction(val) {
return '[Function ' + (val.name || 'anonymous') + ']';
}

function printSymbol(val: Symbol): string {
return symbolToString.call(val).replace(SYMBOL_REGEXP, 'Symbol($1)');
}

function printError(val: Error): string {
return '[' + errorToString.call(val) + ']';
}

function printSimpleValue(val, quoteStrings = false) {
if (val === true || val === false) return '' + val;
if (val === undefined) return 'undefined';
if (val === null) return 'null';

const typeOf = typeof val;

if (typeOf === 'number') return printNumber(val);
if (typeOf === 'string') return quoteStrings ? `"${val}"` : val;
if (isFunction(val)) return printFunction(val);
if (isSymbol(val)) return printSymbol(val);

const tag = toString.call(val);
if (tag === '[object Date]') return isNaN(val.getTime()) ? String(val) : toISOString.call(val);
if (tag === '[object Error]' || val instanceof Error) return printError(val);
if (tag === '[object RegExp]') return regExpToString.call(val);

return null;
}

export default function printValue(value, quoteStrings) {
let result = printSimpleValue(value, quoteStrings);
if (result !== null) return result;

return JSON.stringify(value, function (key, value) {
let result = printSimpleValue(this[key], quoteStrings);
if (result !== null) return result
return value
}, 2)
}
7 changes: 7 additions & 0 deletions test/ValidationError.js
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,13 @@ describe('ValidationError', function() {
str.should.contain('null')
})

it(`should include "NaN" in the message if null is provided as a param`, function() {
const str = ValidationError.formatError('${path} value is ${min}', {
min: NaN
})
str.should.contain('NaN')
})

it(`should include 0 in the message if 0 is provided as a param`, function() {
const str = ValidationError.formatError('${path} value is ${min}', {
min: 0
Expand Down
16 changes: 15 additions & 1 deletion test/mixed.js
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
import mixed from '../src/mixed';
import object from '../src/object';
import string from '../src/string';
import ValidationError from '../src/ValidationError';
import number from '../src/number';
import reach from '../src/util/reach';

let noop = () => {}
Expand Down Expand Up @@ -32,6 +32,20 @@ describe( 'Mixed Types ', function(){
inst.cast(undefined).should.equal('hello')
})

it('should warn about null types', async () => {
let error = await string().strict()
.validate(null).should.be.rejected()

expect(error.message).to.match(/If "null" is intended/)
})

it('should print the original value', async () => {
let error = await number()
.validate('john').should.be.rejected()

expect(error.message).to.match(/the final value was: `NaN`.+cast from the value `"john"`/)
})

it('should check types', async () => {
let inst = string().strict().typeError('must be a ${type}!')

Expand Down