fix(components): [el-form] validation with callbacks throws (#6669)

* fix(components): [el-form] validation with callbacks throws

- Fix Form component's validation with callbacks still throws error
- Fix FormItem component's validation with callbacks still throws error
- Update test cases to make sure this functionality's integrity

* Fix linter

packages/components/form/__tests__/form-item.spec.tsx

@@ -0,0 +1,109 @@
+import { ref, reactive, nextTick } from 'vue'
+import { mount } from '@vue/test-utils'
+import { rAF } from '@element-plus/test-utils/tick'
+import Input from '@element-plus/components/input'
+import FormItem from '../src/form-item.vue'
+import DynamicFormItem from '../mocks/mock-data'
+
+import type { VueWrapper } from '@vue/test-utils'
+import type { InputInstance } from '@element-plus/components/input'
+
+type FormItemInstance = InstanceType<typeof FormItem>
+
+describe('ElFormItem', () => {
+  let wrapper: VueWrapper<InstanceType<typeof DynamicFormItem>>
+  const formItemRef = ref<FormItemInstance>()
+  const inputRef = ref<InputInstance>()
+  const model = reactive({
+    email: '',
+  })
+
+  const createComponent = () => {
+    wrapper = mount(DynamicFormItem, {
+      props: {
+        model,
+      },
+      slots: {
+        default: () => (
+          <FormItem prop="email" required ref={formItemRef}>
+            <Input class="input" ref={inputRef} v-model={model.email} />
+          </FormItem>
+        ),
+      },
+    })
+  }
+
+  beforeAll(() => jest.spyOn(console, 'warn').mockImplementation())
+  afterAll(() => (console.warn as any as jest.SpyInstance).mockRestore())
+  afterEach(() => {
+    formItemRef.value = undefined
+    inputRef.value = undefined
+    model.email = ''
+  })
+
+  describe('When initialized', () => {
+    it('should throw when no form on top', () => {
+      const warnHandler = jest.fn()
+      try {
+        mount(FormItem, {
+          global: {
+            config: {
+              warnHandler,
+            },
+          },
+        })
+      } catch (e) {
+        expect(e).toBeInstanceOf(Error)
+      }
+      expect(warnHandler).toHaveBeenCalled()
+    })
+  })
+
+  describe('when validation dispatches', () => {
+    beforeEach(() => {
+      createComponent()
+    })
+
+    afterEach(() => {
+      wrapper.unmount()
+    })
+
+    describe('it successes', () => {
+      it('should be able to validate successfully without callback', async () => {
+        const emailInput = formItemRef.value!
+        model.email = 'test'
+        await nextTick()
+        await rAF()
+        expect(emailInput.validate('')).resolves.toBe(true)
+      })
+
+      it('should be able to validate successfully with callback', async () => {
+        const emailInput = formItemRef.value!
+        model.email = 'test'
+        await nextTick()
+        await rAF()
+        const callback = jest.fn()
+        expect(emailInput.validate('', callback)).resolves.toBe(true)
+        await rAF()
+        expect(callback).toHaveBeenCalledWith(true)
+      })
+    })
+
+    describe('it fails', () => {
+      it('should be able to validate without callback', async () => {
+        const emailInput = formItemRef.value!
+        expect(emailInput.validate('')).rejects.toHaveProperty('email')
+        expect(console.warn).toHaveBeenCalled()
+      })
+
+      it('should be able to validate with callback without throwing rejection', async () => {
+        const emailInput = formItemRef.value!
+        const callback = jest.fn()
+        expect(console.warn).toHaveBeenCalled()
+        expect(emailInput.validate('', callback)).resolves.toBe(false)
+        await rAF()
+        expect(callback).toHaveBeenCalled()
+      })
+    })
+  })
+})

packages/components/form/__tests__/form.spec.tsx

@@ -2,17 +2,21 @@ import { nextTick, reactive, ref } from 'vue'
 import { mount } from '@vue/test-utils'
 import { rAF } from '@element-plus/test-utils/tick'
 import installStyle from '@element-plus/test-utils/style-plugin'
-import Checkbox from '@element-plus/components/checkbox/src/checkbox.vue'
-import CheckboxGroup from '@element-plus/components/checkbox/src/checkbox-group.vue'
+import {
+  ElCheckboxGroup as CheckboxGroup,
+  ElCheckbox as Checkbox,
+} from '@element-plus/components/checkbox'
 import Input from '@element-plus/components/input'
 import Form from '../src/form.vue'
 import FormItem from '../src/form-item.vue'
 import DynamicDomainForm, { formatDomainError } from '../mocks/mock-data'
 
 import type { VueWrapper } from '@vue/test-utils'
+import type { ValidateFieldsError } from 'async-validator'
 import type { FormRules } from '@element-plus/tokens'
-import type { FormInstance } from '../src/form'
-import type { FormItemInstance } from '../src/form-item'
+
+type FormInstance = InstanceType<typeof Form>
+type FormItemInstance = InstanceType<typeof FormItem>
 
 const findStyle = (wrapper: VueWrapper<any>, selector: string) =>
   wrapper.find<HTMLElement>(selector).element.style
@@ -262,13 +266,13 @@ describe('Form', () => {
     })
     const form = wrapper.findComponent(Form).vm as FormInstance
     form
-      .validate(async (valid) => {
+      .validate(async (valid: boolean) => {
         expect(valid).toBe(false)
         await nextTick()
         expect(wrapper.find('.el-form-item__error').exists()).toBe(false)
         done()
       })
-      .catch((e) => {
+      .catch((e: ValidateFieldsError) => {
         expect(e).toBeDefined()
       })
   })
@@ -531,11 +535,12 @@ describe('Form', () => {
     const onSuccess = jest.fn()
     const onError = jest.fn()
     let wrapper: VueWrapper<InstanceType<typeof DynamicDomainForm>>
-    const createComponent = () => {
+    const createComponent = (onSubmit?: jest.MockedFunction<any>) => {
       wrapper = mount(DynamicDomainForm, {
         props: {
           onSuccess,
           onError,
+          onSubmit,
         },
       })
     }
@@ -580,5 +585,15 @@ describe('Form', () => {
       await rAF()
       expect(onError).toHaveBeenLastCalledWith(formatDomainError(1))
     })
+
+    it('should not throw error when callback passed in', async () => {
+      const onSubmit = jest.fn()
+      createComponent(onSubmit)
+
+      await findSubmitButton().trigger('click')
+      await rAF()
+      expect(onError).not.toHaveBeenCalled()
+      expect(onSubmit).toHaveBeenCalled()
+    })
   })
 })

packages/components/form/index.ts

@@ -11,3 +11,6 @@ export const ElFormItem = withNoopInstall(FormItem)
 export * from './src/form'
 export * from './src/form-item'
 export * from './src/types'
+
+export type FormInstance = InstanceType<typeof Form>
+export type FormItemInstance = InstanceType<typeof FormItem>

packages/components/form/mocks/mock-data.tsx

@@ -1,11 +1,9 @@
-import { defineComponent, ref } from 'vue'
+import { defineComponent, ref, toRef } from 'vue'
 import Input from '@element-plus/components/input'
 import Button from '@element-plus/components/button'
 import Form from '../src/form.vue'
 import FormItem from '../src/form-item.vue'
 
-import type { FormInstance } from '../src/form'
-
 interface DomainItem {
   key: number
   value: string
@@ -15,8 +13,11 @@ const DynamicDomainForm = defineComponent({
   props: {
     onSuccess: Function,
     onError: Function,
+    onSubmit: Function,
+    model: Object,
   },
-  setup(props) {
+  setup(props, { slots }) {
+    const propsModel = toRef(props, 'model')
     const model = ref({
       domains: [
         {
@@ -26,7 +27,7 @@ const DynamicDomainForm = defineComponent({
       ],
     })
 
-    const formRef = ref<FormInstance>()
+    const formRef = ref<InstanceType<typeof Form>>()
 
     const removeDomain = (item: DomainItem) => {
       const index = model.value.domains.indexOf(item)
@@ -45,7 +46,11 @@ const DynamicDomainForm = defineComponent({
     const submitForm = async () => {
       if (!formRef.value) return
       try {
-        await formRef.value.validate()
+        const validate = props.onSubmit
+          ? formRef.value.validate(props.onSubmit as any)
+          : formRef.value.validate()
+
+        await validate
         props.onSuccess?.()
       } catch (e) {
         props.onError?.(e)
@@ -53,7 +58,10 @@ const DynamicDomainForm = defineComponent({
     }
 
     return () => (
-      <Form ref={formRef} model={model.value}>
+      <Form
+        ref={formRef}
+        model={{ ...model.value, ...(propsModel.value || {}) }}
+      >
         {model.value.domains.map((domain, index) => {
           return (
             <FormItem
@@ -80,6 +88,7 @@ const DynamicDomainForm = defineComponent({
             </FormItem>
           )
         })}
+        {slots.default?.()}
 
         <FormItem>
           <Button class="submit" type="primary" onClick={submitForm}>

packages/components/form/src/form-item.ts

@@ -3,8 +3,7 @@ import { buildProps, definePropType } from '@element-plus/utils'
 
 import type { ExtractPropTypes } from 'vue'
 import type { Arrayable } from '@element-plus/utils'
-import type FormItem from './form-item.vue'
-import type { FormItemRule } from './types'
+import type { FormItemRule } from '@element-plus/tokens'
 
 export const formItemValidateStates = [
   '',
@@ -52,5 +51,3 @@ export const formItemProps = buildProps({
   },
 } as const)
 export type FormItemProps = ExtractPropTypes<typeof formItemProps>
-
-export type FormItemInstance = InstanceType<typeof FormItem>

packages/components/form/src/form-item.vue

@@ -51,6 +51,7 @@ import {
   getProp,
   isString,
   isBoolean,
+  isFunction,
   throwError,
 } from '@element-plus/utils'
 import { formItemContextKey, formContextKey } from '@element-plus/tokens'
@@ -241,11 +242,19 @@ const doValidate = async (rules: RuleItem[]): Promise<true> => {
       return Promise.reject(err)
     })
 }
+
 const validate: FormItemContext['validate'] = async (trigger, callback) => {
-  if (!validateEnabled.value) return false
+  const hasCallback = isFunction(callback)
+  if (!validateEnabled.value) {
+    callback?.(false)
+    return false
+  }
 
   const rules = getFilteredRule(trigger)
-  if (rules.length === 0) return true
+  if (rules.length === 0) {
+    callback?.(true)
+    return true
+  }
 
   setValidationState('validating')
 
@@ -257,7 +266,7 @@ const validate: FormItemContext['validate'] = async (trigger, callback) => {
     .catch((err: FormValidateFailure) => {
       const { fields } = err
       callback?.(false, fields)
-      return Promise.reject(fields)
+      return hasCallback ? false : Promise.reject(fields)
     })
 }
 

packages/components/form/src/form.ts

@@ -9,8 +9,7 @@ import {
 
 import type { ExtractPropTypes } from 'vue'
 import type { FormItemProp } from './form-item'
-import type { FormRules } from './types'
-import type Form from './form.vue'
+import type { FormRules } from '@element-plus/tokens'
 
 export const formProps = buildProps({
   model: Object,
@@ -57,5 +56,3 @@ export const formEmits = {
     isString(message),
 }
 export type FormEmits = typeof formEmits
-
-export type FormInstance = InstanceType<typeof Form>

packages/components/form/src/form.vue

@@ -6,12 +6,14 @@
 
 <script lang="ts" setup>
 import { computed, provide, reactive, toRefs, watch } from 'vue'
-import { debugWarn, type Arrayable } from '@element-plus/utils'
+import { debugWarn, isFunction } from '@element-plus/utils'
 import { formContextKey } from '@element-plus/tokens'
 import { useNamespace, useSize } from '@element-plus/hooks'
 import { formProps, formEmits } from './form'
 import { useFormLabelWidth, filterFields } from './utils'
+
 import type { ValidateFieldsError } from 'async-validator'
+import type { Arrayable } from '@element-plus/utils'
 import type {
   FormItemContext,
   FormContext,
@@ -116,6 +118,7 @@ const validateField: FormContext['validateField'] = async (
   modelProps = [],
   callback
 ) => {
+  const shouldThrow = !isFunction(callback)
   try {
     const result = await doValidateField(modelProps)
     // When result is false meaning that the fields are not validatable
@@ -130,7 +133,7 @@ const validateField: FormContext['validateField'] = async (
       scrollToField(Object.keys(invalidFields)[0])
     }
     callback?.(false, invalidFields)
-    return Promise.reject(invalidFields)
+    return shouldThrow && Promise.reject(invalidFields)
   }
 }