shakacode / re-formality

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

form.updateField [value type is automatically derived from the input type.] #86

Closed Prateek13727 closed 3 years ago

Prateek13727 commented 4 years ago

Hey Fellas,

First of all thank-you for creating a wonderful way to handle forms in Reason-React. The ppx version is even better :)

The Issue: The value type is automatically derived from the input type. This becomes an issue when my input type in the form state is something other than a string say integer

type input = {amount: int)};

 form.updateAmount(
              (input, value) => {...input, amount: value},
              event->ReactEvent.Form.target##value,
            )

The Use-case 1.) We type abc in the input field 2.) now this is tagged as an int and we can't validate this. since it is actually a string but reason is seeing it as int

Since the value comes from event->ReactEvent.Form.target##value,, will it make sense to keep it as a string and let the developer handle it the way he/she wants ?

Please find the complete code below. Also, you can see a working version in the repo here too.

Please let me know in case of any clarifications needed / or if we are missing something here.

Cheers Prateek

module DummyForm = [%form
  type input = {amount: option(int)};
  let validators = {
    amount: {
      strategy: OnSubmit,
      validate: ({amount}) => {
        switch (amount) {
        | Some(amount) =>
          if (amount < 10) {
            Belt.Result.Error({js|Donations above the value of ₹ 10 are accepted|js});
          } else if (amount > 10000000) {
            Belt.Result.Error({js|Donations below the value of ₹ 1,00,00,000 are accepted|js});
          } else {
            Belt.Result.Ok(Some(amount));
          }
        | None => Belt.Result.Error("Please enter a donation amount")
        };
      },
    },
  }
];

[@react.component]
let make = (~initiate_payment: int => unit) => {
  let form =
    DummyForm.useForm(
      ~initialInput=DummyForm.{amount: None},
      ~onSubmit=(
                  output: DummyForm.input,
                  submissionCbs:
                    Formality.submissionCallbacks(
                      DummyForm.input,
                      DummyForm.submissionError,
                    ),
                ) => {
        switch (output.amount) {
        | Some(value) => {
          initiate_payment(value)
        }
        | _ => ()
        };
        submissionCbs.reset();
      },
    );

  <form onSubmit={form.submit->FormalityCompat.Dom.preventDefault}>
    <div>
        <label htmlFor="amount">
          "Donation Amount"->React.string
        </label>
        <input
          id="amount"
          type_="text"
          value={switch(form.input.amount){
            | Some(amount) => string_of_int(amount)
            | None => ""
          }}
          disabled={form.submitting}
          onChange={event =>
            form.updateAmount(
              (input, value) => {...input, amount: value},
              event->ReactEvent.Form.target##value,
            )
          }
        />
    </div>
    <hr />
    <button>
      <span> {React.string("Donate")} </span>
    </button>
  </form>;
};
alex35mil commented 4 years ago

Hi @Prateek13727, the root of the issue is that event->ReactEvent.Form.target##value is unsafe. The result of event->ReactEvent.Form.target is Js.t({..})—opened object—and compiler happily infers types of contents by its usage. So if you pass target##value to a function that expects int, the compiler would infer its type as int. In reality though, the type of target##value is string and passing it to a function that expects int, doesn't magically convert string to int.

I can suggest to create an app level module with a typesafe interface:

// ReactForm.re
type target = {
  value: string,
  checked: bool,
};

external target: Js.t({..}) => target = "%identity";
let target = (event): target => event->ReactEvent.Form.target->target;

let value = event => event->target.value;
let checked = event => event->target.checked;

// ReactForm.rei
let value: ReactEvent.Form.t => string;
let checked: ReactEvent.Form.t => bool;

And use it to get data from DOM:

event => event->ReactForm.value->Int.fromString

This is what I personally do.

Prateek13727 commented 4 years ago

Hey @alexfedoseev

Thanks for your reply and the solution. Using %idenity is a good workaround for type casting Js.t({}) to ReasonML record.

Let me know if its a good idea to keep this issue open for a while for discussions?

Thanks Prateek

alex35mil commented 4 years ago

@Prateek13727 No problem. Prolly makes sense to create an issue in reason-react repo as well.

alex35mil commented 4 years ago

As a side note, in one of the early versions of reason-react event target was properly typed as Dom.eventTarget but IIRC it was changed b/c it was quite non-trivial getting value out of it using Webapi.

Prateek13727 commented 4 years ago

@alexfedoseev will create the issue in reason-react as well. got it! thanks for the info.