Preferable form validators API

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

#36

Agreed. I don’t like valueExists either :slight_smile:

#37

From the PR, I noticed that you’re storing ‘empty’ values as None, what’s the use case for that?

Usually, the validity of the field would be determined by the characteristics of the field in question. I.e if it’s not a required field, then having an empty string would be considered Valid and the empty string would be the value of the field, not None. This would be the case even if the field was required: its value is still an empty string, it’s just not Valid. (I think the idea of adding Pristine type had a point)

image

This todo was onto something. :smiley:
If we’re really pushing the message with formality, having a form-wide value type doesn’t make much sense to me.

Maybe we should consider handling the field type, not value. I’d love to somehow be able to have bool, int, float, date(?) values. We could discuss the mechanics of getting correct representation from underlying raw string html values.

#38

What I mean is, maybe developer is supposed to have an option(int) field and then decide for himself whether raw empty string coming from html field is Some(0) or None.

#39

Hmm can you point to the place that made you think that? Maybe you confused storing validation result with storing value? With this change Formality fully delegates value handling to a user so it’s up to a user if value is string or option(string) or variant or whatever.

Maybe we should consider handling the field type, not value. I’d love to somehow be able to have bool, int, float, date(?) values.

Yes! Initially, I tried to bind value type to a field type, it makes a lot of sense. But I faced type issues, AFAIR w/ internal sets & maps (where field is a key) and validators and gave up. Maybe worth to revisit this direction at some point.

Right now you can have those types by defining your value type in form config, e.g.:

type value =
| String(string)
| Int(int)
| Float(float)
| Date(Js.Date.t);

And handle state updates and rendering of these types. The reason why it’s not in the core is that lots of forms are just about strings and forcing variant means give up ease of use & apply perf penalty to all users by default. Also, variant way is really a rabbit hole, b/c for every new use case we will have to add new constructor to the type. But by abstracting this type away, we let users decide how they want to shape their state. Simple strings will remain simple strings.