diff --git a/.changeset/red-hats-jam.md b/.changeset/red-hats-jam.md new file mode 100644 index 000000000..86caeedf3 --- /dev/null +++ b/.changeset/red-hats-jam.md @@ -0,0 +1,6 @@ +--- +'@tanstack/form-core': patch +'@tanstack/react-form': patch +--- + +fix(core): field unmount diff --git a/packages/form-core/src/FieldApi.ts b/packages/form-core/src/FieldApi.ts index f8b1b009e..6404ac7b8 100644 --- a/packages/form-core/src/FieldApi.ts +++ b/packages/form-core/src/FieldApi.ts @@ -1275,6 +1275,7 @@ export class FieldApi< /** * Mounts the field instance to the form. + * @returns A function to unmount the field instance. */ mount = () => { if (this.options.defaultValue !== undefined && !this.getMeta().isTouched) { @@ -1322,8 +1323,80 @@ export class FieldApi< fieldApi: this, }) - // TODO: Remove - return () => {} + if (!this.form.options.cleanupFieldsOnUnmount) { + return () => {} + } + + return () => { + // Stop any in-flight async validation or listener work tied to this instance. + for (const [key, timeout] of Object.entries( + this.timeoutIds.validations, + )) { + if (timeout) { + clearTimeout(timeout) + this.timeoutIds.validations[ + key as keyof typeof this.timeoutIds.validations + ] = null + } + } + for (const [key, timeout] of Object.entries(this.timeoutIds.listeners)) { + if (timeout) { + clearTimeout(timeout) + this.timeoutIds.listeners[ + key as keyof typeof this.timeoutIds.listeners + ] = null + } + } + for (const [key, timeout] of Object.entries( + this.timeoutIds.formListeners, + )) { + if (timeout) { + clearTimeout(timeout) + this.timeoutIds.formListeners[ + key as keyof typeof this.timeoutIds.formListeners + ] = null + } + } + + const fieldInfo = this.form.fieldInfo[this.name] + if (!fieldInfo) return + + // If a newer field instance has already been mounted for this name, + // avoid touching its shared validation state during teardown. + if (fieldInfo.instance !== this) return + + for (const [key, validationMeta] of Object.entries( + fieldInfo.validationMetaMap, + )) { + validationMeta?.lastAbortController.abort() + fieldInfo.validationMetaMap[ + key as keyof typeof fieldInfo.validationMetaMap + ] = undefined + } + + this.form.baseStore.setState((prev) => ({ + // Preserve interaction flags so field-level defaultValue does not + // reseed user-entered values on remount. + ...prev, + fieldMetaBase: { + ...prev.fieldMetaBase, + [this.name]: { + ...defaultFieldMeta, + isTouched: + prev.fieldMetaBase[this.name]?.isTouched ?? + defaultFieldMeta.isTouched, + isBlurred: + prev.fieldMetaBase[this.name]?.isBlurred ?? + defaultFieldMeta.isBlurred, + isDirty: + prev.fieldMetaBase[this.name]?.isDirty ?? + defaultFieldMeta.isDirty, + }, + }, + })) + + fieldInfo.instance = null + } } /** @@ -1790,12 +1863,13 @@ export class FieldApi< promises: Promise[], ) => { const errorMapKey = getErrorMapKey(validateObj.cause) - const fieldValidatorMeta = field.getInfo().validationMetaMap[errorMapKey] + const fieldInfo = field.getInfo() + const fieldValidatorMeta = fieldInfo.validationMetaMap[errorMapKey] fieldValidatorMeta?.lastAbortController.abort() const controller = new AbortController() - this.getInfo().validationMetaMap[errorMapKey] = { + fieldInfo.validationMetaMap[errorMapKey] = { lastAbortController: controller, } @@ -1804,11 +1878,11 @@ export class FieldApi< let rawError!: ValidationError | undefined try { rawError = await new Promise((rawResolve, rawReject) => { - if (this.timeoutIds.validations[validateObj.cause]) { - clearTimeout(this.timeoutIds.validations[validateObj.cause]!) + if (field.timeoutIds.validations[validateObj.cause]) { + clearTimeout(field.timeoutIds.validations[validateObj.cause]!) } - this.timeoutIds.validations[validateObj.cause] = setTimeout( + field.timeoutIds.validations[validateObj.cause] = setTimeout( async () => { if (controller.signal.aborted) return rawResolve(undefined) try { @@ -1838,7 +1912,9 @@ export class FieldApi< const fieldLevelError = normalizeError(rawError) const formLevelError = - asyncFormValidationResults[this.name]?.[errorMapKey] + asyncFormValidationResults[ + field.name as keyof typeof asyncFormValidationResults + ]?.[errorMapKey] const { newErrorValue, newSource } = determineFieldLevelErrorSourceAndValue({ @@ -1846,6 +1922,10 @@ export class FieldApi< fieldLevelError, }) + if (field.getInfo().instance !== field) { + return resolve(undefined) + } + field.setMeta((prev) => { return { ...prev, diff --git a/packages/form-core/src/FormApi.ts b/packages/form-core/src/FormApi.ts index d78c39a8a..e043a4db1 100644 --- a/packages/form-core/src/FormApi.ts +++ b/packages/form-core/src/FormApi.ts @@ -373,6 +373,11 @@ export interface FormOptions< * If true, allows the form to be submitted in an invalid state i.e. canSubmit will remain true regardless of validation errors. Defaults to undefined. */ canSubmitWhenInvalid?: boolean + /** + * If true, mounted fields clean up their validation state when they unmount. + * Defaults to false. + */ + cleanupFieldsOnUnmount?: boolean /** * A list of validators to pass to the form */ @@ -925,7 +930,7 @@ export class FormApi< /** * A record of field information for each field in the form. */ - fieldInfo: Record, FieldInfo> = {} as any + fieldInfo: Partial, FieldInfo>> = {} get state() { return this.store.state @@ -1603,7 +1608,6 @@ export class FormApi< field: TField, cause: ValidationCause, ) => { - // eslint-disable-next-line @typescript-eslint/no-unnecessary-condition const fieldInstance = this.fieldInfo[field]?.instance if (!fieldInstance) { @@ -2222,7 +2226,6 @@ export class FormApi< getFieldInfo = >( field: TField, ): FieldInfo => { - // eslint-disable-next-line @typescript-eslint/no-unnecessary-condition return (this.fieldInfo[field] ||= { instance: null, validationMetaMap: { diff --git a/packages/form-core/tests/FieldApi.spec.ts b/packages/form-core/tests/FieldApi.spec.ts index f9d099022..f1bbc587a 100644 --- a/packages/form-core/tests/FieldApi.spec.ts +++ b/packages/form-core/tests/FieldApi.spec.ts @@ -1603,6 +1603,385 @@ describe('field api', () => { expect(form.getFieldInfo(field.name)).toBeDefined() }) + it('should keep field meta on unmount by default', async () => { + const form = new FormApi({ + defaultValues: { + name: '', + }, + }) + + form.mount() + + const field = new FieldApi({ + form, + name: 'name', + validators: { + onSubmit: ({ value }) => + value.length > 0 ? undefined : 'name is required', + }, + }) + + const unmount = field.mount() + + await form.handleSubmit() + expect(form.state.fieldMeta.name?.errors).toContain('name is required') + + expect(unmount).toBeTypeOf('function') + unmount() + expect(form.state.fieldMeta.name?.errors).toContain('name is required') + expect(form.state.canSubmit).toBe(false) + }) + + it('should clear meta on unmount while preserving value', async () => { + const form = new FormApi({ + defaultValues: { + firstName: 'a', + lastName: 'abc', + }, + cleanupFieldsOnUnmount: true, + onSubmit: () => {}, + }) + + form.mount() + + const firstName = new FieldApi({ + form, + name: 'firstName', + }) + const lastName = new FieldApi({ + form, + name: 'lastName', + validators: { + onSubmit: ({ value }) => + value.length >= 5 ? undefined : 'last name must be at least 5 chars', + }, + }) + + firstName.mount() + const unmountLastName = lastName.mount() + + await form.handleSubmit() + expect(form.state.canSubmit).toBe(false) + expect(lastName.getMeta().errors).toContain( + 'last name must be at least 5 chars', + ) + + expect(unmountLastName).toBeTypeOf('function') + unmountLastName() + + expect(form.getFieldValue('lastName')).toBe('abc') + expect(form.state.fieldMeta.lastName).toMatchObject({ + isTouched: true, + isValid: true, + errors: [], + }) + expect(form.state.canSubmit).toBe(true) + + const remountedLastName = new FieldApi({ + form, + name: 'lastName', + validators: { + onSubmit: ({ value }) => + value.length >= 5 ? undefined : 'last name must be at least 5 chars', + }, + }) + + remountedLastName.mount() + expect(remountedLastName.getMeta().errors).toStrictEqual([]) + expect(remountedLastName.getMeta().isTouched).toBe(true) + expect(remountedLastName.getValue()).toBe('abc') + }) + + it('should preserve field-level defaultValue changes across unmount remount cleanup', () => { + const form = new FormApi({ + defaultValues: {} as { name?: string }, + cleanupFieldsOnUnmount: true, + }) + + form.mount() + + const field = new FieldApi({ + form, + name: 'name', + defaultValue: 'initial', + }) + + const unmount = field.mount() + field.setValue('changed') + expect(unmount).toBeTypeOf('function') + unmount() + + const remountedField = new FieldApi({ + form, + name: 'name', + defaultValue: 'initial', + }) + + remountedField.mount() + expect(remountedField.getValue()).toBe('changed') + }) + + it('should not apply in-flight async validation results after unmount', async () => { + vi.useFakeTimers() + + let resolveValidation!: () => void + const validationPromise = new Promise((resolve) => { + resolveValidation = resolve + }) + + const form = new FormApi({ + defaultValues: { + name: '', + }, + cleanupFieldsOnUnmount: true, + }) + + form.mount() + + const field = new FieldApi({ + form, + name: 'name', + validators: { + onChangeAsyncDebounceMs: 0, + onChangeAsync: async () => { + await validationPromise + return 'async error should be ignored after unmount' + }, + }, + }) + + const unmount = field.mount() + + field.setValue('trigger') + await vi.runAllTimersAsync() + + expect(unmount).toBeTypeOf('function') + unmount() + resolveValidation() + await vi.runAllTimersAsync() + + expect(form.state.fieldMeta.name).toMatchObject({ + isTouched: true, + isValid: true, + errors: [], + }) + + vi.useRealTimers() + }) + + it('should cancel debounced field and form listeners on unmount', async () => { + vi.useFakeTimers() + + const fieldListener = vi.fn() + const formListener = vi.fn() + + const form = new FormApi({ + defaultValues: { + name: '', + }, + cleanupFieldsOnUnmount: true, + listeners: { + onChange: formListener, + onChangeDebounceMs: 200, + }, + }) + + form.mount() + + const field = new FieldApi({ + form, + name: 'name', + listeners: { + onChange: fieldListener, + onChangeDebounceMs: 200, + }, + }) + + const unmount = field.mount() + field.setValue('trigger') + expect(unmount).toBeTypeOf('function') + unmount() + + await vi.advanceTimersByTimeAsync(500) + + expect(fieldListener).toHaveBeenCalledTimes(0) + expect(formListener).toHaveBeenCalledTimes(0) + + vi.useRealTimers() + }) + + it('should ignore cleanup when fieldInfo was deleted before unmount', () => { + const form = new FormApi({ + defaultValues: { + name: '', + }, + cleanupFieldsOnUnmount: true, + }) + + form.mount() + + const field = new FieldApi({ + form, + name: 'name', + }) + + const unmount = field.mount() + form.deleteField('name') + + expect(unmount).toBeTypeOf('function') + expect(() => unmount()).not.toThrow() + }) + + it('should not clear newer instance state when older instance unmounts', () => { + const form = new FormApi({ + defaultValues: { + name: '', + }, + cleanupFieldsOnUnmount: true, + }) + + form.mount() + + const oldField = new FieldApi({ + form, + name: 'name', + }) + const oldUnmount = oldField.mount() + + const newField = new FieldApi({ + form, + name: 'name', + }) + newField.mount() + newField.setValue('new value') + + expect(oldUnmount).toBeTypeOf('function') + oldUnmount() + + expect(form.getFieldInfo('name').instance).toBe(newField) + expect(newField.getValue()).toBe('new value') + expect(newField.getMeta().isTouched).toBe(true) + }) + + it('should not cancel newer instance async validation when older instance unmounts', async () => { + vi.useFakeTimers() + + const form = new FormApi({ + defaultValues: { + name: '', + }, + cleanupFieldsOnUnmount: true, + }) + + form.mount() + + const oldField = new FieldApi({ + form, + name: 'name', + }) + const oldUnmount = oldField.mount() + + const newField = new FieldApi({ + form, + name: 'name', + validators: { + onChangeAsyncDebounceMs: 10, + onChangeAsync: async ({ value }) => + value === 'taken' ? 'name is taken' : undefined, + }, + }) + + newField.mount() + newField.setValue('taken') + + expect(oldUnmount).toBeTypeOf('function') + oldUnmount() + + await vi.runAllTimersAsync() + + expect(newField.getMeta().errors).toContain('name is taken') + + vi.useRealTimers() + }) + + it('should ignore stale async validation results from an older remounted instance', async () => { + vi.useFakeTimers() + + let resolve!: () => void + const promise = new Promise((r) => { + resolve = r as never + }) + + const form = new FormApi({ + defaultValues: { + name: '', + }, + cleanupFieldsOnUnmount: true, + }) + + form.mount() + + const oldField = new FieldApi({ + form, + name: 'name', + validators: { + onChangeAsyncDebounceMs: 0, + onChangeAsync: async () => { + await promise + return 'stale error' + }, + }, + }) + + oldField.mount() + oldField.setValue('taken') + await vi.runAllTimersAsync() + + const newField = new FieldApi({ + form, + name: 'name', + }) + newField.mount() + + resolve() + await vi.runAllTimersAsync() + + expect(newField.getMeta().errors).toStrictEqual([]) + + vi.useRealTimers() + }) + + it('should surface thrown async validator errors', async () => { + vi.useFakeTimers() + + const form = new FormApi({ + defaultValues: { + name: '', + }, + }) + + form.mount() + + const field = new FieldApi({ + form, + name: 'name', + validators: { + onChangeAsyncDebounceMs: 0, + onChangeAsync: async () => { + throw 'async validation failed' + }, + }, + }) + + field.mount() + field.setValue('test') + await vi.runAllTimersAsync() + + expect(field.getMeta().errors).toContain('async validation failed') + + vi.useRealTimers() + }) + it('should show onSubmit errors', async () => { const form = new FormApi({ defaultValues: { @@ -1950,6 +2329,61 @@ describe('field api', () => { ]) }) + it('should cancel linked field async validation when the target field unmounts', async () => { + vi.useFakeTimers() + + let resolve!: () => void + const promise = new Promise((r) => { + resolve = r as never + }) + + const form = new FormApi({ + defaultValues: { + password: '', + confirm_password: '', + }, + cleanupFieldsOnUnmount: true, + }) + + form.mount() + + const passField = new FieldApi({ + form, + name: 'password', + }) + + const passconfirmField = new FieldApi({ + form, + name: 'confirm_password', + validators: { + onChangeListenTo: ['password'], + onChangeAsyncDebounceMs: 0, + onChangeAsync: async () => { + await promise + return 'Passwords do not match' + }, + }, + }) + + passField.mount() + const unmount = passconfirmField.mount() + + passField.setValue('one') + await vi.runAllTimersAsync() + + expect(unmount).toBeTypeOf('function') + unmount() + resolve() + await vi.runAllTimersAsync() + + expect(form.state.fieldMeta.confirm_password).toMatchObject({ + errors: [], + isValid: true, + }) + + vi.useRealTimers() + }) + it('should add a new value to the fieldApi errorMap', () => { interface Form { name: string diff --git a/packages/react-form/tests/useField.test.tsx b/packages/react-form/tests/useField.test.tsx index 861ad9df5..9d74bf2e3 100644 --- a/packages/react-form/tests/useField.test.tsx +++ b/packages/react-form/tests/useField.test.tsx @@ -328,6 +328,108 @@ describe('useField', () => { expect((getByTestId('first-field') as HTMLInputElement).value).toBe('hello') }) + it('should not keep hidden field submit errors after unmount', async () => { + const onSubmit = vi.fn() + + function Comp() { + const form = useForm({ + defaultValues: { + firstName: '', + lastName: '', + }, + cleanupFieldsOnUnmount: true, + onSubmit: ({ value }) => onSubmit(value), + }) + + return ( +
{ + e.preventDefault() + e.stopPropagation() + form.handleSubmit() + }} + > + + {(field) => ( + field.handleChange(e.target.value)} + /> + )} + + + state.values.firstName === 'a'}> + {(showLastName) => + showLastName ? ( + + value.length >= 5 ? undefined : 'lastName too short', + }} + > + {(field) => ( + field.handleChange(e.target.value)} + /> + )} + + ) : null + } + + + [state.canSubmit, state.isSubmitting]} + > + {([canSubmit, isSubmitting]) => ( + + )} + +
+ ) + } + + const { getByTestId, queryByTestId } = render( + + + , + ) + + const submitButton = getByTestId('submit') + + await user.type(getByTestId('first-name'), 'a') + await user.type(getByTestId('last-name'), 'abc') + await user.click(submitButton) + + await waitFor(() => expect(submitButton).toBeDisabled()) + expect(onSubmit).toHaveBeenCalledTimes(0) + + await user.clear(getByTestId('first-name')) + await user.type(getByTestId('first-name'), 'b') + + await waitFor(() => + expect(queryByTestId('last-name')).not.toBeInTheDocument(), + ) + await waitFor(() => expect(submitButton).toBeEnabled()) + + await user.click(submitButton) + await waitFor(() => expect(onSubmit).toHaveBeenCalledTimes(1)) + + await user.clear(getByTestId('first-name')) + await user.type(getByTestId('first-name'), 'a') + + const remountedLastName = await waitFor(() => getByTestId('last-name')) + expect((remountedLastName as HTMLInputElement).value).toBe('abc') + expect(submitButton).toBeEnabled() + }) + it('should validate async on change', async () => { type Person = { firstName: string