From 7705972a8b7e29be4703ad97753af22efed0f0f9 Mon Sep 17 00:00:00 2001 From: Dmitrii Maganov Date: Wed, 6 Feb 2019 16:29:44 +0300 Subject: [PATCH] fix: concat of mixed and subtype (#444) * fix: concat of mixed and subtype * fix: concat with lazy --- src/mixed.js | 16 +++++++--------- src/util/merge.js | 30 ------------------------------ src/util/prependDeep.js | 27 +++++++++++++++++++++++++++ test/mixed.js | 21 ++++++++++++++++++++- test/yup.js | 21 ++++++++++++++++----- 5 files changed, 70 insertions(+), 45 deletions(-) delete mode 100644 src/util/merge.js create mode 100644 src/util/prependDeep.js diff --git a/src/mixed.js b/src/mixed.js index ac45454cb..540057e63 100644 --- a/src/mixed.js +++ b/src/mixed.js @@ -5,7 +5,7 @@ import toArray from 'lodash/toArray'; import { mixed as locale } from './locale'; import Condition from './Condition'; import runValidations from './util/runValidations'; -import merge from './util/merge'; +import prependDeep from './util/prependDeep'; import isSchema from './util/isSchema'; import isAbsent from './util/isAbsent'; import createValidation from './util/createValidation'; @@ -103,7 +103,7 @@ const proto = (SchemaType.prototype = { }, concat(schema) { - if (!schema) return this; + if (!schema || schema === this) return this; if (schema._type !== this._type && this._type !== 'mixed') throw new TypeError( @@ -111,14 +111,14 @@ const proto = (SchemaType.prototype = { this._type } and ${schema._type}`, ); - var cloned = this.clone(); - var next = merge(this.clone(), schema.clone()); - // undefined isn't merged over, but is a valid value for default + var next = prependDeep(schema.clone(), this); + + // new undefined default is overriden by old non-undefined one, revert if (has(schema, '_default')) next._default = schema._default; - next.tests = cloned.tests; - next._exclusive = cloned._exclusive; + next.tests = this.tests; + next._exclusive = this._exclusive; // manually add the new tests to ensure // the deduping logic is consistent @@ -126,8 +126,6 @@ const proto = (SchemaType.prototype = { next = next.test(fn.OPTIONS); }); - next._type = schema._type; - return next; }, diff --git a/src/util/merge.js b/src/util/merge.js deleted file mode 100644 index 79ab8a2ba..000000000 --- a/src/util/merge.js +++ /dev/null @@ -1,30 +0,0 @@ -import has from 'lodash/has'; -import isSchema from './isSchema'; - -let isObject = obj => Object.prototype.toString.call(obj) === '[object Object]'; - -export default function merge(target, source) { - for (var key in source) - if (has(source, key)) { - var targetVal = target[key], - sourceVal = source[key]; - - if (sourceVal === undefined) continue; - - if (isSchema(sourceVal)) { - target[key] = isSchema(targetVal) - ? targetVal.concat(sourceVal) - : sourceVal; - } else if (isObject(sourceVal)) { - target[key] = isObject(targetVal) - ? merge(targetVal, sourceVal) - : sourceVal; - } else if (Array.isArray(sourceVal)) { - target[key] = Array.isArray(targetVal) - ? targetVal.concat(sourceVal) - : sourceVal; - } else target[key] = source[key]; - } - - return target; -} diff --git a/src/util/prependDeep.js b/src/util/prependDeep.js new file mode 100644 index 000000000..6d00e75d4 --- /dev/null +++ b/src/util/prependDeep.js @@ -0,0 +1,27 @@ +import has from 'lodash/has'; +import isSchema from './isSchema'; + +let isObject = obj => Object.prototype.toString.call(obj) === '[object Object]'; + +export default function prependDeep(target, source) { + for (var key in source) + if (has(source, key)) { + var sourceVal = source[key], + targetVal = target[key]; + + if (targetVal === undefined) { + target[key] = sourceVal; + } else if (targetVal === sourceVal) { + continue; + } else if (isSchema(targetVal)) { + if (isSchema(sourceVal)) target[key] = sourceVal.concat(targetVal); + } else if (isObject(targetVal)) { + if (isObject(sourceVal)) + target[key] = prependDeep(targetVal, sourceVal); + } else if (Array.isArray(targetVal)) { + if (Array.isArray(sourceVal)) target[key] = sourceVal.concat(targetVal); + } + } + + return target; +} diff --git a/test/mixed.js b/test/mixed.js index 153f79ed3..a71efe828 100644 --- a/test/mixed.js +++ b/test/mixed.js @@ -1,4 +1,15 @@ -import { array, mixed, string, number, object, ref, reach, bool } from '../src'; +import { + array, + mixed, + string, + number, + object, + ref, + reach, + bool, + ValidationError, +} from '../src'; + let noop = () => {}; function ensureSync(fn) { @@ -585,6 +596,14 @@ describe('Mixed Types ', () => { }.should.not.throw(TypeError)); }); + it('concat should validate with mixed and other type', async function() { + let inst = mixed().concat(number()); + + await inst + .validate([]) + .should.be.rejected(ValidationError, /should be a `number`/); + }); + it('concat should maintain undefined defaults', function() { let inst = string().default('hi'); diff --git a/test/yup.js b/test/yup.js index 51f54e557..9ec8d51ac 100644 --- a/test/yup.js +++ b/test/yup.js @@ -1,5 +1,5 @@ import reach, { getIn } from '../src/util/reach'; -import merge from '../src/util/merge'; +import prependDeep from '../src/util/prependDeep'; import { settled } from '../src/util/runValidations'; import { object, array, string, lazy, number } from '../src'; @@ -36,11 +36,11 @@ describe('Yup', function() { ]); }); - it('should merge', function() { - var a = { a: 1, b: 'hello', c: [1, 2, 3], d: { a: /hi/ }, e: { b: 5 } }; - var b = { a: 4, c: [4, 5, 3], d: { b: 'hello' }, f: { c: 5 }, g: null }; + it('should prepend deeply', function() { + var a = { a: 4, c: [4, 5, 3], d: { b: 'hello' }, f: { c: 5 }, g: null }; + var b = { a: 1, b: 'hello', c: [1, 2, 3], d: { a: /hi/ }, e: { b: 5 } }; - merge(a, b).should.deep.eql({ + prependDeep(a, b).should.deep.eql({ a: 4, b: 'hello', c: [1, 2, 3, 4, 5, 3], @@ -54,6 +54,17 @@ describe('Yup', function() { }); }); + it('should not prepend needlesly', function() { + var schema = string(); + var spy = sinon.spy(schema, 'concat'); + var a = { schema }; + var b = { schema }; + var c = prependDeep(a, b); + + c.schema.should.equal(schema); + spy.should.not.have.been.called(); + }); + it('should getIn correctly', async () => { var num = number(), inst = object().shape({