Repository: jquense/yup
Pull request #598 (2019-08-11 10:52:27)

fix: synchronous conditional object validation with unknown dependencies
This PR fixes an issue where using `when` on an object with dependencies on properties that were not specified in the schema, caused the validation to become asynchronous even with `sync` set to true.


Patch:
diff --git a/src/object.js b/src/object.js
index c624cb4..fef9d0c 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({


Test patch:
diff --git a/test/helpers.js b/test/helpers.js
index 057d4c6..626bcde 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 3f744a1..1a9957c 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 35db472..cc847e5 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({