Skip to content

Improve useFieldArray to allow async validation #187

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.

Sign up for GitHub

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

Open
iamdey wants to merge 1 commit into
base: master
Choose a base branch
from
Open

Improve useFieldArray to allow async validation #187

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
129 changes: 119 additions & 10 deletions src/useFieldArray.test.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,10 +1,11 @@
import * as React from 'react'
import { act, render, cleanup } from '@testing-library/react'
import { act, render, cleanup, waitFor } from '@testing-library/react'
import '@testing-library/jest-dom'
import arrayMutators from 'final-form-arrays'
import { ErrorBoundary } from './testUtils'
import { Form, useField } from 'react-final-form'
import { Form, useField, useFormState } from 'react-final-form'
import useFieldArray from './useFieldArray'
import { ARRAY_ERROR } from 'final-form'

const onSubmitMock = (values: any) => {}

Expand Down Expand Up @@ -41,7 +42,11 @@ describe('FieldArray', () => {
return null
}
render(
<Form onSubmit={onSubmitMock} mutators={arrayMutators as any} subscription={{}}>
<Form
onSubmit={onSubmitMock}
mutators={arrayMutators as any}
subscription={{}}
>
{() => (
<form>
<MyFieldArray />
Expand All @@ -65,9 +70,9 @@ describe('FieldArray', () => {
// undefined is passed instead of a no-op function that always returns undefined.
// This prevents final-form from tracking this field as having a validator,
// which would trigger unnecessary form-wide validation.

const useFieldSpy = jest.spyOn(require('react-final-form'), 'useField')

const MyFieldArray = () => {
const fieldArray = useFieldArray('names')
return null
Expand All @@ -89,17 +94,17 @@ describe('FieldArray', () => {

// Verify that useField was called with validate: undefined
const useFieldCalls = useFieldSpy.mock.calls
const relevantCall = useFieldCalls.find(call => call[0] === 'names')
const relevantCall = useFieldCalls.find((call) => call[0] === 'names')
expect(relevantCall).toBeDefined()
expect(relevantCall![1].validate).toBeUndefined()

useFieldSpy.mockRestore()
})

it('should call validator when validate prop is provided', () => {
const fieldValidate = jest.fn(() => undefined)
const fieldArraySpy = jest.fn()

const MyFieldArray = () => {
const fieldArray = useFieldArray('names', { validate: fieldValidate })
fieldArraySpy(fieldArray)
Expand All @@ -126,11 +131,115 @@ describe('FieldArray', () => {

// Get the last call before mutations
const lastCallBeforeMutations = fieldArraySpy.mock.calls.length - 1

// Push an item to trigger validation again
act(() => fieldArraySpy.mock.calls[lastCallBeforeMutations][0].fields.push('alice'))
act(() =>
fieldArraySpy.mock.calls[lastCallBeforeMutations][0].fields.push('alice')
)

// Field validation should be called again after mutation
expect(fieldValidate.mock.calls.length).toBeGreaterThan(initialCalls)
})

it('should handle array errors', () => {
const spy = jest.fn()
const MyFieldArray = () => {
spy(useFieldArray('names', { validate: (values) => ['required'] }))
return null
}
render(
<Form
onSubmit={onSubmitMock}
mutators={arrayMutators as any}
subscription={{}}
>
{() => (
<form>
<MyFieldArray />
</form>
)}
</Form>
)

expect(spy).toHaveBeenCalledWith(
expect.objectContaining({
meta: expect.objectContaining({
error: ['required']
})
})
)
})

it('should handle string error', () => {
const spy = jest.fn()
const spyState = jest.fn()
const MyFieldArray = () => {
spy(useFieldArray('names', { validate: (values) => 'failed' }))
return null
}
const Debug = () => {
spyState(useFormState().errors)
return null
}
render(
<Form
onSubmit={onSubmitMock}
mutators={arrayMutators as any}
subscription={{}}
>
{() => (
<form>
<MyFieldArray />
<Debug />
</form>
)}
</Form>
)

expect(spy).toHaveBeenCalledWith(
expect.objectContaining({
meta: expect.objectContaining({
error: 'failed'
})
})
)
const expected: any[] = []
;(expected as any)[ARRAY_ERROR] = 'failed'

expect(spyState).toHaveBeenCalledWith({ names: expected })
})
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']
})
})
)
)
})
Comment on lines +211 to +244
Copy link
Copy Markdown

@coderabbitai coderabbitai bot Feb 24, 2026

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟡 Minor

🧩 Analysis chain

🏁 Script executed:

# First, find the test file
find . -name "useFieldArray.test.tsx" -type f

Repository: 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.tsx

Repository: 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.tsx

Repository: 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.tsx

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

Suggested change
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).

})
67 changes: 47 additions & 20 deletions src/useFieldArray.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import { useMemo } from 'react';
import { useMemo } from 'react'
import { useForm, useField } from 'react-final-form'
import { fieldSubscriptionItems, ARRAY_ERROR } from 'final-form'
import { Mutators } from 'final-form-arrays'
Expand All @@ -13,6 +13,21 @@ const all: FieldSubscription = fieldSubscriptionItems.reduce((result, key) => {
return result
}, {} as FieldSubscription)

/**
* handle synced errors
*/
const handleError = (error: string | readonly string[] | void) => {
if (!error || Array.isArray(error)) {
return error
}
const arrayError: string[] = []
// gross, but we have to set a string key on the array
;(arrayError as unknown as Record<string, string>)[ARRAY_ERROR] =
error as string

return arrayError
}

const useFieldArray = (
name: string,
{
Expand All @@ -26,33 +41,45 @@ const useFieldArray = (
const form = useForm('useFieldArray')

const formMutators = form.mutators as unknown as Mutators
const hasMutators = !!(formMutators && (formMutators as any).push && (formMutators as any).pop)
const hasMutators = !!(
formMutators &&
(formMutators as any).push &&
(formMutators as any).pop
)
if (!hasMutators) {
throw new Error(
'Array mutators not found. You need to provide the mutators from final-form-arrays to your form'
)
}
const mutators = useMemo<Record<string, Function>>(() =>
// curry the field name onto all mutator calls
Object.keys(formMutators).reduce((result, key) => {
result[key] = (...args: any[]) => (formMutators as any)[key](name, ...args)
return result
}, {} as Record<string, Function>
), [name, formMutators])
const mutators = useMemo<Record<string, Function>>(
() =>
// curry the field name onto all mutator calls
Object.keys(formMutators).reduce(
(result, key) => {
result[key] = (...args: any[]) =>
(formMutators as any)[key](name, ...args)
return result
},
{} as Record<string, Function>
),
[name, formMutators]
)

const validate: FieldValidator | undefined = useConstant(() =>
!validateProp
? undefined
: (value: any, allValues: any, meta: any) => {
const error = validateProp(value, allValues, meta)
if (!error || Array.isArray(error)) {
return error
} else {
const arrayError: any[] = []
// gross, but we have to set a string key on the array
; (arrayError as any)[ARRAY_ERROR] = error
return arrayError
const validation = validateProp(value, allValues, meta)
if (!validation) {
return undefined
}

if (validation.then) {
return validation.then((error: string | readonly string[] | void) =>
handleError(error)
)
}
return handleError(validation)
}
)

Expand All @@ -62,7 +89,7 @@ const useFieldArray = (
initialValue,
isEqual,
validate,
format: v => v
format: (v) => v
})

// FIX #167: Don't destructure/spread meta object because it has lazy getters
Expand All @@ -82,7 +109,7 @@ const useFieldArray = (
}
}

const map = <T,>(iterator: (name: string, index: number) => T): T[] => {
const map = <T>(iterator: (name: string, index: number) => T): T[] => {
// required || for Flow, but results in uncovered line in Jest/Istanbul
// istanbul ignore next
const len = length || 0
Expand Down Expand Up @@ -110,4 +137,4 @@ const useFieldArray = (
}
}

export default useFieldArray
export default useFieldArray