Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

refactor composeValidators to stay sync when possible #6186

Merged
merged 1 commit into from
Apr 19, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
131 changes: 129 additions & 2 deletions packages/ra-core/src/form/validate.spec.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,3 @@
import expect from 'expect';

import {
required,
minLength,
Expand All @@ -11,6 +9,7 @@ import {
email,
choices,
composeValidators,
combine2Validators,
} from './validate';

describe('Validators', () => {
Expand All @@ -28,6 +27,134 @@ describe('Validators', () => {
);
};

describe('combine2Validators', () => {
it('should create a new validator that always return the result directly when both validator are synchronous', () => {
const includesFoo = value =>
value.match(/foo/) ? null : 'value must include foo';
const includesBar = value =>
value.match(/bar/) ? null : 'value must include bar';

const combinedValidator = combine2Validators(
includesFoo,
includesBar
);
expect(combinedValidator('foobar', null, null)).toBe(null);
expect(combinedValidator('bar', null, null)).toBe(
'value must include foo'
);
expect(combinedValidator('foo', null, null)).toBe(
'value must include bar'
);
expect(combinedValidator('', null, null)).toBe(
'value must include foo'
);
});

it('should create a new validator that always return a promise when both validator are asynchronous', async () => {
const includesFoo = value =>
Promise.resolve(
value.match(/foo/) ? null : 'value must include foo'
);
const includesBar = value =>
Promise.resolve(
value.match(/bar/) ? null : 'value must include bar'
);

const combinedValidator = combine2Validators(
includesFoo,
includesBar
);
const validPromise = combinedValidator('foobar', null, null);
expect(validPromise.then).toBeDefined();
expect(await validPromise).toBe(null);
const missingFooPromise = combinedValidator('bar', null, null);
expect(missingFooPromise.then).toBeDefined();
expect(await missingFooPromise).toBe('value must include foo');

const missingBarPromise = combinedValidator('foo', null, null);
expect(missingBarPromise.then).toBeDefined();
expect(await missingBarPromise).toBe('value must include bar');

const invalidPromise = combinedValidator('', null, null);
expect(invalidPromise.then).toBeDefined();
expect(await invalidPromise).toBe('value must include foo');
});

describe('synchronous validator + asynchronous validator', () => {
const includesFoo = value =>
value.match(/foo/) ? null : 'value must include foo';
const includesBar = value =>
Promise.resolve(
value.match(/bar/) ? null : 'value must include bar'
);
const combinedValidator = combine2Validators(
includesFoo,
includesBar
);

it('should return valid result inside a promise when both validators pass', async () => {
const promise = combinedValidator('foobar', null, null);
expect(promise.then).toBeDefined();
expect(await promise).toBe(null);
});

it('should return invalid result directly when both validators fail', () => {
expect(combinedValidator('', null, null)).toBe(
'value must include foo'
);
});

it('should return invalid result directly when first validator fail', () => {
expect(combinedValidator('bar', null, null)).toBe(
'value must include foo'
);
});

it('should return invalid result inside a promise when second validator fail', async () => {
const promise = combinedValidator('foo', null, null);
expect(promise.then).toBeDefined();
expect(await promise).toBe('value must include bar');
});
});

describe('asynchronous validator + synchronous validator', () => {
const includesFoo = value =>
Promise.resolve(
value.match(/foo/) ? null : 'value must include foo'
);
const includesBar = value =>
value.match(/bar/) ? null : 'value must include bar';
const combinedValidator = combine2Validators(
includesFoo,
includesBar
);

it('should return valid result inside a promise when both validators pass', async () => {
const promise = combinedValidator('foobar', null, null);
expect(promise.then).toBeDefined();
expect(await promise).toBe(null);
});

it('should return valid result inside a promise when both validators fail', async () => {
const promise = combinedValidator('', null, null);
expect(promise.then).toBeDefined();
expect(await promise).toBe('value must include foo');
});

it('should return invalid result in a promise when first validator fail', async () => {
const promise = combinedValidator('bar', null, null);
expect(promise.then).toBeDefined();
expect(await promise).toBe('value must include foo');
});

it('should return invalid result inside a promise when second validator fail', async () => {
const promise = combinedValidator('foo', null, null);
expect(promise.then).toBeDefined();
expect(await promise).toBe('value must include bar');
});
});
});

describe('composeValidators', () => {
const asyncSuccessfullValidator = async =>
new Promise(resolve => resolve(undefined));
Expand Down
51 changes: 27 additions & 24 deletions packages/ra-core/src/form/validate.ts
Original file line number Diff line number Diff line change
Expand Up @@ -76,35 +76,38 @@ const memoize: Memoize = (fn: any) =>

const isFunction = value => typeof value === 'function';

export const combine2Validators = (
validator1: Validator,
validator2: Validator
): Validator => {
return (value, values, meta) => {
const result1 = validator1(value, values, meta);
if (!result1) {
return validator2(value, values, meta);
}
if (
typeof result1 === 'string' ||
isValidationErrorMessageWithArgs(result1)
) {
return result1;
}

return result1.then(resolvedResult1 => {
if (!resolvedResult1) {
return validator2(value, values, meta);
}
return resolvedResult1;
});
};
};

// Compose multiple validators into a single one for use with final-form
export const composeValidators = (...validators) => async (
value,
values,
meta
) => {
export const composeValidators = (...validators) => {
const allValidators = (Array.isArray(validators[0])
? validators[0]
: validators
).filter(isFunction) as Validator[];

for (const validator of allValidators) {
const errorPromise = validator(value, values, meta);

if (errorPromise) {
if (typeof errorPromise == 'string') {
return errorPromise;
}
if (isValidationErrorMessageWithArgs(errorPromise)) {
return errorPromise;
}

const error = await errorPromise;

if (error) {
return error;
}
}
}
return allValidators.reduce(combine2Validators, () => null);
};

// Compose multiple validators into a single one for use with final-form
Expand Down