Preferable form validators API


#1

Hey, me and @rauan were discussing today should validators in re-formality be wrapped in abstractions like Min, Max, Email, Required etc.

Right now validator is implemented as simple function which should return validation result. E.g.

validate: (value, _state) => 
  switch value {
  | "" => {
      valid: false,
      message: Some("Uh oh, it's required!"),
      meta: None
    }
  | _ when String.length(value) < minLength => {
      valid: false,
      message: Some("Too short"),
      meta: None
    }
  | _ => {
      valid: true,
      message: Some("Nice!"),
      meta: None
    }
  }

To provide some default validators I can make validate a list. Something like this:

validate: [
  Required({ message: Some("Required field"), meta: None }),
  Min(7, { message: Some("Must be at least 7"), meta: None }),
  Custom((value, _state) => 
    switch value {
    | "invalid" => {
        valid: false,
        message: Some("That's invalid!"),
        meta: None
      }
    | _ => {
        valid: true,
        message: Some("Nice!"),
        meta: None
      }
    })
]

The question is: should I? :slight_smile: Do default validators worth one more abstraction?

The main reason why I haven’t implemented it yet is b/c every time I need some more or less complex validation like email or credit card, I always end up using something custom but not standart regexes. Email: b/c it must use exactly the same regex as my server, credit card: b/c I use vendor’s helpers (e.g. Stripe’s). And Required or Min is so simple that I’m not sure if it’s worth it.

But maybe I’m wrong and people actively use those?


#2

Fwiw, I think it’s the right attitude (and a growing one within our community; which I personally appreciate!) to question the worth of adding one more abstraction before doing so!

I got lazy and read the snippets first. The first one’s definitely lighter, more composable, more agnostic, faster (since I imagine the latter calls the former under the hood) and can be understood in one reading without looking elsewhere by a colleague. I hesitate adding an abstraction when its main benefit is mostly to make things read more like English.

Btw, does the valid state need a message? If not, it should be probably be Valid(meta) | Invalid(message, meta) instead?


#3

:100: agree

I don’t think I ever used message on success in the real world apps, only icons, so agreed on this. I spent some time bikeshedding this API and ended up w/ simple record b/c pattern matching against it is really flexible. For example, I can either:

/* use single branch for valid/invalid cases if markup is almost the same */
switch (MyForm.Email |> form.result) {
| Some({ valid }) => <div className=(valid ? "success" : "failure") />
}

/* or easily handle these cases in separate branches */
switch (MyForm.Email |> form.result) {
| Some({ valid: true }) => ...
| Some({ valid: false }) => ...
}

In case of Valid/Invalid it seems first case is not that smooth:

switch (MyForm.Email |> form.result) {
| Valid
| Invalid => <div className=(/* oops, I need one more switch here or handle it in separate branches */) />
}

#4

That’s true; that inconvenience can be easily fixed. But allow me to yak shave a little: everything should be secondary to removing conditional dependencies between record fields.

One really harmful pattern I see in codebases is when js objects/reason records, whose fields’ values are usually assumed to be independent of each other, actually aren’t. For example:

{
  id: Some(foo),
  name: "bar",
  age: 15,
  tempId: None
}

where there’s just no change for a coworker to understand that e.g. id being Some is tied to tempId being None, and that there’s never gonna be a combination of id: None, tempId: None. You end up with:
(1 + (number of possibilities of foo)) * (1 + (number of possibilities of tempId's inner type)))
combinations of results instead of the actual
number of possibilities of foo + number of possibilities of tempId's inner type.
No wonder we get so many invalid state errors! (The record should instead be {id: bla} where type bla = PermanentId(foo) | TempId(bar)).

This is even worse in JS, where the existence of the field itself can also depend on the value of another field. Add some field-related introspection & manipulation (add/remove/spread that changes fields) and things get out of control pretty quickly.

We’ve always been informally codifying arbitrary if-elses in our data structures; variants give you real if-else as data. Let’s use them.


#5

I totally understand what you’re saying and I enjoy variants b/c I can turn my JS’ish state:

{
  status: 'fetching' | 'ready' | 'error',
  data: Data | null,
  error: Error | null,
}

into beautiful variant:

switch status {
| Fetching => ...
| Ready(data) => ...
| Error(error) => ...
}

I just haven’t thought about message as something strictly impossible on success.

I’m going to do some more bikeshedding in the next couple of days on this :+1:

@thangngoc89 @rauan Does it work for your use cases?


#6

That looks like it’s designed for async validation only. Because I don’t use async validation so often.


#7

@thangngoc89 Sorry for confusion, this status example is totally unrelated to what I’m asking :slight_smile:

I was asking about suggested by @chenglou change: current record to variant:

/* now */
{
  valid: bool,
  message: option('message),
  meta: option(string),
}

/* suggested */
Valid(option(meta)) | Invalid(option(message), option(meta))

Note no message on Valid state.


#8

I can’t think of a case where I need message in Valid case. But who know?


#9

I support this change wholeheartedly, more so that’s exactly what I had in mind.
I believe what @chenglou mentioned about combinations resonates well with ocaml’s practice of making illegal states unrepresentable.

That being said, I still think having built-in validators gives more value than it brings complexity. People will likely create their shortcuts, anyway. I can’t see myself copy pasting validators for every form I have, especially the basic, tedious, but absolutely necessary ones.

I realise that current explicit validators are self-explaining, but they make forms so painfully verbose. From the UX/DX standpoint, making people write functions for number range or string length and regexes for emails and dates is a big turn off.

What I see less needed, however, is the meta field. I can’t think of any essential use case for it other than password strength.

As for Valid message, I’d consider adding it to the form’s state instead (to possibly consume it from the response).


#10

If you wanna go down this road, then I believe what you want is a monadic Result pipeline, right? (Though please don’t throw monadic infix operators in newcomers face!)


#11

That’s what I’m vaguely imagining. (minus the infix part :smile:)

I maybe am short-sighted, but I see value in polishing the form state to make it seamless to work with fields, their states and changes.

It’s probably a topic for a different thread, right now I’m lacking the experience to understand what might actually work.


#12

Credit card type, phone number’s country etc etc

I’m not sure I fully understand your idea, but I have this TODO to add ability to pass error messages from response.


#13

I was wondering how could we make validation more composable, chainable.
Instead of a list of strategies and validators, monadic Result pipeline would be a great experiment (also, idiomatic). Cc: @alexfedoseev


#14
type fieldStatus =
  | Valid
  | Invalid(string) // Invalid(list(string)) ?

type formStatus = 
  | Submitting
  | Ready(`data, option(string))
  | Error(list(`field, fieldStatus), option(string))

I had something like this in mind. ^

Oh I see. Imo, that’s completely out of the validation scope. (it’s a value derived from the current state, not the validation status)


#15

Now we’re talking :smile:

This is pretty much how it’s gonna be except Invalid(option('message)) b/c there can be no massage at all + message might be i18n object instead of string. And this data is stored inside formality container as a Map w/ field as a key and option(fieldStatus) as a value.

I think we can leave list out of our scope as message type is defined by user, for us it’s abstraction: what we receive is what we store and return. We just pipe it to UI in the right time. So user might define type message = list(string);. Does it makes sense?

That prolly also ok. Need to play around w/ it a bit but after Valid/Invalid change.

BTW what is option(string)?

Makes sense! Moving meta to user land also makes it sound as it’s possible to use variants instead of strings.


#16

I should have wrote

type fieldStatus =
  | Valid
  | Invalid(list(`message))

I was thinking about the case where we could pass our field through all the validators without ā€˜failing fast’. (just a thought, not sure this will fly with Result monad)

Invalid(["Min length should be 6", "Incorrect format"])

It’s a message.

Ready(data, Some("Document has been successfully updated"))
Error(error_map, Some("We couldn't save your blogpost, please try again later"))

#17

I mean you already can do this if you define your type as list.

type message = list(string);
...
valudate: (value, _state) => {
  /* fold results in a list... */
  Invalid(Some(["Min length should be 6", "Incorrect format"]))
}

Otherwise you force all users handle list in the render which might be overhead (perf & code) if user is fine w/ just a single message.

just a thought, not sure this will fly with Result monad

Me either b/c I have zero experience using monads & I’m in the bubble of current API, so show me the code :sweat_smile:


#18

This was awesome change! Thanks @chenglou @rauan

Feel free to review: https://github.com/alexfedoseev/re-formality/pull/8


#19

Just wanted to say: this thread was really interesting to read through! Great thoughts on all sides. I like the change to the validation result type and agree it feels better as a variant than an object.

Regarding the original question - I tend to agree that ā€œdefaultā€ validations often don’t actually meet my specific needs for whatever reason. Aside from ā€œrequiredā€ they tend to have kind of specific requirements depending on the exact use case. That said, I love the idea of the validate function taking a list of validators, or somehow making validations able to be composed. That way I can define my own ā€œdefaultsā€ for things like required, min, max, whatever and reuse them throughout my app by combining them as needed. You could even provide some example code for common validations - that way beginners could borrow those implementations without too much effort, but you’re not stuck maintaining them as a part of the package itself, responding to requests for different use cases, etc.

Thanks for putting the thought into all this. I’m really enjoying working with re-formality!


#20

@rauan I’m going to work on form status next. I want to discuss if we really need these messages (option(string)):

| Ready(`data, option(string))
| Error(list(`field, fieldStatus), option(string))

When server responds w/ success client knows that result is achieved (e.g. Document has been successfully updated) and Ready('data) status should be enough to derive this message inside render w/o additional argument in constructor. Same about Error: all details are stored in list('field, fieldStatus) and the fact that We couldn't save your blogpost can be easily derived from Error(list('field, fieldStatus)) itself in render. Or am I missing some use cases?