Enriching validators responses about deep-waters HOT 14 OPEN

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 expected, actual, and value.

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.

from deep-waters.

@antonioru

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' }

Joi also make transformations and support default values so maybe if we wish to support that it make sense.

if you have and error you don't have value and vice versa.

@alonronin
In the future perhaps we could introduce few sanitising utilities but for moment I prefer deep-waters to remains a validation-only library as I find it a more attainable goal.

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.
For the instance, any use case would be something like:

// 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.
The final goal is not only to use the built-in validators but to encourage the creation of custom ones.

That said, I'll start this very weekend in developing the requested features and in addition I'll add the following one:

• memoization // validators should cache the response to boost performaces
• createValidator // an easy-to-use thunk to create custom validators accordingly to our proposed definition

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

from deep-waters.

Hello @alonronin
I think you're right and yes, I received the same feedback from reddit as well!

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!
We agree that a boolean does not inform on which part of the validation fails.

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 :)

from deep-waters.

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.

from deep-waters.

@alonronin

that looks outstanding!

I'll think about it this evening and I shall come with a new proposal

from deep-waters.

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:

• Sometimes you just need to check if the validation is complete (here it comes the valid property)
• Sometimes you don't need to know which part of the validation failed, you just need the errors (here it comes the errors array)
• The detail property reports which validation failed and why (it can also be a nested object of other compositions). This approach assumes that each validator has a function's name, if the provided validator has no function name, we can show anonymous as an array

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?

from deep-waters.

@antonioru

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' }

Joi also make transformations and support default values so maybe if we wish to support that it make sense.

if you have and error you don't have value and vice versa.

from deep-waters.

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 🍻

from deep-waters.

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
thanks for your comment and I apologies it took me 3 days to reply,
I'm currently studying the best way to refactor the library and return the response object we've defined without writing everything again from scratch.
The best solution I've found so far is to apply a Kleisli composition using monads to represent the error/response.

I'll let you know soon, and again: thank you so much for showing interest in this project

from deep-waters.

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.

from deep-waters.

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 withError.
I'll be able to present the whole refactory approach and the whole idea under this very same post in few days, I apologise it is taking so long. I'm sorry.
Until then I have to beg you to wait so I'll have a clear outlook on how precisely you or anyone else can help me!
I hope you don't mind.

I'll keep you all posted, thanks

from deep-waters.

@antonioru

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!

from deep-waters.

Hello guys,

I've finally managed to properly find a solution to the problem!
(Actually two solutions, both are drafts and I'm open to suggestions.)

My first approach has been to create a new thunk function called withError.

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".
It also accept a validatorName that will be "hidden" within the response object prototype (it will be used later to compose the detail property of the composition response), if not defined it will try to guess the validator's name from the fn.name property.
It could be used as the following and lead to an quick and easy refactory:

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.
It uses a monad object called ResponseMonad to do so.

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:

• the details property is utterly expansive to be calculated as it needs to merge a ton of objects
• the details property it's not so deterministic as it should be and there's no way to be sure that one of the composed validators has somehow "hacked" its outcome... I mean it can be easily broken by using withError with anonymous functions or by providing "hacky" names:

// 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 detail property but I've been pondering on this matter: what is the real value that the detail property brings to the user?
Of course, it can be useful for debugging purposes... on the other hand the errors array is there for the very same reason and is definitely less verbose.
Initially @alonronin pointed out that the errors array only doesn't make clear where the errors come from... and he's totally right, but I'm not sure you want to... I mean by using a validator you probably want to archive something like this:

and I think it can be archived with the errors array only.
( of course there are probably few exceptions, such as hasShape... but it can be managed )

So I've tried a second solution, I got rid of the details property from the response object and I've kept withError but it does need the validator name and I've changed the compose function.

Here's the branch for this second solution (look at the compose fn and withError)
https://github.com/antonioru/deep-waters/tree/feature/with-error

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!
As I initially started the project I think I might be biased so I'm totally open to your suggestion, if you think I'm making a mistake please say it out loud!

cheers

from deep-waters.

@antonioru it looks interesting, i didn't had time to play with it.
however, i have an idea for an abstraction that can be done by the user and not in the library, so the lib will stay agnostic to how the user uses it.

when i'll have a spare hour i'll write something quick to demonstrate the usage of it.

from deep-waters.