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

Ghost validation error when removing item from ArrayInput with nested ArrayInput #9961

Open
k4v1cs opened this issue Jun 27, 2024 · 8 comments
Labels
dependencies Pull requests that update a dependency file

Comments

@k4v1cs
Copy link

k4v1cs commented Jun 27, 2024

What you were expecting:
Removing an item form an ArrayInput should not cause validation errors on non-existent fields

What happened instead:
After I removed the item and validated the form, there was a validation error for a no longer existing index

Steps to reproduce:
Have an ArrayInput with items which also have ArrayInputs. The arrays as well as their items are validated as required.
Codesandbox: https://codesandbox.io/p/devbox/nested-arrayinput-remove-item-issue-hz4yq8

  1. Open edit view
  2. Remove the second top level array item from the 4 ("name2")
  3. Click submit

The form is invalid, there is a validation error for the 4th item's nested array.

React.App.-.Google.Chrome.2024-06-27.14-44-32.mov

Related code:

      <ArrayInput source="names" validate={required()}>
        <SimpleFormIterator>
          <TextInput source="name" validate={required()} />
          <ArrayInput source="hobbies" validate={required()}>
            <SimpleFormIterator>
              <TextInput source="hobby" validate={required()} />
            </SimpleFormIterator>
          </ArrayInput>
        </SimpleFormIterator>
      </ArrayInput>

Other information:
The issue is reproducible with version 5 as well: https://codesandbox.io/p/devbox/nested-arrayinput-remove-item-issue-forked-8shcyl

Environment

  • React-admin version: 4.16.10
  • Last version that did not exhibit the issue (if applicable):
  • React version: 18.2.0
  • Browser: Chrome
  • Stack trace (in case of a JS error):
@k4v1cs
Copy link
Author

k4v1cs commented Jun 27, 2024

#9839 might be related or the same issue

@slax57
Copy link
Contributor

slax57 commented Jun 27, 2024

Reproduced, thanks for the report.

It seems the issue disappears if you set shouldUnregister on the <Form>.

const TestEdit = () => {
  return (
    <Edit>
      <SimpleForm shouldUnregister>
        <Form />
      </SimpleForm>
    </Edit>
  );
};

This can't be considered as a fix for the issue, but may be used as a workaround.

@slax57
Copy link
Contributor

slax57 commented Jun 27, 2024

I tried to reproduce the issue in a RHF only sandbox, to see if the issue came from their code or ours, but couldn't reproduce the issue there (https://codesandbox.io/p/sandbox/calc-forked-hp97w7), which seems to indicate this affects react-admin only.

Also, I realize it is possible to set shouldUnregister at the useFieldArray level, so maybe this is something we should do in <ArrayInput>. Not so sure about the impacts though...

@fzaninotto fzaninotto removed the bug label Oct 3, 2024
@fzaninotto
Copy link
Member

As seen in #10271, useFieldArray has a known limitation with shouldUnregister. So using it by default in <ArrayInput> is not an option.

@fzaninotto
Copy link
Member

I managed to reproduce it with react-hook-form alone, based on @slax57 's Codesandbox:

https://codesandbox.io/p/sandbox/calc-forked-93m7f3?workspaceId=3123472d-a31f-4ddf-a4e0-79c596270dc0

The key difference is to use a validate method instead of a validator in the register call.

So this is definitely a react-hook-form bug. I'll open an issue in their tracker.

@fzaninotto
Copy link
Member

Nope, false alarm, I didn't manage to reproduce it with react-hook-form alone. Still investigating.

@fzaninotto
Copy link
Member

React-hook-form v7.53.1 seems to fix a related issue, but the bug is still present in react-admin with this version.

See react-hook-form/react-hook-form#12291

@fzaninotto
Copy link
Member

OK, it IS a react-hook-form issue, as I reproduced it in the following sandbox that doesn't use react-admin:

https://codesandbox.io/p/sandbox/calc-forked-93m7f3?workspaceId=3123472d-a31f-4ddf-a4e0-79c596270dc0

I have reported the problem to react-hook-form (see react-hook-form/react-hook-form#12385)

@fzaninotto fzaninotto added the dependencies Pull requests that update a dependency file label Oct 29, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
dependencies Pull requests that update a dependency file
Projects
None yet
Development

No branches or pull requests

3 participants