From 9ac8e1380197ee46b8797734c26ba30f3ef99568 Mon Sep 17 00:00:00 2001 From: Jason Quense Date: Mon, 31 Jul 2017 10:13:25 -0400 Subject: [PATCH] Clearer errors for common pitfalls --- src/ValidationError.js | 12 +++---- src/array.js | 8 ++++- src/locale.js | 14 +++++++- src/mixed.js | 22 +++++------- src/object.js | 24 +++++++++---- src/util/createValidation.js | 22 ++++++++---- src/util/printValue.js | 67 ++++++++++++++++++++++++++++++++++++ test/ValidationError.js | 7 ++++ test/mixed.js | 16 ++++++++- 9 files changed, 155 insertions(+), 37 deletions(-) create mode 100644 src/util/printValue.js diff --git a/src/ValidationError.js b/src/ValidationError.js index 5431ae68f..6bacf2ced 100644 --- a/src/ValidationError.js +++ b/src/ValidationError.js @@ -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) { @@ -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) @@ -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) diff --git a/src/array.js b/src/array.js index 63794149f..d00264b26 100644 --- a/src/array.js +++ b/src/array.js @@ -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)) @@ -70,6 +73,8 @@ inherits(ArraySchema, MixedSchema, { return value } + originalValue = originalValue || value + let validations = value.map((item, idx) => { var path = makePath`${options.path}[${idx}]` @@ -78,7 +83,8 @@ inherits(ArraySchema, MixedSchema, { ...options, path, strict: true, - parent: value + parent: value, + originalValue: originalValue[idx] }; if (subType.validate) diff --git a/src/locale.js b/src/locale.js index c42ccad2f..5bf17f17b 100644 --- a/src/locale.js +++ b/src/locale.js @@ -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, } diff --git a/src/mixed.js b/src/mixed.js index c6fa6ceda..56f222a41 100644 --- a/src/mixed.js +++ b/src/mixed.js @@ -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'; @@ -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}` : '') ); } @@ -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) @@ -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) @@ -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) - ) -} diff --git a/src/object.js b/src/object.js index 868dd8193..b3e53b1f5 100644 --- a/src/object.js +++ b/src/object.js @@ -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) @@ -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; diff --git a/src/util/createValidation.js b/src/util/createValidation.js index 3663e7f58..7bbbccc11 100644 --- a/src/util/createValidation.js +++ b/src/util/createValidation.js @@ -8,15 +8,23 @@ 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 }) } } @@ -24,14 +32,14 @@ function createErrorFactory({ value, label, resolve, ...opts}) { 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 }) diff --git a/src/util/printValue.js b/src/util/printValue.js new file mode 100644 index 000000000..db3edf377 --- /dev/null +++ b/src/util/printValue.js @@ -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) +} diff --git a/test/ValidationError.js b/test/ValidationError.js index 78ea48a51..153b68637 100644 --- a/test/ValidationError.js +++ b/test/ValidationError.js @@ -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 diff --git a/test/mixed.js b/test/mixed.js index 897d15eb1..dd588e0ab 100644 --- a/test/mixed.js +++ b/test/mixed.js @@ -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 = () => {} @@ -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}!')