React-final-form: Form can be submitted even with validation errors

[vladshcherbin]

Hey :)

Are you submitting a bug report or a feature request

Bug report

What is the current behavior

Even with validation errors, I am able to submit a non-valid form if not all fields are rendered.

What is the expected behavior

I expect that I will be not able to submit a non-valid form that has validation errors.

How to reproduce the issue

I took Synchronous Record-Level Validation example and slightly modified it - removed two fields and added console.log with validation errors.

Here is a codesandbox demo - https://codesandbox.io/s/53xrn6v86p

Try to submit an empty form and you will have 3 validation errors. Now, fill in the only field in the form and you'll still have 2 validation errors. Even though you have errors left, you will be able to submit the form.

Screen Shot

---

This bug also exists in redux-form and was mentioned in issues multiple times. 😢
Maybe it'll be fixed in the final-form (the title sounds awesome) 🎉

[skvale]

To clarify, the validation errors in the sandbox aren't Fields in the form. While firstName does trigger validation, lastName etc.. aren't Fields and are ignored.

It looks like final-form is built to address this use case with a special error key Symbol('form-error') in

import { FORM_ERROR } from 'final-form'
...
   errors[FORM_ERROR] = 'General form error'
...

As in this codesandbox demo https://codesandbox.io/s/wwj6xzkrol

[vladshcherbin]

@skvale no, FORM_ERROR is used for global form errors.

The issue is with Field errors, if the Field for the error is not rendered (and not registered I guess), you'll be able to submit the form with errors for this field. This is bad when you forgot to add a field but have a rule that checks it, when you have tabs in your form, when you have a multi-page wizard form, etc.

[erikras]

It's not obvious to me whether this is a feature or a bug; I think it depends on your assumptions.

It was a conscious decision made between redux-form v5 and v6 to only require _rendered_ fields to pass validation to submit. In the redux-form Wizard Example, the Next action to move from page to page _is_ calling submit. This is so that it won't let you off the first page until all the fields are valid (and only marks those rendered fields as touched), and let all pages share the same record-level validation function.

@VladShcherbin How would you recommend keeping the user on the first page of a wizard form if there is a validation error on that page?

[vladshcherbin]

@erikras actually I've never had a wizard form in my projects, but I've seen the same question - https://github.com/erikras/redux-form/issues/1520#issuecomment-239271091. The proposed solution was to use a step (or a page) prop and to validate different rules based on this prop. I think I'd use a similar solution if I had a wizard form:

validate = (values, props) => {
  if (props.step === 1) {
    return validate(values, { username: 'required', password: 'required' })
  }

  if (props.step === 2) {
    return validate(values, { bio: 'required' })
  }
}

Another possible solution is to add a form config flag, smth like validateUnmounted. Not sure about the name though, because in my mind if validation returns any errors, you shouldn't be able to submit the form and why does the non-valid form need to know, what fields were mounted or not to be invalid.

What do you think?

[erikras]

Having thought about this for a few hours, it seems like you could accomplish what you want with this:

const validate = values => {
  const errors = {}
  // do validation
  return errors
}

const onSubmit = async values => {
  const errors = validate(values)
  if(Object.keys(errors).length) {
    return errors
  }
  // actually submit
  ajax.post('/blahblah', values)
}

You could reuse your record-level validation function to return submission errors, thus halting the submit.

Reasonable? Unreasonable?

[MichaelDeBoey]

@erikras Don't know if your solution would work when using react-router...?

[vladshcherbin]

@erikras yeah, can work as a current workaround, however what about validate then, it kinda supposed to do the same thing so we don't need to call validation in every submit manually. I've got probably 15 or more forms, manually adding this to all of them feels really sad.

What cons/problems do we have if onSubmit doesn't work until validate returns no errors - wizard forms or something else?

I feel like this issue is a very common one and needs a better solution.

[erikras]

Don't know if your solution would work when using react-router...?

Not sure what react-router has to do with anything?

What cons/problems do we have if onSubmit doesn't work until validate returns no errors - wizard forms or something else?

The main problem I have with it is that there is no good way to tell the user that they made a mistake if they aren't even seeing the field they made the mistake on. Say you have a form with name and email, but there is an error because city is required. What does that UX look/feel like?

[MichaelDeBoey]

Not sure what react-router has to do with anything?

@erikras Forget about that, the ajax call is for backend, my bad 😓

[vladshcherbin]

@erikras

The main problem I have with it is that there is no good way to tell the user that they made a mistake if they aren't even seeing the field they made the mistake on. Say you have a form with name and email, but there is an error because city is required. What does that UX look/feel like?

Correct me if I'm wrong, the form has a valid prop which should not be true until there are errors, returned in validate.

A developer can check this prop and show a message that the form has errors. From the docs I didn't find a way how you can get all form errors using props, only FORM_ERROR error using submitError/error prop. Maybe this can be added so a developer can show all errors if needed.

This can also be a great addition when you have tabs with hidden fields. You can check all errors and find which field errors you have and highlight tabs with this fields inside.

What do you think about this one?

[erikras]

Yes. Injecting errors, like we do values into the form render function makes sense.

[export-mike]

@VladShcherbin but surely by using the validation booleans that are provided? invalid, pristine? protects submit handlers?

<button type="submit" disabled={pristine || invalid}>Submit</button>

[erikras]

The valid/invalid flags are currently based solely on _registered_ fields.

Having thought about this for 24 hours, I think @VladShcherbin might be right. Non-registered field errors should also affect valid/invalid flags and halt submission.

[erikras]

Published fix in [email protected].

[erikras]

☝️ This is exactly why I like the idea of having separate libraries. I can fix bugs in one and don't necessarily need to do releases on both of them.

[vladshcherbin]

@erikras I've been waiting for this fix for so long, after I found this in redux-form the first thing I check in forms packages is this one.

Thank you ❤️

[erikras]

Technically this is a "breaking" change but the library is two days old, so I doubt I'm causing much grief with this slight change. 😄

[erikras]

LOL, this change actually _reduced_ the size of the bundle.

[MichaelDeBoey]

@erikras extra functionality + reduced bundle size 🙂
Maybe you have to reconsider your

I plan on being a little more willing to say, “No, I will not implement your specific feature,” with this library, to minimize its complexity and surface area.

You can add "If it reduces bundle size, it will have a bigger change of being implemented" 😂

[erikras]

Published _actual_ fix in final-form v1.2.1.

[lock[bot]]

This thread has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.