rescriptbr / reform

📋 Reasonably making forms sound good
https://rescript-reform.netlify.app/
MIT License
354 stars 41 forks source link

Unexpected validation pass about nested fields #244

Closed mununki closed 1 year ago

mununki commented 1 year ago

Hi team!

Thank you for your awesome work! I think the ReForm is a unique form validation library built from the scratch with ReScript. I really like your work.

I encounter a weird unexpected validation pass about the nested field array. Here is my example:

module ProfileFormFields = %lenses(
  type t = {
    nickname: string,
    age: int,
  }
)

module FormFields = %lenses(
  type state = {
    name: string,
    email: string,
    profiles: array<ProfileFormFields.t>,
  }
)

module Form = ReForm.Make(FormFields)
...
        schema([
          custom(({profiles}) => {
            let errors: array<ReSchema.childFieldError> = // <-- 1
              profiles
              ->Array.mapWithIndex((index, profile) =>
                if profile.nickname == "" || profile.age <= 0 {
                  let error: ReSchema.childFieldError = {
                    error: "Invalid profile",
                    index,
                    name: "???",
                  }
                  Some(error)
                } else {
                  None
                }
              )
              ->Array.keepMap(x => x)

            switch errors {
            | [] => Valid
            | _ => NestedErrors(errors) // <- 2. return NestedErrors!
            }
          }, Profiles)
        ])
...

Field(Profiles)->form.getNestedFieldError(index)->Js.log // errors are all None??

The length of errors in comment:1 is not 0, so it means that this predicate function returns NestedErrors(errors). But none of the indexes in the nested field has an error. So, the submission proceeded even though the nested fields are not satisfied in the custom schema.

I guess that validate function (https://github.com/rescriptbr/reschema/blob/master/src/ReSchema.res#L287:L292) in ReSchema should check the NestedErrors there. Or anything I missed for the nested fields validation?

mununki commented 1 year ago

Full example is here: https://gist.github.com/mununki/1e9cecfcdf9d52b22aafd517b7b33df5

vmarcosp commented 1 year ago

Hey @mununki, first off, sorry for the delay, it has been a busy month.

Thank you for your awesome work! I think the ReForm is a unique form validation library built from the scratch with ReScript. I really like your work.

Actually, thank YOU for the awesome work on the compiler, I was just about to DM you on twitter to thank you (and all people from green-labs) for the amazing contributions to the ReScript ecosystem <3

About the issue, I'll try to run the given example in a demo project and I'll come with a solution as soon as possible.

@fakenickels I think you can help us here, since you're worked on this part.

vmarcosp commented 1 year ago

Hey @mununki

I created a demo project and ran your example locally. It's a weird behaviour, the reschema should validate the field state and looking for the nested error constructor instead of just returning a valid state.

We're working on fix this and we'll release as soon as possible.

Since this is not an issue with ReForm, I'm going to move the conversation to the ReSchema's repository, ok? You don't do need to do anything, I'll replicate the issue there and tag you to follow the incoming updates.

mununki commented 1 year ago

@vmarcosp Thank you for following up on this issue.

Since this is not an issue with ReForm, I'm going to move the conversation to the ReSchema's repository, ok?

No problem. 👍