react-admin icon indicating copy to clipboard operation
react-admin copied to clipboard

Ghost validation error when removing item from ArrayInput with nested ArrayInput

Open k4v1cs opened this issue 1 year ago • 3 comments

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.

https://github.com/marmelab/react-admin/assets/4885562/839554e9-5159-4a56-b4c6-91453e9c40d9

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 avatar Jun 27 '24 13:06 k4v1cs

https://github.com/marmelab/react-admin/issues/9839 might be related or the same issue

k4v1cs avatar Jun 27 '24 13:06 k4v1cs

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 avatar Jun 27 '24 13:06 slax57

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...

slax57 avatar Jun 27 '24 14:06 slax57

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

fzaninotto avatar Oct 29 '24 10:10 fzaninotto

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 avatar Oct 29 '24 12:10 fzaninotto

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

fzaninotto avatar Oct 29 '24 13:10 fzaninotto

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 https://github.com/react-hook-form/react-hook-form/issues/12291

fzaninotto avatar Oct 29 '24 13:10 fzaninotto

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 https://github.com/react-hook-form/react-hook-form/issues/12385)

fzaninotto avatar Oct 29 '24 15:10 fzaninotto

This was apparently fixed https://github.com/react-hook-form/react-hook-form/pull/12405 And included in version 7.54.0

WiXSL avatar Dec 27 '24 17:12 WiXSL

Confirmed! Forking the provided sandbox and updating RHF to the latest version (7.54.2) fixes the issue! Thanks @WiXSL for the update.

slax57 avatar Jan 06 '25 10:01 slax57