Skip to content

Fix <FilterLiveForm> compatibility with react-hook-form 7.55.0, part 2#10697

Merged
djhi merged 4 commits intomasterfrom
fix-FilterLiveForm-RHF-7.55-part-2
Apr 25, 2025
Merged

Fix <FilterLiveForm> compatibility with react-hook-form 7.55.0, part 2#10697
djhi merged 4 commits intomasterfrom
fix-FilterLiveForm-RHF-7.55-part-2

Conversation

@slax57
Copy link
Contributor

@slax57 slax57 commented Apr 24, 2025

Problem

Despite #10657, 2 issues are still present when using RHF version >= 7.55.0:

Closes #10695

Solution

It turns out the fix brought by #10657 was not enough.
In a nutshell, this fix made it so that the filter form is not reset if changed values only affect fields that are not part of the form. But the form was still initialized with defaultValues equal to the filterValues, so external values could still leak into the form state.
Part of the solution is then to remove the form defaultValues, and rely only on the useEffect to (selectively) apply them.

Other part of the solution was to remove the formChangesPending ref, as it turns out this workaround no longer works with RHF v7.55.0 onwards, because they tend to call the useWatch callback multiple times, even for fields whose value has not changed.
Instead, properly managing the form state and only resetting when values known to the form are modified (i.e. part 1 of the fix above) seems to be enough to avoid re-applying old values.

How To Test

2 new stories were added:

Also, #10695 can be reproduced in story http://localhost:9010/?path=/story/ra-ui-materialui-list-filter-filterbutton--basic

Unit tests were added to cover all known issues.

=> Hence, to test the fix, I recommend:

  1. Playing around with the added stories (the unit tests are already run by the CI)
  2. Then, update RHF to its latest version by running yarn up react-hook-form, run the unit tests (make test-unit), the E2E tests (make test-e2e), and then play with the added stories (make storybook)

Note: You will notice that some tests not related to filters are failing when using RHF v7.56.1, which is something we'll need to address in a future PR, but all tests related to filters should pass with this PR.

Additional Checks

  • The PR targets master for a bugfix or a documentation fix, or next for a feature
  • The PR includes unit tests (if not possible, describe why)
  • The PR includes one or several stories (if not possible, describe why)
  • The documentation is up to date

Also, please make sure to read the contributing guidelines.

@slax57 slax57 added RFR Ready For Review WIP Work In Progress and removed RFR Ready For Review labels Apr 24, 2025
@slax57
Copy link
Contributor Author

slax57 commented Apr 24, 2025

Some E2E tests are failing, back to WIP

@slax57
Copy link
Contributor Author

slax57 commented Apr 24, 2025

Fixed, back to RFR!

Copy link
Contributor

@djhi djhi 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 good but maybe we should update react-hook-form to its latest version?

@slax57
Copy link
Contributor Author

slax57 commented Apr 25, 2025

I agree but we should do that on next. Besides, as I said, some other tests are failing which we'll need to investigate (soon).

@djhi djhi merged commit 8b164de into master Apr 25, 2025
16 checks passed
@djhi djhi deleted the fix-FilterLiveForm-RHF-7.55-part-2 branch April 25, 2025 07:45
@djhi djhi added this to the 5.7.4 milestone Apr 25, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

RFR Ready For Review

Development

Successfully merging this pull request may close these issues.

Last filter can not be removed if multiple filters added

2 participants