Skip to content

Commit

Permalink
fix: onSubmit should now behave as expected
Browse files Browse the repository at this point in the history
* fix: memoize all non-function form props by default

* chore: migrate useStableFormOpts to useMemo

* fix: options passed to useField will not always cause a re-render

* chore: minor code cleanups

* test: add previously failing test to demonstrate fix

* chore: fix linting

* fix: running form.update repeatedly should not wipe form state

* test: add test to validate formapi update behavior

* test: add initial tests for useStableOptions

* chore: remove useStableOpts and useFormCallback
  • Loading branch information
crutchcorn committed Sep 7, 2023
1 parent 9424ed9 commit 6f76fee
Show file tree
Hide file tree
Showing 11 changed files with 171 additions and 60 deletions.
9 changes: 3 additions & 6 deletions docs/framework/react/quick-start.md
Original file line number Diff line number Diff line change
Expand Up @@ -13,12 +13,9 @@ import { useForm } from '@tanstack/react-form'
export default function App() {
const form = useForm({
// Memoize your default values to prevent re-renders
defaultValues: React.useMemo(
() => ({
fullName: '',
}),
[],
),
defaultValues: {
fullName: '',
},
onSubmit: async (values) => {
// Do something with form data
console.log(values)
Expand Down
12 changes: 4 additions & 8 deletions docs/overview.md
Original file line number Diff line number Diff line change
Expand Up @@ -44,14 +44,10 @@ function FieldInfo({ field }: { field: FieldApi<any, any> }) {

export default function App() {
const form = useForm({
// Memoize your default values to prevent re-renders
defaultValues: React.useMemo(
() => ({
firstName: '',
lastName: '',
}),
[],
),
defaultValues: {
firstName: '',
lastName: '',
},
onSubmit: async (values) => {
// Do something with form data
console.log(values)
Expand Down
11 changes: 4 additions & 7 deletions examples/react/simple/src/index.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -17,13 +17,10 @@ function FieldInfo({ field }: { field: FieldApi<any, any> }) {
export default function App() {
const form = useForm({
// Memoize your default values to prevent re-renders
defaultValues: React.useMemo(
() => ({
firstName: "",
lastName: "",
}),
[],
),
defaultValues: {
firstName: "",
lastName: "",
},
onSubmit: async (values) => {
// Do something with form data
console.log(values);
Expand Down
36 changes: 19 additions & 17 deletions packages/form-core/src/FormApi.ts
Original file line number Diff line number Diff line change
Expand Up @@ -69,21 +69,20 @@ function getDefaultFormState<TData>(
defaultState: Partial<FormState<TData>>,
): FormState<TData> {
return {
values: {} as any,
fieldMeta: {} as any,
canSubmit: true,
isFieldsValid: false,
isFieldsValidating: false,
isFormValid: false,
isFormValidating: false,
isSubmitted: false,
isSubmitting: false,
isTouched: false,
isValid: false,
isValidating: false,
submissionAttempts: 0,
formValidationCount: 0,
...defaultState,
values: defaultState.values ?? ({} as never),
fieldMeta: defaultState.fieldMeta ?? ({} as never),
canSubmit: defaultState.canSubmit ?? true,
isFieldsValid: defaultState.isFieldsValid ?? false,
isFieldsValidating: defaultState.isFieldsValidating ?? false,
isFormValid: defaultState.isFormValid ?? false,
isFormValidating: defaultState.isFormValidating ?? false,
isSubmitted: defaultState.isSubmitted ?? false,
isSubmitting: defaultState.isSubmitting ?? false,
isTouched: defaultState.isTouched ?? false,
isValid: defaultState.isValid ?? false,
isValidating: defaultState.isValidating ?? false,
submissionAttempts: defaultState.submissionAttempts ?? 0,
formValidationCount: defaultState.formValidationCount ?? 0,
}
}

Expand Down Expand Up @@ -156,15 +155,18 @@ export class FormApi<TFormData> {
this.store.batch(() => {
const shouldUpdateValues =
options.defaultValues &&
options.defaultValues !== this.options.defaultValues
options.defaultValues !== this.options.defaultValues &&
!this.state.isTouched

const shouldUpdateState =
options.defaultState !== this.options.defaultState
options.defaultState !== this.options.defaultState &&
!this.state.isTouched

this.store.setState(() =>
getDefaultFormState(
Object.assign(
{},
this.state,
// eslint-disable-next-line @typescript-eslint/no-unnecessary-condition
shouldUpdateState ? options.defaultState : {},
// eslint-disable-next-line @typescript-eslint/no-unnecessary-condition
Expand Down
54 changes: 54 additions & 0 deletions packages/form-core/src/tests/FormApi.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -213,4 +213,58 @@ describe('form api', () => {

expect(form.getFieldValue('names')).toStrictEqual(['one', 'three', 'two'])
})

it('should not wipe values when updating', () => {
const form = new FormApi({
defaultValues: {
name: 'test',
},
})

form.setFieldValue('name', 'other')

expect(form.getFieldValue('name')).toEqual('other')

form.update()

expect(form.getFieldValue('name')).toEqual('other')
})

it('should wipe default values when not touched', () => {
const form = new FormApi({
defaultValues: {
name: 'test',
},
})

expect(form.getFieldValue('name')).toEqual('test')

form.update({
defaultValues: {
name: 'other',
},
})

expect(form.getFieldValue('name')).toEqual('other')
})

it('should not wipe default values when touched', () => {
const form = new FormApi({
defaultValues: {
name: 'one',
},
})

expect(form.getFieldValue('name')).toEqual('one')

form.setFieldValue('name', 'two', { touch: true })

form.update({
defaultValues: {
name: 'three',
},
})

expect(form.getFieldValue('name')).toEqual('two')
})
})
50 changes: 48 additions & 2 deletions packages/react-form/src/tests/useForm.test.tsx
Original file line number Diff line number Diff line change
@@ -1,9 +1,9 @@
/// <reference lib="dom" />
import { render } from '@testing-library/react'
import { render, waitFor } from '@testing-library/react'
import userEvent from '@testing-library/user-event'
import '@testing-library/jest-dom'
import * as React from 'react'
import { createFormFactory } from '..'
import { createFormFactory, useForm } from '..'

const user = userEvent.setup()

Expand Down Expand Up @@ -78,4 +78,50 @@ describe('useForm', () => {
expect(await findByText('FirstName')).toBeInTheDocument()
expect(queryByText('LastName')).not.toBeInTheDocument()
})

it('should handle submitting properly', async () => {
function Comp() {
const [submittedData, setSubmittedData] = React.useState<{
firstName: string
} | null>(null)

const form = useForm({
defaultValues: {
firstName: 'FirstName',
},
onSubmit: (data) => {
setSubmittedData(data)
},
})

return (
<form.Provider>
<form.Field
name="firstName"
children={(field) => {
return (
<input
value={field.state.value}
onBlur={field.handleBlur}
onChange={(e) => field.handleChange(e.target.value)}
placeholder={'First name'}
/>
)
}}
/>
<button onClick={form.handleSubmit}>Submit</button>
{submittedData && <p>Submitted data: {submittedData.firstName}</p>}
</form.Provider>
)
}

const { findByPlaceholderText, getByText } = render(<Comp />)
const input = await findByPlaceholderText('First name')
await user.clear(input)
await user.type(input, 'OtherName')
await user.click(getByText('Submit'))
await waitFor(() =>
expect(getByText('Submitted data: OtherName')).toBeInTheDocument(),
)
})
})
8 changes: 8 additions & 0 deletions packages/react-form/src/types.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
import type { FieldOptions } from '@tanstack/form-core'

export type UseFieldOptions<TData, TFormData> = FieldOptions<
TData,
TFormData
> & {
mode?: 'value' | 'array'
}
25 changes: 12 additions & 13 deletions packages/react-form/src/useField.tsx
Original file line number Diff line number Diff line change
@@ -1,5 +1,4 @@
import * as React from 'react'
//
import React, { useState } from 'react'
import { useStore } from '@tanstack/react-store'
import type {
DeepKeys,
Expand All @@ -9,6 +8,8 @@ import type {
} from '@tanstack/form-core'
import { FieldApi, functionalUpdate } from '@tanstack/form-core'
import { useFormContext, formContext } from './formContext'
import { useIsomorphicLayoutEffect } from './utils/useIsomorphicLayoutEffect'
import type { UseFieldOptions } from './types'

declare module '@tanstack/form-core' {
// eslint-disable-next-line no-shadow
Expand All @@ -17,13 +18,6 @@ declare module '@tanstack/form-core' {
}
}

export type UseFieldOptions<TData, TFormData> = FieldOptions<
TData,
TFormData
> & {
mode?: 'value' | 'array'
}

export type UseField<TFormData> = <TField extends DeepKeys<TFormData>>(
opts?: { name: Narrow<TField> } & UseFieldOptions<
DeepValue<TFormData, TField>,
Expand All @@ -37,7 +31,7 @@ export function useField<TData, TFormData>(
// Get the form API either manually or from context
const { formApi, parentFieldName } = useFormContext()

const [fieldApi] = React.useState<FieldApi<TData, TFormData>>(() => {
const [fieldApi] = useState<FieldApi<TData, TFormData>>(() => {
const name = (
typeof opts.index === 'number'
? [parentFieldName, opts.index, opts.name]
Expand All @@ -53,8 +47,13 @@ export function useField<TData, TFormData>(
return api
})

// Keep options up to date as they are rendered
fieldApi.update({ ...opts, form: formApi } as never)
/**
* fieldApi.update should not have any side effects. Think of it like a `useRef`
* that we need to keep updated every render with the most up-to-date information.
*/
useIsomorphicLayoutEffect(() => {
fieldApi.update({ ...opts, form: formApi } as never)
})

useStore(
fieldApi.store as any,
Expand All @@ -66,7 +65,7 @@ export function useField<TData, TFormData>(
)

// Instantiates field meta and removes it when unrendered
React.useEffect(() => fieldApi.mount(), [fieldApi])
useIsomorphicLayoutEffect(() => fieldApi.mount(), [fieldApi])

return fieldApi
}
Expand Down
17 changes: 10 additions & 7 deletions packages/react-form/src/useForm.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -2,9 +2,10 @@ import type { FormState, FormOptions } from '@tanstack/form-core'
import { FormApi, functionalUpdate } from '@tanstack/form-core'
import type { NoInfer } from '@tanstack/react-store'
import { useStore } from '@tanstack/react-store'
import React from 'react'
import React, { type ReactNode, useState } from 'react'
import { type UseField, type FieldComponent, Field, useField } from './useField'
import { formContext } from './formContext'
import { useIsomorphicLayoutEffect } from './utils/useIsomorphicLayoutEffect'

declare module '@tanstack/form-core' {
// eslint-disable-next-line no-shadow
Expand All @@ -17,15 +18,13 @@ declare module '@tanstack/form-core' {
) => TSelected
Subscribe: <TSelected = NoInfer<FormState<TFormData>>>(props: {
selector?: (state: NoInfer<FormState<TFormData>>) => TSelected
children:
| ((state: NoInfer<TSelected>) => React.ReactNode)
| React.ReactNode
children: ((state: NoInfer<TSelected>) => ReactNode) | ReactNode
}) => any
}
}

export function useForm<TData>(opts?: FormOptions<TData>): FormApi<TData> {
const [formApi] = React.useState(() => {
const [formApi] = useState(() => {
// @ts-ignore
const api = new FormApi<TData>(opts)

Expand Down Expand Up @@ -58,9 +57,13 @@ export function useForm<TData>(opts?: FormOptions<TData>): FormApi<TData> {

formApi.useStore((state) => state.isSubmitting)

React.useEffect(() => {
/**
* formApi.update should not have any side effects. Think of it like a `useRef`
* that we need to keep updated every render with the most up-to-date information.
*/
useIsomorphicLayoutEffect(() => {
formApi.update(opts)
}, [formApi, opts])
})

return formApi as any
}
3 changes: 3 additions & 0 deletions packages/react-form/src/utils/isBrowser.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
/* c8 ignore start */
export const isBrowser = typeof window !== 'undefined'
/* c8 ignore end */
6 changes: 6 additions & 0 deletions packages/react-form/src/utils/useIsomorphicLayoutEffect.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
/* c8 ignore start */
import { useEffect, useLayoutEffect } from 'react'
import { isBrowser } from './isBrowser'

export const useIsomorphicLayoutEffect = isBrowser ? useLayoutEffect : useEffect
/* c8 ignore end */

0 comments on commit 6f76fee

Please sign in to comment.