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

Ajv resolver not using custom error messages on required errors #615

Open
amyc92 opened this issue Aug 13, 2023 · 4 comments · May be fixed by #728
Open

Ajv resolver not using custom error messages on required errors #615

amyc92 opened this issue Aug 13, 2023 · 4 comments · May be fixed by #728

Comments

@amyc92
Copy link

amyc92 commented Aug 13, 2023

Describe the bug
Without a custom required error message set on my AJV form schema, required form field errors are returned as expected with the default AJV required error message. With a custom required error message set, no error is set on the form field. The required error message is attached to the error body on a non existent "" form field

To Reproduce
Steps to reproduce the behaviour:

Set the form schema:

{
  "type": "object",
  "properties": {
    "name": {
      "type": "string"
    },
    "email": {
      "type": "string"
    }
  },
  "required": ["name", "email"],
  "errorMessage": {
    "required": {
      "name": "Name must be defined",
      "email": "Email must be defined"
    }
  }
}

Set the form value:

{
  "name": undefined,
  "email": undefined
}

Expected react hook form error when both name and email are not defined:

{
  "name": {
    "message": "Name must be defined",
    "type": "required"
  },
  "email": {
    "message": "Email must be defined",
    "type": "required"
  }
}

Actual error returned:

{
  "": { "message": "Name must be defined", "type": "errorMessage" }
}

Codesandbox link (Required)

Example

First name is using the default ajv error messages. Last name is using a custom required error message. On submitting the form, only the required first name error message is displayed.

Expected behavior
Custom required error messages should be returned by form errors on the relevant form field.

Desktop (please complete the following information):

  • OS: macOS Ventura v13.4
  • Browser Chrome
  • Version 115.0.5790.170
@jorisre
Copy link
Member

jorisre commented Aug 17, 2023

I'm unsure if we can resolve this issue.
An error should be associated with a field, aligning with how react-hook-form and resolvers function.

When you configure a custom error message using AJV (ajv-error) for the required property, AJV doesn't substitute the default error message with your customized one. Instead, it introduces a new error at the top level.
Due to this behavior, we can't establish a connection between the custom error message and the default one, which is why you're seeing "": {}.

After delving into this, I haven't discovered a way to substitute the default error message with the customized version. If you're aware of a solution, kindly share it, and I'll attempt to update the resolver accordingly.

This CodeSandbox shows a basic AJV validation with a custom error message
https://codesandbox.io/s/suspicious-silence-kt7qqk

The result from the validation:

[
    {
        "instancePath": "",
        "schemaPath": "#/errorMessage",
        "keyword": "errorMessage",
        "params": {
            "errors": [
                {
                    "instancePath": "",
                    "schemaPath": "#/required",
                    "keyword": "required",
                    "params": {
                        "missingProperty": "username"
                    },
                    "message": "must have required property 'username'",
                    "emUsed": true
                }
            ]
        },
        "message": "username is required" // ⚠️ we can't associate this error message with the error
    }
]

@ig4e
Copy link

ig4e commented Apr 19, 2024

@jorisre Can you explain this more to me? I'm trying to fix it

@Youpiiiii
Copy link

Me too

@sjahan
Copy link

sjahan commented Jun 28, 2024

I just encountered this issue, not being able to display a custom error message for a required property.

What I don't get is that ajvResolver already handles the case of a required error that is bind to the root and not to the path of the field.
In https://github.com/react-hook-form/resolvers/blob/master/ajv/src/ajv.ts, you will find it from line 11 to 16.

 // Ajv will return empty instancePath when require error
  ajvErrors.forEach((error) => {
    if (error.keyword === 'required') {
      error.instancePath += '/' + error.params.missingProperty;
    }
  });

This piece of code rebuilds the path from the missing property in the case of a required error, so it feels like ajvResolver will handle the case, but actually, it does not match the payload of the actual error (which is the format mentioned in #615 (comment).

ajvResolver's code is quite standalone and easy to fork, it may be a dirty quick fix (not having every react-hook-form/resolver points in mind), but adding the following loop after the first one did the trick for me:

ajvErrors.forEach((error) => {
    if (error.keyword === 'errorMessage') {
      const subError = error.params.errors[0];
      if (subError.keyword === 'required') {
        error.instancePath += '/' + subError.params.missingProperty;
      }
    }
  });

As stated, it may be simplistic and/or dirty, but it definitely feels like ajvResolver could handle this.
I hope this will help!

ktunador added a commit to ktunador/reach-hook-form-resolvers that referenced this issue Dec 10, 2024
When `errorMessage` is used, ajv returns a special error object,`error.keyword='errorMessage'`,
and keeps original validation errors in `error.errors` field.
The proposed changes normalze the ajv validation errors,
so the resolver returns original error types, like `required` and `pattern`
while using custom error messages.

This also fixes react-hook-form#615
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants