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?