shakacode / re-formality

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

Why can't extract change and blur handler? #30

Closed hulufei closed 5 years ago

hulufei commented 6 years ago

Hi, thanks for creating this library.

I noticed there is a common pattern like:

                   onChange=(
                     event =>
                       event
                       |> Formality.Dom.toValueOnChange
                       |> form.change(SignUpForm.Email)
                   )
                   onBlur=(
                     event =>
                       event
                       |> Formality.Dom.toValueOnBlur
                       |> form.blur(SignUpForm.Email)
                   )

So I tried to extract two functions to handle this:

             let handleChange = (field, event) =>
               event |> Formality.Dom.toValueOnChange |> form.change(field);
             let handleBlur = (field, event) =>
               event |> Formality.Dom.toValueOnBlur |> form.change(field);

            /* Usage */
             onChange=(handleChange(SignUpForm.Email))
             onBlur=(handleBlur(SignUpForm.Email))

Then it doesn't validate any more, why?

alex35mil commented 6 years ago

@hulufei I can't say from these snippets where these helpers are defined and my question is: should you pass form record to the helper as well? E.g.:

let handleChange = (form, field, event) =>
  event |> Formality.Dom.toValueOnChange |> form.change(field);
let handleBlur = (form, field, event) =>
  event |> Formality.Dom.toValueOnBlur |> form.change(field);

/* Usage */
onChange=(handleChange(form, SignUpForm.Email))
onBlur=(handleBlur(form, SignUpForm.Email))
hulufei commented 6 years ago

@alexfedoseev the whole snippets is inside a spread children, so form is a parameter in outside scope:

<FormContainer>
...(form => {
  let handleChange = (field, event) =>
    event |> Formality.Dom.toValueOnChange |> form.change(field);
  let handleBlur = (field, event) =>
    event |> Formality.Dom.toValueOnBlur |> form.change(field);
})
</FormContainer>
alex35mil commented 6 years ago

Hmm, that's odd. I need to get married now but I will look into this soon.

hulufei commented 6 years ago

@alexfedoseev Congratulations! take your time:)

alex35mil commented 6 years ago

@hulufei I tested all possible ways to extract handlers with changes in #37 and it worked for me. Will release soon. Feedback on those changes are also appreciated.

alex35mil commented 5 years ago

@hulufei I'm closing this, feel free to re-open if it's still an issue for you.