marmelab / react-admin

A frontend Framework for building data-driven applications running on top of REST/GraphQL APIs, using TypeScript, React and Material Design
http://marmelab.com/react-admin
MIT License
24.52k stars 5.2k forks source link

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

Open k4v1cs opened 2 weeks ago

k4v1cs commented 2 weeks ago

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

k4v1cs commented 2 weeks ago

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

slax57 commented 2 weeks ago

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 commented 2 weeks ago

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