Skip to content

Commit

Permalink
fix: synchronous conditional object validation with unknown dependenc…
Browse files Browse the repository at this point in the history
…ies (#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
  • Loading branch information
mikaelkolkinn authored and jquense committed Aug 11, 2019
1 parent 2126580 commit 1081c41
Show file tree
Hide file tree
Showing 4 changed files with 89 additions and 22 deletions.
5 changes: 4 additions & 1 deletion src/object.js
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down Expand Up @@ -167,7 +170,7 @@ inherits(ObjectSchema, MixedSchema, {
return field.validate(value[key], innerOptions);
}

return Promise.resolve(true);
return promise(sync).resolve(true);
});

return runValidations({
Expand Down
17 changes: 17 additions & 0 deletions test/helpers.js
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}
26 changes: 5 additions & 21 deletions test/mixed.js
Original file line number Diff line number Diff line change
@@ -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();
Expand Down
63 changes: 63 additions & 0 deletions test/object.js
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ import {
lazy,
reach,
} from '../src';
import { ensureSync } from './helpers';

describe('Object types', () => {
describe('casting', () => {
Expand Down Expand Up @@ -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({
Expand Down

0 comments on commit 1081c41

Please sign in to comment.