Improve useFieldArray to allow async validation#187
Improve useFieldArray to allow async validation#187iamdey wants to merge 1 commit intofinal-form:masterfrom
useFieldArray to allow async validation#187Conversation
|
Thanks for this contribution! 👍 I can see you've been using this in production for a couple of years. What this adds:
Code review observations: Questions/concerns:
@erikras - This is a feature enhancement (not a bug fix) that enables async validation for field arrays, matching the API from regular fields. Needs CI before merge and consideration of interaction with #509 async validation work. I'll hold off on approval until CI runs and we can verify no conflicts with ongoing async validation work. Would you like me to help with documentation if this gets merged? |
|
Rebase, please? |
also add more tests
iamdey
force-pushed
the
async_validator
branch
from
206b2d6 to
b3e2be5
Compare
📝 WalkthroughWalkthroughThe changes enhance Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
I though you were talking to your bot |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/useFieldArray.test.tsx`:
- Around line 211-244: The test "should handle Promises errors" uses waitFor
without awaiting it, which can let the test finish before the async expectation
runs; update the test by awaiting the waitFor call that wraps the expectation so
the Promise returned by waitFor is resolved before the test completes (modify
the test body where waitFor(...) is called in the "should handle Promises
errors" spec that renders MyFieldArray and uses spy to assert useFieldArray's
meta.error).
| it('should handle Promises errors', async () => { | ||
| const spy = jest.fn() | ||
| const MyFieldArray = () => { | ||
| spy( | ||
| useFieldArray('names', { | ||
| validate: (values) => Promise.resolve(['await fail']) | ||
| }) | ||
| ) | ||
| return null | ||
| } | ||
| render( | ||
| <Form | ||
| onSubmit={onSubmitMock} | ||
| mutators={arrayMutators as any} | ||
| subscription={{}} | ||
| > | ||
| {() => ( | ||
| <form> | ||
| <MyFieldArray /> | ||
| </form> | ||
| )} | ||
| </Form> | ||
| ) | ||
|
|
||
| waitFor(() => | ||
| expect(spy).toHaveBeenCalledWith( | ||
| expect.objectContaining({ | ||
| meta: expect.objectContaining({ | ||
| error: ['await fail'] | ||
| }) | ||
| }) | ||
| ) | ||
| ) | ||
| }) |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# First, find the test file
find . -name "useFieldArray.test.tsx" -type fRepository: final-form/react-final-form-arrays
Length of output: 106
🏁 Script executed:
# Once found, check the file size and read the specific lines
wc -l src/useFieldArray.test.tsxRepository: final-form/react-final-form-arrays
Length of output: 108
🏁 Script executed:
# Read lines 211-244 and surrounding context (with imports at top)
head -20 src/useFieldArray.test.tsxRepository: final-form/react-final-form-arrays
Length of output: 850
🏁 Script executed:
# Read the specific test around lines 211-244
sed -n '205,250p' src/useFieldArray.test.tsxRepository: final-form/react-final-form-arrays
Length of output: 949
Await waitFor to ensure the async assertion runs.
The test function is async but the waitFor call on line 235 is not awaited. Since waitFor returns a Promise, the test will complete before the expectation inside it runs, potentially causing false passes.
Fix
- waitFor(() =>
+ await waitFor(() =>
expect(spy).toHaveBeenCalledWith(
expect.objectContaining({
meta: expect.objectContaining({
error: ['await fail']
})
})
)
)📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| it('should handle Promises errors', async () => { | |
| const spy = jest.fn() | |
| const MyFieldArray = () => { | |
| spy( | |
| useFieldArray('names', { | |
| validate: (values) => Promise.resolve(['await fail']) | |
| }) | |
| ) | |
| return null | |
| } | |
| render( | |
| <Form | |
| onSubmit={onSubmitMock} | |
| mutators={arrayMutators as any} | |
| subscription={{}} | |
| > | |
| {() => ( | |
| <form> | |
| <MyFieldArray /> | |
| </form> | |
| )} | |
| </Form> | |
| ) | |
| waitFor(() => | |
| expect(spy).toHaveBeenCalledWith( | |
| expect.objectContaining({ | |
| meta: expect.objectContaining({ | |
| error: ['await fail'] | |
| }) | |
| }) | |
| ) | |
| ) | |
| }) | |
| it('should handle Promises errors', async () => { | |
| const spy = jest.fn() | |
| const MyFieldArray = () => { | |
| spy( | |
| useFieldArray('names', { | |
| validate: (values) => Promise.resolve(['await fail']) | |
| }) | |
| ) | |
| return null | |
| } | |
| render( | |
| <Form | |
| onSubmit={onSubmitMock} | |
| mutators={arrayMutators as any} | |
| subscription={{}} | |
| > | |
| {() => ( | |
| <form> | |
| <MyFieldArray /> | |
| </form> | |
| )} | |
| </Form> | |
| ) | |
| await waitFor(() => | |
| expect(spy).toHaveBeenCalledWith( | |
| expect.objectContaining({ | |
| meta: expect.objectContaining({ | |
| error: ['await fail'] | |
| }) | |
| }) | |
| ) | |
| ) | |
| }) |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/useFieldArray.test.tsx` around lines 211 - 244, The test "should handle
Promises errors" uses waitFor without awaiting it, which can let the test finish
before the async expectation runs; update the test by awaiting the waitFor call
that wraps the expectation so the Promise returned by waitFor is resolved before
the test completes (modify the test body where waitFor(...) is called in the
"should handle Promises errors" spec that renders MyFieldArray and uses spy to
assert useFieldArray's meta.error).
3 participants
In order to have similar API as in Asynchronous Field-Level Validation cf. https://final-form.org/docs/react-final-form/examples
This allows to get errors from an async function.
This PR is similar to #179 but with tests and non-breaking for sync validators tests.
I used this code for a couple of years now.
Summary by CodeRabbit
Bug Fixes
Tests