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

perf(react-form): skip updating isValidating by default #1291

Draft
wants to merge 2 commits into
base: main
Choose a base branch
from
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
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
18 changes: 14 additions & 4 deletions packages/form-core/src/FieldApi.ts
Original file line number Diff line number Diff line change
Expand Up @@ -439,6 +439,10 @@ export interface FieldOptions<
* Disable the `flat(1)` operation on `field.errors`. This is useful if you want to keep the error structure as is. Not suggested for most use-cases.
*/
disableErrorFlat?: boolean
/**
* Should the field's isValidating state be updated during validation
*/
trackValidationState?: boolean
}

/**
Expand Down Expand Up @@ -1473,12 +1477,14 @@ export class FieldApi<
>,
)

if (!this.state.meta.isValidating) {
if (!this.state.meta.isValidating && this.options.trackValidationState) {
this.setMeta((prev) => ({ ...prev, isValidating: true }))
}

for (const linkedField of linkedFields) {
linkedField.setMeta((prev) => ({ ...prev, isValidating: true }))
if (linkedField.options.trackValidationState) {
linkedField.setMeta((prev) => ({ ...prev, isValidating: true }))
}
}

/**
Expand Down Expand Up @@ -1577,10 +1583,14 @@ export class FieldApi<
await Promise.all(linkedPromises)
}

this.setMeta((prev) => ({ ...prev, isValidating: false }))
if (this.options.trackValidationState) {
this.setMeta((prev) => ({ ...prev, isValidating: false }))
}

for (const linkedField of linkedFields) {
linkedField.setMeta((prev) => ({ ...prev, isValidating: false }))
if (linkedField.options.trackValidationState) {
linkedField.setMeta((prev) => ({ ...prev, isValidating: false }))
}
}

return results.filter(Boolean)
Expand Down
61 changes: 60 additions & 1 deletion packages/react-form/tests/useField.test.tsx
Copy link
Author

@MVaik MVaik Mar 16, 2025

Choose a reason for hiding this comment

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

"should properly update conditionally rendered fields" is now failing after this change, the "hello" that is written into firstField somehow ends up in secondField after the checkbox is toggled and firstField is no longer shown. This turns secondField into "helloworld" instead of "world".

Original file line number Diff line number Diff line change
Expand Up @@ -290,7 +290,6 @@ describe('useField', () => {
</form.Field>
)
}

return (
<form.Field name="secondField">
{({ handleChange, state }) => (
Expand Down Expand Up @@ -1130,4 +1129,64 @@ describe('useField', () => {
// field2 should not have rerendered
expect(renderCount.field2).toBe(field2InitialRender)
})

it('should render once per change if not tracking validation state', async () => {
let regularRenders = 0
let rendersWithTracking = 0
const inputText = 'example'
const expectedRenders = inputText.length
// 1 by default and seems like 1 for useStore
const baseRenders = 2

function Comp() {
const form = useForm({
defaultValues: {
field1: '',
field2: '',
},
})

return (
<>
<form.Field
name="field1"
children={(field) => {
regularRenders++
return (
<input
data-testid="fieldinput1"
value={field.state.value}
onBlur={field.handleBlur}
onChange={(e) => field.handleChange(e.target.value)}
/>
)
}}
/>
<form.Field
name="field2"
trackValidationState
children={(field) => {
rendersWithTracking++
return (
<input
data-testid="fieldinput2"
value={field.state.value}
onBlur={field.handleBlur}
onChange={(e) => field.handleChange(e.target.value)}
/>
)
}}
/>
</>
)
}

const { getByTestId } = render(<Comp />)
await user.type(getByTestId('fieldinput1'), inputText)
expect(regularRenders).toEqual(expectedRenders + baseRenders)

await user.type(getByTestId('fieldinput2'), inputText)
// Each change triggers two renders due to isValidating state being updated
expect(rendersWithTracking).toEqual(expectedRenders * 2 + baseRenders)
})
})