From 04e5eb007095632db20bc53ee269e513fa61e551 Mon Sep 17 00:00:00 2001 From: JIHOON LEE Date: Thu, 21 Aug 2025 18:18:50 +0900 Subject: [PATCH 1/3] fix: move canSubmit validation after field validation in form submission flow --- packages/form-core/src/FormApi.ts | 5 +- packages/form-core/tests/FormApi.spec.ts | 323 +++++++++++++++++++++++ 2 files changed, 324 insertions(+), 4 deletions(-) diff --git a/packages/form-core/src/FormApi.ts b/packages/form-core/src/FormApi.ts index 78e7fa8a4..aa12b6fb5 100644 --- a/packages/form-core/src/FormApi.ts +++ b/packages/form-core/src/FormApi.ts @@ -1915,8 +1915,6 @@ export class FormApi< ) }) - if (!this.state.canSubmit) return - const submitMetaArg = submitMeta ?? (this.options.onSubmitMeta as TSubmitMeta) @@ -1940,8 +1938,7 @@ export class FormApi< await this.validate('submit') - // Fields are invalid, do not submit - if (!this.state.isValid) { + if (!this.state.canSubmit) { done() this.options.onSubmitInvalid?.({ value: this.state.values, diff --git a/packages/form-core/tests/FormApi.spec.ts b/packages/form-core/tests/FormApi.spec.ts index 1725f02be..c6b697a0f 100644 --- a/packages/form-core/tests/FormApi.spec.ts +++ b/packages/form-core/tests/FormApi.spec.ts @@ -3954,3 +3954,326 @@ it('should accept formId and return it', () => { expect(form.formId).toEqual('age') }) + +describe('onSubmitInvalid callback', () => { + it('should call onSubmitInvalid when canSubmit is false', async () => { + const onInvalid = vi.fn() + const form = new FormApi({ + defaultValues: { name: '' }, + validators: { + onMount: ({ value }) => value.name ? undefined : 'Name is required', + }, + onSubmitInvalid: ({ value, formApi, meta }) => { + onInvalid(value, formApi, meta) + } + }) + + form.mount() + expect(form.state.canSubmit).toBe(false) + + await form.handleSubmit() + + expect(onInvalid).toHaveBeenCalledTimes(1) + expect(onInvalid).toHaveBeenCalledWith( + { name: '' }, + form, + undefined + ) + }) + + it('should call onSubmitInvalid with updated error state after validation', async () => { + const onInvalid = vi.fn() + const form = new FormApi({ + defaultValues: { name: '', email: '' }, + validators: { + onSubmit: ({ value }) => { + const errors = [] + if (!value.name) errors.push('Name is required') + if (!value.email) errors.push('Email is required') + return errors.length > 0 ? errors.join(', ') : undefined + }, + }, + onSubmitInvalid: ({ value, formApi, meta }) => { + onInvalid(value, formApi.state.errors, meta) + } + }) + + form.mount() + await form.handleSubmit() + + expect(onInvalid).toHaveBeenCalledTimes(1) + expect(onInvalid).toHaveBeenCalledWith( + { name: '', email: '' }, + ['Name is required, Email is required'], + undefined + ) + }) + + it('should call onSubmitInvalid when field validation fails', async () => { + const onInvalid = vi.fn() + const form = new FormApi({ + defaultValues: { name: 'test', email: '' }, + onSubmitInvalid: ({ value, formApi, meta }) => { + onInvalid(value, formApi.state.isFieldsValid, formApi.state.fieldMeta) + } + }) + + const emailField = new FieldApi({ + form, + name: 'email', + validators: { + onSubmit: ({ value }) => !value ? 'Email is required' : undefined, + }, + }) + + form.mount() + emailField.mount() + + await form.handleSubmit() + + expect(onInvalid).toHaveBeenCalledTimes(1) + expect(onInvalid).toHaveBeenCalledWith( + { name: 'test', email: '' }, + false, + expect.objectContaining({ + email: expect.objectContaining({ + errors: ['Email is required'] + }) + }) + ) + }) + + it('should call onSubmitInvalid with correct formApi instance', async () => { + const onInvalid = vi.fn() + const form = new FormApi({ + defaultValues: { name: '' }, + validators: { + onSubmit: ({ value }) => !value.name ? 'Name is required' : undefined, + }, + onSubmitInvalid: ({ value, formApi, meta }) => { + onInvalid(formApi === form, formApi.state.values) + } + }) + + form.mount() + await form.handleSubmit() + + expect(onInvalid).toHaveBeenCalledWith(true, { name: '' }) + }) + + it('should call onSubmitInvalid with meta parameter when provided', async () => { + const onInvalid = vi.fn() + const customMeta = { source: 'test' } + const form = new FormApi({ + defaultValues: { name: '' }, + validators: { + onSubmit: ({ value }) => !value.name ? 'Name is required' : undefined, + }, + onSubmitInvalid: ({ value, formApi, meta }) => { + onInvalid(meta) + } + }) + + form.mount() + await (form.handleSubmit as any)(customMeta) + + expect(onInvalid).toHaveBeenCalledWith(customMeta) + }) + + it('should handle multiple validation errors correctly in onSubmitInvalid', async () => { + const onInvalid = vi.fn() + const form = new FormApi({ + defaultValues: { name: '', email: '', age: 0 }, + onSubmitInvalid: ({ value, formApi, meta }) => { + onInvalid(formApi.state.fieldMeta) + } + }) + + const nameField = new FieldApi({ + form, + name: 'name', + validators: { + onSubmit: ({ value }) => !value ? 'Name is required' : undefined, + }, + }) + + const emailField = new FieldApi({ + form, + name: 'email', + validators: { + onSubmit: ({ value }) => !value ? 'Email is required' : undefined, + }, + }) + + const ageField = new FieldApi({ + form, + name: 'age', + validators: { + onSubmit: ({ value }) => value < 18 ? 'Must be 18 or older' : undefined, + }, + }) + + form.mount() + nameField.mount() + emailField.mount() + ageField.mount() + + await form.handleSubmit() + + expect(onInvalid).toHaveBeenCalledTimes(1) + expect(onInvalid).toHaveBeenCalledWith( + expect.objectContaining({ + name: expect.objectContaining({ + errors: ['Name is required'] + }), + email: expect.objectContaining({ + errors: ['Email is required'] + }), + age: expect.objectContaining({ + errors: ['Must be 18 or older'] + }) + }) + ) + }) + + it('should call onSubmitInvalid when both onMount and onSubmit validators fail', async () => { + const onInvalid = vi.fn() + const form = new FormApi({ + defaultValues: { name: '', email: 'invalid' }, + validators: { + onMount: ({ value }) => !value.name ? 'Name is required on mount' : undefined, + onSubmit: ({ value }) => !value.email.includes('@') ? 'Invalid email format' : undefined, + }, + onSubmitInvalid: ({ value, formApi, meta }) => { + onInvalid(formApi.state.errors, formApi.state.canSubmit) + } + }) + + form.mount() + expect(form.state.canSubmit).toBe(false) + + await form.handleSubmit() + + expect(onInvalid).toHaveBeenCalledTimes(1) + expect(onInvalid).toHaveBeenCalledWith( + ['Name is required on mount', 'Invalid email format'], + false + ) + }) + + it('should set isSubmitting to false when onSubmitInvalid is called', async () => { + const onInvalid = vi.fn() + const form = new FormApi({ + defaultValues: { name: '' }, + validators: { + onSubmit: ({ value }) => !value.name ? 'Name is required' : undefined, + }, + onSubmitInvalid: ({ value, formApi, meta }) => { + onInvalid(formApi.state.isSubmitting) + } + }) + + form.mount() + + expect(form.state.isSubmitting).toBe(false) + + const submitPromise = form.handleSubmit() + expect(form.state.isSubmitting).toBe(true) + + await submitPromise + + expect(onInvalid).toHaveBeenCalledWith(false) + expect(form.state.isSubmitting).toBe(false) + }) + + it('should not call onSubmitInvalid when form is valid and submits successfully', async () => { + const onInvalid = vi.fn() + const onSubmit = vi.fn() + const form = new FormApi({ + onSubmit: ({ value }) => { + onSubmit(value) + }, + onSubmitInvalid: ({ value }) => { + onInvalid(value) + } + }) + + await form.handleSubmit() + + expect(onSubmit).toHaveBeenCalledTimes(1) + expect(onInvalid).not.toHaveBeenCalled() + }) + + it('should call onSubmitInvalid even when canSubmit is false due to onMount validation', async () => { + const onSubmitInvalid = vi.fn() + const form = new FormApi({ + defaultValues: { name: '' }, + validators: { + onMount: () => 'Form error on mount', + }, + onSubmitInvalid, + }) + + form.mount() + expect(form.state.canSubmit).toBe(false) + + await form.handleSubmit() + + expect(onSubmitInvalid).toHaveBeenCalledWith({ + value: { name: '' }, + formApi: form, + meta: undefined, + }) + expect(form.state.isSubmitting).toBe(false) + }) + + it('should NOT call onSubmitInvalid when canSubmitWhenInvalid is true and form is invalid', async () => { + const onSubmitInvalid = vi.fn() + const onSubmit = vi.fn() + const form = new FormApi({ + defaultValues: { name: '' }, + canSubmitWhenInvalid: true, + validators: { + onSubmit: () => 'Form error on submit', + }, + onSubmitInvalid, + onSubmit, + }) + + form.mount() + expect(form.state.canSubmit).toBe(true) // canSubmitWhenInvalid allows submission + + await form.handleSubmit() + + // With canSubmitWhenInvalid: true, onSubmit should be called even if invalid + expect(onSubmit).toHaveBeenCalled() + expect(onSubmitInvalid).not.toHaveBeenCalled() + expect(form.state.isSubmitting).toBe(false) + }) + + + it('should call onSubmit when canSubmitWhenInvalid is true even with onMount errors', async () => { + const onSubmitInvalid = vi.fn() + const onSubmit = vi.fn() + const form = new FormApi({ + defaultValues: { name: '' }, + canSubmitWhenInvalid: true, + validators: { + onMount: () => 'Mount error but submission still allowed', + }, + onSubmitInvalid, + onSubmit, + }) + + form.mount() + // With canSubmitWhenInvalid: true, canSubmit is always true regardless of errors + expect(form.state.canSubmit).toBe(true) + expect(form.state.isValid).toBe(false) // Form is invalid due to onMount error + + await form.handleSubmit() + + // With canSubmitWhenInvalid: true, onSubmit should be called even with errors + expect(onSubmit).toHaveBeenCalled() + expect(onSubmitInvalid).not.toHaveBeenCalled() + }) +}) From 9d30c9488233806b86c1b6bc80e33fa2f2ec1538 Mon Sep 17 00:00:00 2001 From: JIHOON LEE Date: Thu, 21 Aug 2025 18:23:36 +0900 Subject: [PATCH 2/3] refactor: clean up test comments for form submission behavior --- packages/form-core/tests/FormApi.spec.ts | 10 ++++------ 1 file changed, 4 insertions(+), 6 deletions(-) diff --git a/packages/form-core/tests/FormApi.spec.ts b/packages/form-core/tests/FormApi.spec.ts index c6b697a0f..da0c03268 100644 --- a/packages/form-core/tests/FormApi.spec.ts +++ b/packages/form-core/tests/FormApi.spec.ts @@ -4241,11 +4241,10 @@ describe('onSubmitInvalid callback', () => { }) form.mount() - expect(form.state.canSubmit).toBe(true) // canSubmitWhenInvalid allows submission + expect(form.state.canSubmit).toBe(true) await form.handleSubmit() - // With canSubmitWhenInvalid: true, onSubmit should be called even if invalid expect(onSubmit).toHaveBeenCalled() expect(onSubmitInvalid).not.toHaveBeenCalled() expect(form.state.isSubmitting).toBe(false) @@ -4266,13 +4265,12 @@ describe('onSubmitInvalid callback', () => { }) form.mount() - // With canSubmitWhenInvalid: true, canSubmit is always true regardless of errors + expect(form.state.canSubmit).toBe(true) - expect(form.state.isValid).toBe(false) // Form is invalid due to onMount error + expect(form.state.isValid).toBe(false) await form.handleSubmit() - - // With canSubmitWhenInvalid: true, onSubmit should be called even with errors + expect(onSubmit).toHaveBeenCalled() expect(onSubmitInvalid).not.toHaveBeenCalled() }) From 69f146460dcf930849ef93bd21104517ff798051 Mon Sep 17 00:00:00 2001 From: "autofix-ci[bot]" <114827586+autofix-ci[bot]@users.noreply.github.com> Date: Thu, 21 Aug 2025 14:29:22 +0000 Subject: [PATCH 3/3] ci: apply automated fixes and generate docs --- packages/form-core/tests/FormApi.spec.ts | 134 +++++++++++------------ 1 file changed, 66 insertions(+), 68 deletions(-) diff --git a/packages/form-core/tests/FormApi.spec.ts b/packages/form-core/tests/FormApi.spec.ts index da0c03268..cd4735e02 100644 --- a/packages/form-core/tests/FormApi.spec.ts +++ b/packages/form-core/tests/FormApi.spec.ts @@ -3961,24 +3961,20 @@ describe('onSubmitInvalid callback', () => { const form = new FormApi({ defaultValues: { name: '' }, validators: { - onMount: ({ value }) => value.name ? undefined : 'Name is required', + onMount: ({ value }) => (value.name ? undefined : 'Name is required'), }, onSubmitInvalid: ({ value, formApi, meta }) => { onInvalid(value, formApi, meta) - } + }, }) - + form.mount() expect(form.state.canSubmit).toBe(false) - + await form.handleSubmit() - + expect(onInvalid).toHaveBeenCalledTimes(1) - expect(onInvalid).toHaveBeenCalledWith( - { name: '' }, - form, - undefined - ) + expect(onInvalid).toHaveBeenCalledWith({ name: '' }, form, undefined) }) it('should call onSubmitInvalid with updated error state after validation', async () => { @@ -3995,17 +3991,17 @@ describe('onSubmitInvalid callback', () => { }, onSubmitInvalid: ({ value, formApi, meta }) => { onInvalid(value, formApi.state.errors, meta) - } + }, }) - + form.mount() await form.handleSubmit() - + expect(onInvalid).toHaveBeenCalledTimes(1) expect(onInvalid).toHaveBeenCalledWith( { name: '', email: '' }, ['Name is required, Email is required'], - undefined + undefined, ) }) @@ -4015,31 +4011,31 @@ describe('onSubmitInvalid callback', () => { defaultValues: { name: 'test', email: '' }, onSubmitInvalid: ({ value, formApi, meta }) => { onInvalid(value, formApi.state.isFieldsValid, formApi.state.fieldMeta) - } + }, }) - + const emailField = new FieldApi({ form, name: 'email', validators: { - onSubmit: ({ value }) => !value ? 'Email is required' : undefined, + onSubmit: ({ value }) => (!value ? 'Email is required' : undefined), }, }) - + form.mount() emailField.mount() - + await form.handleSubmit() - + expect(onInvalid).toHaveBeenCalledTimes(1) expect(onInvalid).toHaveBeenCalledWith( { name: 'test', email: '' }, false, expect.objectContaining({ email: expect.objectContaining({ - errors: ['Email is required'] - }) - }) + errors: ['Email is required'], + }), + }), ) }) @@ -4048,16 +4044,16 @@ describe('onSubmitInvalid callback', () => { const form = new FormApi({ defaultValues: { name: '' }, validators: { - onSubmit: ({ value }) => !value.name ? 'Name is required' : undefined, + onSubmit: ({ value }) => (!value.name ? 'Name is required' : undefined), }, onSubmitInvalid: ({ value, formApi, meta }) => { onInvalid(formApi === form, formApi.state.values) - } + }, }) - + form.mount() await form.handleSubmit() - + expect(onInvalid).toHaveBeenCalledWith(true, { name: '' }) }) @@ -4067,16 +4063,16 @@ describe('onSubmitInvalid callback', () => { const form = new FormApi({ defaultValues: { name: '' }, validators: { - onSubmit: ({ value }) => !value.name ? 'Name is required' : undefined, + onSubmit: ({ value }) => (!value.name ? 'Name is required' : undefined), }, onSubmitInvalid: ({ value, formApi, meta }) => { onInvalid(meta) - } + }, }) - + form.mount() await (form.handleSubmit as any)(customMeta) - + expect(onInvalid).toHaveBeenCalledWith(customMeta) }) @@ -4086,53 +4082,54 @@ describe('onSubmitInvalid callback', () => { defaultValues: { name: '', email: '', age: 0 }, onSubmitInvalid: ({ value, formApi, meta }) => { onInvalid(formApi.state.fieldMeta) - } + }, }) - + const nameField = new FieldApi({ form, name: 'name', validators: { - onSubmit: ({ value }) => !value ? 'Name is required' : undefined, + onSubmit: ({ value }) => (!value ? 'Name is required' : undefined), }, }) - + const emailField = new FieldApi({ form, name: 'email', validators: { - onSubmit: ({ value }) => !value ? 'Email is required' : undefined, + onSubmit: ({ value }) => (!value ? 'Email is required' : undefined), }, }) - + const ageField = new FieldApi({ form, name: 'age', validators: { - onSubmit: ({ value }) => value < 18 ? 'Must be 18 or older' : undefined, + onSubmit: ({ value }) => + value < 18 ? 'Must be 18 or older' : undefined, }, }) - + form.mount() nameField.mount() emailField.mount() ageField.mount() - + await form.handleSubmit() - + expect(onInvalid).toHaveBeenCalledTimes(1) expect(onInvalid).toHaveBeenCalledWith( expect.objectContaining({ name: expect.objectContaining({ - errors: ['Name is required'] + errors: ['Name is required'], }), email: expect.objectContaining({ - errors: ['Email is required'] + errors: ['Email is required'], }), age: expect.objectContaining({ - errors: ['Must be 18 or older'] - }) - }) + errors: ['Must be 18 or older'], + }), + }), ) }) @@ -4141,23 +4138,25 @@ describe('onSubmitInvalid callback', () => { const form = new FormApi({ defaultValues: { name: '', email: 'invalid' }, validators: { - onMount: ({ value }) => !value.name ? 'Name is required on mount' : undefined, - onSubmit: ({ value }) => !value.email.includes('@') ? 'Invalid email format' : undefined, + onMount: ({ value }) => + !value.name ? 'Name is required on mount' : undefined, + onSubmit: ({ value }) => + !value.email.includes('@') ? 'Invalid email format' : undefined, }, onSubmitInvalid: ({ value, formApi, meta }) => { onInvalid(formApi.state.errors, formApi.state.canSubmit) - } + }, }) - + form.mount() expect(form.state.canSubmit).toBe(false) - + await form.handleSubmit() - + expect(onInvalid).toHaveBeenCalledTimes(1) expect(onInvalid).toHaveBeenCalledWith( ['Name is required on mount', 'Invalid email format'], - false + false, ) }) @@ -4166,22 +4165,22 @@ describe('onSubmitInvalid callback', () => { const form = new FormApi({ defaultValues: { name: '' }, validators: { - onSubmit: ({ value }) => !value.name ? 'Name is required' : undefined, + onSubmit: ({ value }) => (!value.name ? 'Name is required' : undefined), }, onSubmitInvalid: ({ value, formApi, meta }) => { onInvalid(formApi.state.isSubmitting) - } + }, }) - + form.mount() - + expect(form.state.isSubmitting).toBe(false) - + const submitPromise = form.handleSubmit() expect(form.state.isSubmitting).toBe(true) - + await submitPromise - + expect(onInvalid).toHaveBeenCalledWith(false) expect(form.state.isSubmitting).toBe(false) }) @@ -4195,11 +4194,11 @@ describe('onSubmitInvalid callback', () => { }, onSubmitInvalid: ({ value }) => { onInvalid(value) - } + }, }) - + await form.handleSubmit() - + expect(onSubmit).toHaveBeenCalledTimes(1) expect(onInvalid).not.toHaveBeenCalled() }) @@ -4241,7 +4240,7 @@ describe('onSubmitInvalid callback', () => { }) form.mount() - expect(form.state.canSubmit).toBe(true) + expect(form.state.canSubmit).toBe(true) await form.handleSubmit() @@ -4250,7 +4249,6 @@ describe('onSubmitInvalid callback', () => { expect(form.state.isSubmitting).toBe(false) }) - it('should call onSubmit when canSubmitWhenInvalid is true even with onMount errors', async () => { const onSubmitInvalid = vi.fn() const onSubmit = vi.fn() @@ -4265,12 +4263,12 @@ describe('onSubmitInvalid callback', () => { }) form.mount() - + expect(form.state.canSubmit).toBe(true) - expect(form.state.isValid).toBe(false) + expect(form.state.isValid).toBe(false) await form.handleSubmit() - + expect(onSubmit).toHaveBeenCalled() expect(onSubmitInvalid).not.toHaveBeenCalled() })