Preferable form validators API


#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?


#21

Form-level status messages make sense when backend is the one doing all the heavy lifting (not just doing puny CRUD).

So if we assume that an api call response has data and message (any meta, really), it’s a matter of choice where we want to have it. (same way, it’s a matter of choice how we want to handle submission: have an url in the form’s type and make assumptions, or completely leave it out)

Personally, I found general status messages to be better placed in the form scope.
It could easily be moved into user land, I don’t have a clear opinion on that yet.


#22

@alexfedoseev I’ve look at the new API in 0.4.0 and I think the API could be cleaner.

Instead of a switch like this

| Some(Invalid('a))
| Some(Valid)
| None

We can change it to

| Valid
| Invalid('a)
| Pristine

My proposed API would also my current usecase, where I want some field to be optional, I’m currently return Valid for empty string but it feels wrong. I need to return Pristine


#23

@thangngoc89 TBH I’m not sure about it. In my mental model there’s clear separation of 2 concerns:

— Validators are responsible for validation of value. Basically, validator answers the question “Is this value valid?” but not “Should I show result in UI?” or something else. Thus this abstraction is not coupled to UI and can be reused whenever you want to check validity of attribute.
— Formality is responsible for managing when UI receives feedback. On each update it answers the question “Should I show validation result right now?”.

With addition of Pristine tag I feel like we introduce the responsibility that validator shouldn’t care about. Also, I’m concerned that it might be confusing for users who not aware of the optional field use case and it’s not clear when Pristine should be returned.

But the optional field case you mentioned is absolutely valid and I completely forgot to handle it. AFAICT empty string is the only case when success result shouldn’t be ever emitted. How I solve it in JS implementation: internally I just special case it and don’t emit results unless value exists. So in Formality I can special case it as well: e.g. you can still return Valid on empty string but formality will still emit None.

Does it make sense?


#24

This is totally make senses. But your proposed approach doesn’t feel right. Currently, required fields will show an error after blur. If you return None for empty string then there won’t be any errors.


#25

Yes, of course, Invalid result will be emitted, I mean something like

| Valid when value === "" => None
| Valid => Some(Valid)
| Invalid => Some(Invalid(message))

#26

I see. This is an acceptable solution for me. But I still feel like I have to keep a mental model about this in mind. :confused: not so obvious


#27

Just to make clear :slight_smile: This snippet is internal implementation, there won’t be any changes in your app:

validate: (value, _state) =>
  switch (value) {
  | "" => Valid
  | ...
  }

...

<div>
  (
    switch(MyField |> form.result) {
    | None => /* this is the case if value is "" */
    | Some(Valid) => /* it won't get here if value is "", only if value is not empty */
    }
  )
</div>

#28

Ah. Thank you. This makes sense now


#29

@thangngoc89 Please try 0.4.1.


#30

Wow. That was fast !. I have created nice success indicator with the latest change

image


#31

@rauan brought up one more topic for discussion today: where to store form state — inside or outside.

JS version of this lib uses the latter approach: it requires state to be passed as prop + it needs change handlers to update external state. This was made for one reason: in our app form state might be in vanilla react components as well as in the redux state. So validation container is not the owner of the state but just a layer that validates passed data. However, this approach has its downsides and the main one is that updates of the data & validation state are decoupled and happen in different async transactions.

Since in Reason we have only one primary way to manage the state of the form I made container the owner of the state. So update of the form state and validation state happens within single transaction. If some components need form data initial plan was to just lift container up and pass this data to all interested children.

Any thoughts if current approach is fine or it makes sense to think about externalizing form state?


#32

@rauan @thangngoc89 @kgoggin I hope value problem is solved by making it a user-defined type.

PR: https://github.com/alexfedoseev/re-formality/pull/12

With this change user must define:

  1. type value;
  2. let valueExists: value => bool;
    (I need this to avoid emitting success on empty strings or whatever considered an empty value)

:+1: / :-1: ?


#33

I don’t see why valueExists is needed ? I’m think about a more generic API like using polymorphic variant because you don’t need to define all possible types before hands.


#34

Because by making value an abstract type I can’t judge anymore if value is empty or not (we don’t want emitting Some(Valid) result on empty string).


#35

Ah. Alright, if that’s the intention then it’s not a very good name. maybe valueEmpty ?