Use a slightly more explicit api surface to define validations without the need for explicit lists

[gaku-sei]

From now on we can define a validation this way:

let required = (
  #name(#required),
  ({Validator.Args.label: label, value}) =>
    switch value {
    | "" => #error(#error(("required", label)))
    | _ => #ok(value)
    },
)

Some changes:

• Under the hood, no more Lists, at all
• No more a record, since we have #name it's probably explicit enough, and it prevents this: names: #name(#required) which is a bit weird
• Might have type issues if you use #names([...]) in the code, but it's clearly not a use case

It's a simple alternative to #22 that should be more lightweight and not as breaking 👍

Closes #22
Closes #6

[infothien]

could be shorten to just do flatMap(Validations.getNames) right Kevin-san

[gaku-sei]

absolutely right I dropped the function call but didn't update the code, let me fix that 👍

[infothien]

so we will use tuple in the end and not convert to record @gaku-sei -san?

[infothien]

ah I see just read the description ! sorry :P

[gaku-sei]

Only for this reason:

No more a record, since we have #name it's probably explicit enough, and it prevents this: names: #name(#required) which is a bit weird

It's really weird to define a validation with names: name 😕

But I don't mind to bring them back ^^

[infothien]

since you are using kind like above, I think it is clear enough :D. And this one looks simpler than 22 so probably good with it

[infothien]

Only one concern since we introduced #names(array<'label>), but dont see that there is a case to use it.
The best is if you could add example for that as well ! So that we know everything implemented here is actually usecases

[gaku-sei]

Only one concern since we introduced #names(array<'label>), but dont see that there is a case to use it.

To be honest noone should use it apart from the internal code, but there is no way to hide it to the users... That's why it's not documented at all. The documentation would mention it but would also discourage its use.

[infothien]

I see :( uhm but not so cool if we show sth but user should not use right. But it is a minor thing so I dont think it will be a problem

[gaku-sei]

I see :( uhm but not so cool if we show sth but user should not use right. But it is a minor thing so I dont think it will be a problem
So far we don't actually, and the example doesn't really show anything ^^

To be fully honest, we can document it, and I can imagine some use cases for it (while I think composing validations would solve all the use cases, but anyway), the only thing is that it involves using the functor pattern from #22, which I don't want to enforce downstream 😞

[gaku-sei]

Wait, let me try something else ^^

[gaku-sei]

Nope, i tried this:

let required = Description.make(~name=(#required :> Label.t), ({label, value}) =>
  switch value {
  | "" => #error(#error(("required", label)))
  | _ => #ok(value)
  }
)

But it fails with the same error as for #22 😞

We might have to go for the functor pattern at one point I think 😞

[infothien]

:( T_T . I still dont know why it is now throwing that error #22 but not before?

[gaku-sei]

Because of the array being mutable 😞

I found this:

let required = values =>
  Description.make(~values, ~name=#required, ({label, value}) =>
    switch value {
    | "" => #error(#error(("required", label)))
    | _ => #ok(value)
    }
  )

It basically explicitely give the type of Values, and it's not too verbose. On call site though it looks like this required(module(Values)) 😞

[infothien]

look like if you did it like that, the email validation will be the same right. I meant we have to do email(module(Values))? Kevin-san
if so @@ :( then not that great and Functor pattern will be easier

[gaku-sei]

I agree... Functor is just more heavy as you need to recreate an object with validations each time you define a form, the values above will be dropped at compile time. Notice that some validations, like the equals in the example are already aware of the type of values and should work without the values

[gaku-sei]

Might go for the simplest/lightest solution for now, and we'll see how we can improve it from that 😄

The functor pattern is not a bad solution, just a solution I'd like to postpone for now if you don't mind.

So basically I'll keep the pr as is for now, and update #22 to introduce only the functor pattern.

What do you think @infothien @Hach7ko ? Would this pr be ready to merge then?

[infothien]

Thank you @gaku-sei -san. For me, it is okie to be merged !

[gaku-sei]

This might change again in the future actually ^^