-
Notifications
You must be signed in to change notification settings - Fork 9
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
Enriching validators responses #1
Comments
Hello @alonronin This project has started as a basic exercise of FP, then has developed in something more structured. Now that has been released I think having a better response from the composition of each validator should be the next step! I like the idea of returning error reporting objects and merging them in the composition, something like: isString({ invalid: true }); // -> { valid: boolean, error: Error }
const myShapeValidator = hasShape({ key: isString, num: isNumber });
myShapeValidator({ key: 10, num: 10 }); // -> { key: { valid: boolean, error: Error }, num: { valid: true } } What do you think? About the async validation i would open another topic, definitely a future implementation :) |
it looks great, but i can't understand how you'll define the message for the error. maybe something like: const isUserName = compose(
isString('must be a string'),
minLength(4)('at least 4 characters needed')
)
const validate = hasShape({ userName: isUser, num: isNumber });
try {
validate({ userName: 123 })
} catch (e) {
console.log(e.errors) // { userName: 'must be a string' }
}
try {
validate({ userName: 'abc' })
} catch (e) {
console.log(e.errors) // { userName: 'must be at least 4 characters' }
}
validate({ userName: 'admin' }) // true we could also have sane default for messages for each built-in validator. |
that looks outstanding! I'll think about it this evening and I shall come with a new proposal |
Hello @alonronin I've thought quite a lot about restyling the response from validators and validator compositions and I came up with the following solution, each validator will return an object telling whether the validation is successful and the possible error, something like this: { valid: Boolean, error: null | ErrorString } whilst the composition of more validators will return something like: const myValidator = compose(isString, minLength(4));
myValidator(value);
/* composition result looks like this:
{
valid: Boolean,
errors: ErrorString[],
detail: {
isString: { valid: Boolean, error: ErrorString },
minLength: { valid: Boolean, error: ErrorString }
}
}
*/ The reason for this choice are:
The idea to provide custom errors to each validator as a further argument sounds good to me: const isUserName = compose(
isString('must be a string'),
minLength(4)('at least 4 characters needed')
) The result of this validation should then be the following: const result = isUserName("foo");
/* result is:
{
valid: false,
errors: ['at least 4 characters needed'],
detail: {
isString: { valid: true },
minLength: { valid: false, error: 'at least 4 characters needed' }
}
}
*/ What do you think? |
we can take inspiration from Joi. they return an object like so: schema.validate({ username: 'abc', birth_year: 1994 });
// -> { value: { username: 'abc', birth_year: 1994 } }
schema.validate({});
// -> { value: {}, error: '"username" is required' }
if you have and error you don't have value and vice versa. |
Hey @antonioru really like the project. I'm adding deep-waters to a project right now! About this issue, I like your solution 👍 . Only comment is to add metadata to the error object to better communicate to developers/clients what the problem is. Specifically, I'm thinking E.g. const result = isUserName("foo");
/* result is:
{
valid: false,
errors: ['at least 4 characters needed'],
detail: {
isString: { valid: true },
minLength: {
valid: false,
error: 'at least 4 characters needed',
expected: 'at least 4 characters',
actual: '3 characters',
value: 'foo'
}
}
}
*/ This could be better suited as a feature request to move to another issue. It would be easy to extend the error objects with more data with a general solution like this. |
@alonronin For this reason I don't think reporting the received value in the response message (as suggested by @rayepps as well, btw thanks mate) it's somehow useful. // create validator
const myValidator = compose(myFn1, myFn2, others...);
// usage
const result = myValidator(value); // <-- you have the value right here
console.log(result);
/*
{
valid: false,
errors: ['foo', 'bar'],
detail: {
foo: { valid: false, error: 'bar' },
bar: { valid: false, error: 'foo' }
}
}
now... if you need to access the value, you have it right there before using the validator.
*/ Regarding the idea by @rayepps of introducing the expected and actual prop within the detail sub-objects, it is definitely a good idea, unfortunately it increases the number of parameters and evaluations one have to consider in creating a custom validator. That said, I'll start this very weekend in developing the requested features and in addition I'll add the following one:
I'm very grad of your contribution guys :) And of course, feel free to share any other idea or if you want to join in me I'll be flattered |
Thanks for the quick and detailed replies @antonioru! I'd like to contribute as well. I pulled the project and got familiar but I expect any a change to simply convert boolean returns to objects could get complex so I'm holding off making changes. I'm sure like most creators, you have a backlog of todo items in your head. If you would like help with them just make an issue with info and tag me 🍻 |
Hi @rayepps I'll let you know soon, and again: thank you so much for showing interest in this project |
Hello! I would like to contribute to this project, but i don't know what i could do. May you help me? I'm minded to invert the necessary time, but i need to know how i can help. |
Hello @CarlosPerezMoreno I am flattered, thank you very much! At the moment I am working on a quite-big refectory of the library in order to allow validators to return a proper customisable and composable error message. If you want to contribute (and I shall be glad of it), perhaps, you can help me with the refactory: there will be a big number of validator tests to be updated and some validators shall be wrapped into a higher order function called I'll keep you all posted, thanks |
Okay, got it. Well, i'm beginner in this world; i barely can use JS, and now i'm starting with React. My knowledge is very limited, but i'm excited about learning and contribute. So, we keep in touch! Thanks! |
Hello guys, I've finally managed to properly find a solution to the problem! My first approach has been to create a new thunk function called type Response = {
valid: boolean,
error?: string,
}
withError(fn: Function, error: string, validatorName?: string) -> fn -> Response it takes a validator fn and an error string then returns a new validator (behaving as the provided one) that returns a "response object". const isSomething = (value) => ( ... );
export default withError(isSomething, 'Is not something'); Then I've edited the compose function to merge the responses from all the various validators and create the detail property. So, in a nutshell it will be used this way: const isZero = withError((value) => value === 0, 'Not Zero', 'isZero');
const isSomethingElse = withError(() => ..., 'Foo Error', 'isSomethingElse');
isZero(0); // -> { valid: true };
isZero(2); // -> { valid: false, error: 'Not zero' };
const composedValidator = compose(isZero, isSomethingElse);
composedValidator(2);
/*
* {
* valid: false,
* errors: ['Not zero', 'not something else'],
* details: {
* isZero: { valid: false, error: 'Not zero' },
* isSomethingElse: { valid: false, error: 'Foo Error' },
* }
* }
*/ You can see first draft of this solution here. Also @rayepps provided an alternative implementation that chuffed me to bits, you can see it here #3 (thanks mate, you're the bee's knees!!!) ✌️✌️✌️ Let me know what you think! 😅 BUT.... to be honest I see some problems with the compose function response object:
// hackish name provided for isZero validator that could override isBiggerThan results
const isZero = withError((value) => value === 0, 'Not Zero', 'isBiggerThan');
/* use withError in the composition process without passing the validator name will cause a nonsense response */
const myValidator = compose(isZero, isBiggerThan(3), withError((value) => value < 5, 'err')); These two problems can actually be avoided by imposing a strict set of rules when creating a validator, but this increases the complexity of the whole library and it's something I really want to avoid. So, in the attempt of finding a solution, I've tried to reshape the and I think it can be archived with the So I've tried a second solution, I got rid of the Here's the branch for this second solution (look at the compose fn and withError) As a result I think it's faster, it's less confusing for the users and it's easy to create new validators, which is actually one of the final goals of the library. Please let me know what you think! cheers |
@antonioru it looks interesting, i didn't had time to play with it. when i'll have a spare hour i'll write something quick to demonstrate the usage of it. |
Hi,
Thank you very much for your contribution! it is super awesome!
I'm still missing here some kind of message system, so we can know which validation rule has failed and show the correct message to the user.
Also, if result and pipe will be async we could also have async validation.
Do you have any ideas for these subjects and how to implement it with current state of the project?
Maybe implementing
Future
type? and compose each validator with aMessage
type?The text was updated successfully, but these errors were encountered: