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 <ArrayInput> should keep error state on children after unmount #9677

Merged
merged 1 commit into from
Mar 1, 2024

Conversation

sweco-sedalh
Copy link
Contributor

We have a rather large form, in which we for optimization reasons unmount parts of the form when not visible. This doesn't work to well with <ArrayInput /> however, which clears all state (errors, touched, etc.) when it is unmounted, which both means that the general valid/invalid state of the form is no longer correct.

To reproduce:

  1. Setup a react-admin project with an <ArrayInput /> that can be unmounted and some field that can have errors as child
  2. Open, enter an invalid value, try to save, field gets marked as error
  3. Trigger unmounting and remounting of <ArrayInput />
  4. The error is gone, despite the value still being invalid

Here is a reproducible example: https://stackblitz.com/edit/github-ntzyud-hb8uu4?file=src%2Fposts%2FPostEdit.tsx (note that tabbed forms have a similar issue, however that can more easily be worked around in user-space code)

This fix uses the react-hook-form options to keep the state of the <ArrayInput /> when it is unmounted.

I'm not 100% sure if this might break BC (I think it would be possible to build a form that depends on the current behavior, but I don't think it would make much sense to do so), if that is the case let me now and I'll retarget this PR to the next branch.

Copy link
Contributor

@slax57 slax57 left a comment

Choose a reason for hiding this comment

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

Thanks for your contribution, and for the detailed explanation!

AFAICT this goes in the right direction, and fixes the form's behavior to make it more coherent.

I can't think of a use-case where this would be a breaking change, since you already cannot validate the form if the ArrayInput is invalid, even unmounted. This fix simply allows to see the validation error at the correct time.

However I'll request the review of @djhi or @fzaninotto , just to be on the safe side.

Also, it would be nice if you could add a unit test covering this case, so that we don't run into this issue in the future. Can you try adding a new test?

Thanks

@slax57 slax57 requested a review from djhi February 23, 2024 16:22
@fzaninotto
Copy link
Member

Seems fair to me, too. I agree this needs unit tests to prove that it fixes something, and that we won't break it in the future.

@sweco-sedalh
Copy link
Contributor Author

sweco-sedalh commented Feb 26, 2024

Done!

Test fails without the fix and succeeds with the fix.

Copy link
Contributor

@slax57 slax57 left a comment

Choose a reason for hiding this comment

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

Thanks!

@slax57 slax57 added this to the 4.16.12 milestone Mar 1, 2024
@slax57 slax57 merged commit 0f9c16d into marmelab:master Mar 1, 2024
10 checks passed
@slax57 slax57 changed the title Keep errors and other state on ArrayInput unmount Fix <ArrayInput> should keep error state on children after unmount Mar 1, 2024
@sweco-sedalh sweco-sedalh deleted the fix-keepError branch March 5, 2024 08:53
djhi added a commit that referenced this pull request Jun 24, 2024
mjarosch pushed a commit to mjarosch/react-admin that referenced this pull request Jul 1, 2024
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 this pull request may close these issues.

3 participants