shakacode / re-formality

Form validation tool for reason-react
https://re-formality.now.sh
MIT License
244 stars 35 forks source link

API overhaul #35

Closed alex35mil closed 5 years ago

alex35mil commented 5 years ago

Major

Minor

Details

Fast pipe API

Change all functions to data-first style.

Switch to Belt data structures

Right now library uses OCaml std Map & Set. In context of bucklescript Belt is more appropriate choice. This change is also related to fast-pipe change.

Handle value using GADTs

Ok, this is the most interesting and controversial part and I very appreciate help/feedback on this.

In current implementation if you need your value type to be something else rather than string it's possible to define it as variant: see example. But by doing this there are a number of cases when it's possible to get into invalid state at runtime: e.g. here. Ideally, value type should be tied to field constructor but since these constructors are used as id's internally we can't add payload to it. From what I read, GADT seems like a way to go here and I started poking around w/ it. First step was to convert config to the new shape:

```reason module LoginForm = { type field(_) = | Email: field(string) | Password: field(string) | Remember: field(bool); type state = { email: string, password: string, remember: bool, }; type message = string; let get: type value. (field(value), state) => value = (field, state) => switch (field) { | Email => state.email | Password => state.password | Remember => state.remember }; let set: type value. ((field(value), value), state) => state = ((field, value), state) => switch (field, value) { | (Email, value) => {...state, email: value} | (Password, value) => {...state, password: value} | (Remember, value) => {...state, remember: value} }; let valueEmpty: type value. (field(value), value) => bool = (field, value) => switch (field, value) { | (Email, value) when value == "" => true | (Password, value) when value == "" => true | (Email, _) => false | (Password, _) => false | (Remember, _) => false }; let validators: type value. list(FormalityNext.validator(field(value), value, state, message)) = FormalityNext.[ { field: Email, strategy: Strategy.OnFirstSuccessOrFirstBlur, dependents: None, validate: (value, _state) => { let emailRegex = [%bs.re {|/.*@.*\..+/|}]; switch (value) { | "" => Invalid("Email is required") | _ as value when !value->Js.Re.test(emailRegex) => Invalid("Email is invalid") | _ => Valid }; }, }, { field: Password, strategy: Strategy.OnFirstBlur, dependents: None, validate: (value, _state) => switch (value) { | "" => Invalid("Password is required") | _ => Valid }, }, ]; }; ```

As you can see GADT requires some additional annotations. IMO not a big deal but it's not the primary issue right now.

The snippet above doesn't compile. The show stopper is that I can't find a way to pack GADT constructors into iterable data structure (like list or Map). W/o it I can't move further since it's critical to have the ability to store various field data inside something searchable & iterable.

Simplified version that illustrates the problem:

type t(_) =
  | S: t(string)
  | I: t(int);

let items: type a. list((t(a), a => bool)) = [
  (S, x => x == ""),
  (I, x => x == 0),
];

/*
  This has type:
    string t
  But somewhere wanted:
    a t

  The incompatible parts:
    string
    vs
    a
*/

Pack validators into list instead of Map

Simplifies public API:

Before: ```reason module Validators = Formality.MakeValidators({ type t = field; }); type validators = Validators.t(Formality.validator(field, value, state, message)); let validators = Formality.( Validators.empty |> Validators.add( Email, { strategy: Strategy.OnFirstSuccessOrFirstBlur, dependents: None, validate: (value, _) => { let emailRegex = [%bs.re {|/.*@.*\..+/|}]; switch (value) { | String("") => Invalid("Email is required") | String(value) when !(emailRegex |> Js.Re.test(value)) => Invalid("Email is invalid") | String(_) => Valid | _ => failwith("Email validator received bad input") /* sadly, as well */ }; }, }, ) |> Validators.add( Password, { strategy: Strategy.OnFirstBlur, dependents: None, validate: (value, _) => switch (value) { | String("") => Invalid("Password is required") | String(_) => Valid | _ => failwith("Password validator received bad input") /* sadly, as well */ }, }, ) ); ``` After: ```reason let validators = Formality.[ { field: Email, strategy: Strategy.OnFirstSuccessOrFirstBlur, dependents: None, validate: (value, _state) => { let emailRegex = [%bs.re {|/.*@.*\..+/|}]; switch (value) { | "" => Invalid("Email is required") | _ as value when !value->Js.Re.test(emailRegex) => Invalid("Email is invalid") | _ => Valid }; }, }, { field: Password, strategy: Strategy.OnFirstBlur, dependents: None, validate: (value, _state) => switch (value) { | "" => Invalid("Password is required") | _ => Valid }, }, ]; ```

Use re-debouncer

Originally, re-debouncer was extracted from here. Makes sense to use this package instead of current inlined code.

alex35mil commented 5 years ago

First 3 major items are almost complete. Switch to Belt and to list of validators removed lots of unnecessary layers, good stuff.

baransu commented 5 years ago

Thank you for those improvements! 🎉

Few things I've notices when using this library.

Validation

I created abstraction over validation to remove API surface as much as possible. This is the example validation I have:

Validation.[
  validate(Email, state => state.email, [required, email]),
  validate(
    Code,
    state => (state.mode, state.code),
    [
      fun
      | (Mode.OneForAll, code) when String.length(code) > 0 => Formality.Valid
      | (Mode.ManyForAll, _) => Formality.Valid
      | _ => Formality.Invalid(I18n.required),
    ],
  ),
]

I probably misuse the API because I don't care about the value and about getter in form in general and use custom getter. For me it's easier to combine small validators like required or email which are just string => validationResult. It allows omitting default values for example. Here is my implementation:

let validate =
    (
      ~dependents=?,
      ~strategy=Formality.Strategy.OnFirstSuccessOrFirstBlur,
      field,
      getter: 'state => 'value,
      rules: list(rule('value)),
    )
    : ('field, Formality.validator('field, 'v, 'state, 'message)) => (
  field,
  {
    strategy,
    dependents,
    validate: (_, state) => {
      let value = getter(state);
      Belt.List.reduce(rules, Formality.Valid, (state, rule) =>
        switch (state) {
        | Formality.Valid => rule(value)
        | _ => state
        }
      );
    },
  },
);

Because I only care about only first error I don't use other rules if the field is already invalid.

Value

I've tried to use polymorphic variants. It gives you the ability to create basic value for simple use cases and extend that value when you need it. Because polymorphic variants don't require to have type in scope it's really easy to use but gives you weaker type guarantees and can cause type errors to show in weird places unrelated to the place where the error really is. I have basicValue

type basicValue = [
  | `String(string)
  | `Int(int)
  | `Float(float)
  | `Bool(bool)
  | `ArrayOfString(array(string), option(arrayAction))

and arrayAction = [ | `Set(int) | `Update(int) | `Push | `Remove(int)];

and set of array helpers I use. I have small module that gives me functionality to handle arrays which works on arrayAction. If I don't need more complex value I just use basicValue but in case of something more complex I can extend basicValue like that:

type value = [
  basicValue
  | `SomethingComplex(Complex.t)
];

Right now I have it in 2 mid size forms and it works quite ok.

I have't try GADT's ever nor I have knowledge about them so it's hard for me to give any feedback in that matter