final-form / react-final-form

🏁 High performance subscription-based form state management for React
https://final-form.org/react
MIT License
7.38k stars 481 forks source link

Wizard example bug - validation between steps #664

Open kg-currenxie opened 4 years ago

kg-currenxie commented 4 years ago

Are you submitting a bug report or a feature request?

🐛

What is the current behavior?

  1. Fill in firstName and lastName to be able to proceed to step 2
  2. Don't touch any field, go back to Step 1
  3. Don't touch any field, go forward to Step 2 <---- Don't allow me to

What is the expected behavior?

Sandbox Link

Official example from docs: https://codesandbox.io/s/github/final-form/react-final-form/tree/master/examples/wizard?from-embed

My version: https://codesandbox.io/s/wizard-example-iqycj

What I changed:

What's your environment?

"final-form": "4.18.5", "react-final-form": "6.3.0",

Other

I understand that it's up to us to handle the validations, and that final-form only provides the validate prop, but as this example is official in your docs, I assume it should work.

jsrhodes15 commented 4 years ago

Since the fields on page 2 don't have validation fns attached, the validation is not removed when the field unmounts (which "unrigesters" the field, also removing any validation).

This leaves the form in an "invalid" state, which blocks submission. If you need to consolidate your validation to "record level", perhaps you can check if the field is still registered and make that part of the condition for returning an error?

Looking at the code I don't see a straitfoward way to change this behavior in the lib..

eliseumds commented 4 years ago

@jsrhodes15 do you know if there is a way to access the registered fields in the validate callback? I logged all the parameters and the only thing I get are the values. That's a considerable change relative to redux-form and it'd be great if I could maintain the same behaviour while migrating everything.

jsrhodes15 commented 4 years ago

@eliseumds I have spent the last couple of hours trying to figure this out for you. I don't see a way to do exactly what you are trying to do while using the Form's validate prop for record level validation. In our conversion from Redux-Form to react-final-form, we went with field level validation mostly, and did our own custom record level validation in the function we pass to onSubmit.

When function is called we have access to the form instance, where we can call form.getRegisteredFields. That will return an object where the keys are field names. You could then use those field names to only run validation on fields that are registered. In this case, you will want to return an object in the shape of your form values (just like the errors object).

https://final-form.org/docs/react-final-form/types/FormProps - Look for the onSubmit type to see what I am talking about. This may get you the closes parity and reuse of your existing code (you could use whatever you were passing to validate inside of the onSubmit.

Hope that helps.

One thing I should say is that in the Wizard example from the docs, if you go on to the third page of the wizard, you will see the same behavior that you are seeing in your example. So you aren't doing anything wrong. Once the fields mount, they are registered, and the validation errors are also applied. This makes the form invalid, so submit will fail when attempting to go from page 2 to page 3.

eliseumds commented 4 years ago

@jsrhodes15 I'll try that, thanks for the suggestion!

joshf commented 4 years ago

i am facing the same problem

kg-currenxie commented 4 years ago

We're not quite keen on refactoring all our forms and components to use field level validation :|

Wouldn't it be possible to get the form argument in the validate prop arguments? Then we could use getRegisteredFields() inside the record level validation

  onSubmit: (
    values: FormValues,
    form: FormApi<FormValues>,
    callback?: (errors?: SubmissionErrors) => void
  ) =>
    | SubmissionErrors
    | Promise<SubmissionErrors | undefined>
    | undefined
    | void
  validate?: (
    values: FormValues,
+   form: FormApi<FormValues>
  ) => ValidationErrors | Promise<ValidationErrors> | undefined

:D

kg-currenxie commented 4 years ago

Update: While trying to refactor, I accidentally made a change that actually fixed the bug

Before:

      <Form
        initialValues={values}
        validate={this.validate}

After:

      <Form
        initialValues={values}
        validate={(values) => {
          return this.validate(values)
        }}

PS: I have actually no idea why this fixed it. It seems like it should behave the same..? But after applying this fix; I've played around with the form trying to reproduce the bug, and I CAN'T :) Reverting it, would immediately reproduce it.

akbkk commented 3 years ago

Ran into the same issue. If I understand correctly final-form is memoize'ing everything. Thus validate won't run when we return to the previous page because the values are the same. But inline lambda is recreated with every render thus memoize won't work. So validate WILL run on the return and the errors will be cleared.

eddythemeddy commented 3 years ago

@kg-currenxie Can you please show me your code in sandbox, I am having the same issue and I did exactly what you did to the form but Im still having this validation issue!

kg-currenxie commented 3 years ago

@eddythemeddy seems i can't reproduce it in the sandbox example to be honest :( But it has something to do with re-rendering and what @akbkk said

eddythemeddy commented 3 years ago

@kg-currenxie No worries mate i fixed it, you were right about the validate. Thing is I was using the Wizard as well, so I had to set it up properly in both places. Got it working :). Thanks though!