From 1081c4168b55d7a2d97011b69084ae0b1c93f022 Mon Sep 17 00:00:00 2001 From: mikaelkolkinn Date: Sun, 11 Aug 2019 15:52:21 +0200 Subject: [PATCH] fix: synchronous conditional object validation with unknown dependencies (#598) * Moved the `ensureSync` utility method from `test/mixed.js` into `test/helpers.js`. * Tests for reproducing a bug where unknown object dependencies cause validation to become asynchronous. * Ensure that `object._validate` is synchronous when `sync` is true for unknown properties --- src/object.js | 5 +++- test/helpers.js | 17 +++++++++++++ test/mixed.js | 26 ++++---------------- test/object.js | 63 +++++++++++++++++++++++++++++++++++++++++++++++++ 4 files changed, 89 insertions(+), 22 deletions(-) diff --git a/src/object.js b/src/object.js index c624cb421..fef9d0c64 100644 --- a/src/object.js +++ b/src/object.js @@ -12,9 +12,12 @@ import sortByKeyOrder from './util/sortByKeyOrder'; import inherits from './util/inherits'; import makePath from './util/makePath'; import runValidations, { propagateErrors } from './util/runValidations'; +import { SynchronousPromise } from 'synchronous-promise'; let isObject = obj => Object.prototype.toString.call(obj) === '[object Object]'; +let promise = sync => (sync ? SynchronousPromise : Promise); + function unknown(ctx, value) { let known = Object.keys(ctx.fields); return Object.keys(value).filter(key => known.indexOf(key) === -1); @@ -167,7 +170,7 @@ inherits(ObjectSchema, MixedSchema, { return field.validate(value[key], innerOptions); } - return Promise.resolve(true); + return promise(sync).resolve(true); }); return runValidations({ diff --git a/test/helpers.js b/test/helpers.js index 057d4c664..626bcde6a 100644 --- a/test/helpers.js +++ b/test/helpers.js @@ -43,3 +43,20 @@ export let validateAll = (inst, { valid = [], invalid = [] }) => { }); } }; + +export function ensureSync(fn) { + let run = false; + let resolve = t => { + if (!run) return t; + throw new Error('Did not execute synchronously'); + }; + let err = t => { + if (!run) throw t; + throw new Error('Did not execute synchronously'); + }; + + let result = fn().then(resolve, err); + + run = true; + return result; +} diff --git a/test/mixed.js b/test/mixed.js index 3f744a1cc..1a9957c73 100644 --- a/test/mixed.js +++ b/test/mixed.js @@ -1,35 +1,19 @@ import { array, + bool, + lazy, mixed, - string, number, object, - ref, reach, - bool, - lazy, + ref, + string, ValidationError, } from '../src'; +import { ensureSync } from './helpers'; let noop = () => {}; -function ensureSync(fn) { - let run = false; - let resolve = t => { - if (!run) return t; - throw new Error('Did not execute synchonously'); - }; - let err = t => { - if (!run) throw t; - throw new Error('Did not execute synchonously'); - }; - - let result = fn().then(resolve, err); - - run = true; - return result; -} - global.YUP_USE_SYNC && it('[internal] normal methods should be running in sync Mode', async () => { let schema = number(); diff --git a/test/object.js b/test/object.js index 35db47246..cc847e5b3 100644 --- a/test/object.js +++ b/test/object.js @@ -10,6 +10,7 @@ import { lazy, reach, } from '../src'; +import { ensureSync } from './helpers'; describe('Object types', () => { describe('casting', () => { @@ -715,6 +716,68 @@ describe('Object types', () => { ]); }); + it('should handle conditionals with unknown dependencies', () => { + let inst = object().shape({ + value: number().when('isRequired', { + is: true, + then: number().required(), + }), + }); + + return Promise.all([ + inst + .isValid({ + isRequired: true, + value: 1234, + }) + .should.eventually.equal(true), + inst + .isValid({ + isRequired: true, + }) + .should.eventually.equal(false), + + inst + .isValid({ + isRequired: false, + value: 1234, + }) + .should.eventually.equal(true), + inst + .isValid({ + value: 1234, + }) + .should.eventually.equal(true), + ]); + }); + + global.YUP_USE_SYNC && + it('should handle conditionals synchronously', () => { + let inst = object().shape({ + knownDependency: bool(), + value: number().when(['unknownDependency', 'knownDependency'], { + is: true, + then: number().required(), + }), + }); + + return Promise.all([ + ensureSync(() => + inst.validate({ + unknownDependency: true, + knownDependency: true, + value: 1234, + }), + ).should.not.be.rejected(), + ensureSync(() => + inst.validate({ + unknownDependency: true, + knownDependency: true, + }), + ).should.be.rejectedWith(Error, /required/), + ]); + }); + it('should allow opt out of topo sort on specific edges', () => { (function() { object().shape({