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

Fix global validation not firing after submit with ArrayInput #8118

Conversation

slax57
Copy link
Contributor

@slax57 slax57 commented Aug 30, 2022

Fixes #7594

Todo

Screenshots

Before

Peek 31-08-2022 15-07-previously

After

Peek 31-08-2022 14-58

Additional info

So here is some testing I made by myself:

In RA v3:

validate's result's shape works to display errors works to clear errors
flat ❌️ ❌️
tree ✅️ ✅️

In RA v4 - before this PR:

validate's result's shape works to display errors works to clear errors
flat ✅️ ❌️
tree ✅️ ❌️

In RA v4 - after this PR:

validate's result's shape works to display errors works to clear errors
flat ✅️ ❌️
tree ✅️ ✅️

Where:

  • flat syntax is like: {backlinks.1.date: 'A date is required'}
  • tree syntax is like: {backlinks: [{}, {date: 'A date is required'}]}

@slax57 slax57 added help wanted WIP Work In Progress labels Aug 30, 2022
Copy link
Member

@fzaninotto fzaninotto left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Excellent work! But I think that now that you've done all the digging, we should write down the proper format in the documentation.

Also, do we still need the flattenErrors utility?

values: {},
errors: {
title: { type: 'manual', message: 'title too short' },
backlinks: [
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

How does one specify errors in some of the items but not all? Does this work?

backlinks: [null, null, { url: { type: 'manual', message: 'url too short' }}, null]

I think it's worth a test.

Edit: I see you've made that test later. Well, it's worth some documentation in ArrayInput and in the global validation section.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've added some documentation. Please tell me if this is enough or if I need to emphasize that returning something like [{}, {}] is supported

{ url: 'url too short', id: 'missing id' },
{ url: 'url too short', id: 'missing id' },
],
describe('flattenErrors', () => {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why do we flatten in the first place if react-hook-form requires a tree structure?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

dunno 🤷

I was able to track its origin down to this issue and especially this comment: #7500 (comment)

But as I said I don't know what made you say this at the time. Since this is currently not documented on rhf's side either I can't tell if there was a change on their side or not...

// `type: 'manual'` and a `message` prop like react-hook-form expects it
const transformedErrors = transformErrorFields(errors);

// If, after transformation, there are no errors, return the form values
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

How is this possible? Please elaborate in the comment.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

it can happen if the validator returns an error object like:

        {
            backlinks: [{}, {}],
        }

in this case we first need to transform the error field to realize it actually contains no errors.

I'll add this in the comment.

@fzaninotto
Copy link
Member

Also, needs rebase.

Copy link
Contributor

@WiXSL WiXSL left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This looks great!

@fzaninotto
Copy link
Member

The added doc is fine!

@fzaninotto fzaninotto merged commit 95ab599 into master Sep 2, 2022
@fzaninotto fzaninotto deleted the 7594-global-validation-not-firing-after-submit-with-arrayinput branch September 2, 2022 08:55
@fzaninotto fzaninotto added this to the 4.3.1 milestone Sep 2, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
RFR Ready For Review
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Global validation not firing after submit with ArrayInput
3 participants