Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Global and input level validation can not be combined #7697

Closed
soullivaneuh opened this issue May 16, 2022 · 8 comments · Fixed by #7703
Closed

Global and input level validation can not be combined #7697

soullivaneuh opened this issue May 16, 2022 · 8 comments · Fixed by #7703

Comments

@soullivaneuh
Copy link
Contributor

This is a follow up of #6270, closed because of a v3 environment bug report: #6270 (comment)

I'll be happy to close this issue and re-open if I notice this behaviour in v4

@andrico1234 the behavior I reported before is for v4, so here is a new one! :-)

What you were expecting:

Using both global and input level validations on a same form should merge the behavior.

What happened instead:

Only the global validation is executed

Steps to reproduce:

  1. make run-simple
  2. Go to http://localhost:8080/#/comments/create
  3. Fill the "Author" field with foo
  4. Submit the form

The "Author" form should trigger an error because of minLength(10) input level configuration.

Related code:

pocivaneuh#2

Other information:

Environment

  • React-admin version: v4
  • Last version that did not exhibit the issue (if applicable): v3
  • React version: v17
@slax57
Copy link
Contributor

slax57 commented May 17, 2022

Thank you for taking the time to update this issue.

Actually, it appears that this is a known limitation of the react-hook-form library, and this is documented in the migration guide here:

Validation: Form Level & Input Level Are Mutually Exclusive

But, I have to admit this does not appear clearly enough (IMO) in the Form Validation doc.
I'll do a quick PR to have this highlighted in this section of the docs as well.

@soullivaneuh
Copy link
Contributor Author

@slax57 Well, this is not really convenient and reduce a lot the interest of the input level validation.

Here is my use case with a discount code form:

const validate: SimpleFormProps['validate'] = (values) => {
  const errors: any = {};

  if (values.beginAt && values.endAt && values.beginAt >= values.endAt) {
    errors.endAt = 'Doit être supérieure à la date de début';
  }

  return errors;
};

export const DiscountCodeForm: FC<Omit<SimpleFormProps, 'children'>> = (props) => (
  <SimpleForm
    {...props}
    validate={validate}
  >
    <TextInput
      source="code"
      validate={[
        required(),
        minLength(5),
        maxLength(20),
        regex(/^[a-z0-9]+$/i, 'Uniquement des lettres et des chiffres'),
      ]}
    />
    <TextInput
      source="label"
      validate={[
        required(),
      ]}
    />
    <NumberInput
      source="amount"
      validate={[
        required(),
        minValue(0.1),
      ]}
    />
    <DateInput
      source="beginAt"
      validate={[
        required(),
      ]}
    />
    <DateInput
      source="endAt"
      validate={[
        required(),
      ]}
    />
    <ArrayInput
      source="whitelist"
      helperText="Laisser vide pour autoriser n'importe qui."
    >
      <SimpleFormIterator>
        <TextInput
          source=""
          label="fields.emailAddress"
        />
      </SimpleFormIterator>
    </ArrayInput>
  </SimpleForm>
);

As you can see, I have many required fields, some advanced validation for the code field, and a global validator to verify the beginAt value is lower than the endAt value.

If I follow the documentation, then I have to reproduce the same behavior of each function defined in validate.ts manually on the global validation.

This is very cumbersome and kill the simplicity of react-admin.

Don't we have any way to wrap the validation system to apply both react-hook-form validation and the react-admin inline ones?

By the way, do we have a link of the react-hook-form documentation talking about this limitation?

Thanks

@slax57
Copy link
Contributor

slax57 commented May 19, 2022

I agree. I'll reopen this as an enhancement feature so that we can discuss with the team what we can do about this.

@soullivaneuh
Copy link
Contributor Author

soullivaneuh commented May 19, 2022

Thanks! By the way, I share to you two information:

  1. This limitation currently make some examples not reproducible, like the second code part of the async validation: https://marmelab.com/react-admin/doc/4.0/Validation.html#async-validation
  2. Dealing with the global validation, the cumbersome part apart, is currently not doable because of The validate errors message are not display when passed as translatable object #7690

@slax57
Copy link
Contributor

slax57 commented May 20, 2022

Okay, so.
Team confirms this is a problem on the react-hook-form side. What's more, I created a little codesandbox using nothing but react-hook-form that reproduces the problem:
https://codesandbox.io/s/react-hook-form-get-started-forked-yerxqs?file=/src/index.js
Comment line 34 and uncomment line 35 to see the problem happen.

We have tried to overcome limitations of third-party libs by the past, but this really turned out to be a pain, so for this problem we highly suggest you open an issue on the react-hook-form tracker, asking them to support this.

Regarding your latest comment, indeed the doc needs to be fixed.
Regarding #7690: don't the workarounds I suggest actually work? 😶

@soullivaneuh
Copy link
Contributor Author

Hi @slax57 and thanks for the feedback.

For now I just remove the global validation as we use it only once. We will work on a better API related validation displaying, so we don't have concrete case for the moment.

Because of that, I won't look further for now. I suggest to keep that issue open, maybe you can ask react-hook-form yourself later. By the way, I'm sure you'll have a better knowledge of the situation than me to do that. 😉

About #7690, I didn't even try for the previous reason. I was struggling with another more important upgrade issues. 😉

@slax57
Copy link
Contributor

slax57 commented May 23, 2022

#7726 has been merged and fixes the documentation part.

Keeping this open as an enhancement request.

@fzaninotto
Copy link
Member

I'm closing this as the enhancement should come from the react-hook-form side.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
3 participants