From 56e8b28a8109a36639c971797556b6885a0a2daa Mon Sep 17 00:00:00 2001 From: zombiej Date: Wed, 12 Jun 2019 15:15:54 +0800 Subject: [PATCH 1/5] adjust demo component --- examples/components/Input.tsx | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/examples/components/Input.tsx b/examples/components/Input.tsx index e1e62677..75b02126 100644 --- a/examples/components/Input.tsx +++ b/examples/components/Input.tsx @@ -4,9 +4,9 @@ const Input = (props: any) => { return ; }; -const CustomizeInput = (props: any) => ( +const CustomizeInput = ({ value = '', ...props }: any) => (
- +
); From f318678cb662c48fdfeec02990aab9e2f0130da7 Mon Sep 17 00:00:00 2001 From: zombiej Date: Wed, 12 Jun 2019 15:59:06 +0800 Subject: [PATCH 2/5] support promise --- README.md | 33 ++++++++++------- examples/StateForm-validate-perf.tsx | 7 ++-- src/interface.ts | 2 +- src/useForm.ts | 3 +- src/utils/validateUtil.ts | 55 +++++++++++++++++++++++----- 5 files changed, 70 insertions(+), 30 deletions(-) diff --git a/README.md b/README.md index d7a615da..88434254 100644 --- a/README.md +++ b/README.md @@ -161,20 +161,25 @@ class Demo extends React.Component { ### Rule -| Prop | Type | -| --------------- | ------------------------------------------------------------------------------------ | -| enum | any[] | -| len | number | -| max | number | -| message | string | -| min | number | -| pattern | RegExp | -| required | boolean | -| transform | (value) => any | -| type | string | -| validator | ([rule](#rule), value, callback: (error?: string) => void, [form](#useform)) => void | -| whitespace | boolean | -| validateTrigger | string \| string[] | +| Prop | Type | +| --------------- | ----------------------------------------------------------------------------------------------- | +| enum | any[] | +| len | number | +| max | number | +| message | string | +| min | number | +| pattern | RegExp | +| required | boolean | +| transform | (value) => any | +| type | string | +| validator | ([rule](#rule), value, callback: (error?: string) => void, [form](#useform)) => Promise \| void | +| whitespace | boolean | +| validateTrigger | string \| string[] | + +#### validator + +To keep sync with `rc-form` legacy usage of `validator`, we still provides `callback` to trigger validate finished. +But in `rc-field-form`, we strongly recommend to return a Promise instead. ### ListOperations diff --git a/examples/StateForm-validate-perf.tsx b/examples/StateForm-validate-perf.tsx index 690a371c..24be6d7b 100644 --- a/examples/StateForm-validate-perf.tsx +++ b/examples/StateForm-validate-perf.tsx @@ -50,12 +50,11 @@ export default class Demo extends React.Component { rules={[ { required: true }, { - validator(_, value, callback, { getFieldValue }) { + async validator(_, value, __, { getFieldValue }) { if (getFieldValue('password') !== value) { - callback('password2 is not same as password'); - return; + return Promise.reject('password2 is not same as password'); } - callback(); + return Promise.resolve(); }, }, ]} diff --git a/src/interface.ts b/src/interface.ts index 62e2910f..cd8ccf1d 100644 --- a/src/interface.ts +++ b/src/interface.ts @@ -52,7 +52,7 @@ export interface Rule { value: any, callback: (error?: string) => void, context: FormInstance, // TODO: Maybe not good place to export this? - ) => void; + ) => Promise | void; whitespace?: boolean; /** Customize rule level `validateTrigger`. Must be subset of Field `validateTrigger` */ diff --git a/src/useForm.ts b/src/useForm.ts index dbffb1fa..146e9aa7 100644 --- a/src/useForm.ts +++ b/src/useForm.ts @@ -1,4 +1,5 @@ import * as React from 'react'; +import warning from 'warning'; import { Callbacks, FieldData, @@ -89,7 +90,7 @@ export class FormStore { }; } - console.error('`getInternalHooks` is internal usage. Should not call directly.'); + warning(false, '`getInternalHooks` is internal usage. Should not call directly.'); return null; }; diff --git a/src/utils/validateUtil.ts b/src/utils/validateUtil.ts index 71810178..2df08d3d 100644 --- a/src/utils/validateUtil.ts +++ b/src/utils/validateUtil.ts @@ -1,4 +1,5 @@ import AsyncValidator from 'async-validator'; +import warning from 'warning'; import { FieldError, InternalNamePath, @@ -61,7 +62,7 @@ function convertMessages(messages: ValidateMessages, name: string, rule: Rule) { return fillTemplate(setValues({}, defaultValidateMessages, messages)); } -function validateRule( +async function validateRule( name: string, value: any, rule: Rule, @@ -74,14 +75,15 @@ function validateRule( const messages = convertMessages(options.validateMessages, name, rule); validator.messages(messages); - return Promise.resolve(validator.validate({ [name]: value }, { ...options })) - .then(() => []) - .catch(errObj => { - if (errObj.errors) { - return errObj.errors.map(e => e.message); - } - return messages.default(); - }); + try { + await Promise.resolve(validator.validate({ [name]: value }, { ...options })); + return []; + } catch (errObj) { + if (errObj.errors) { + return errObj.errors.map(e => e.message); + } + return messages.default(); + } } /** @@ -105,7 +107,40 @@ export function validateRules( return { ...currentRule, validator(rule: any, val: any, callback: any) { - currentRule.validator(rule, val, callback, context); + let hasPromise = false; + + // Wrap callback only accept when promise not provided + const wrappedCallback = (...args: string[]) => { + warning( + !hasPromise, + 'Your validator function has already return a promise. `callback` will be ignored.', + ); + + if (!hasPromise) { + callback(...args); + } + }; + + // Get promise + const promise = currentRule.validator(rule, val, wrappedCallback, context); + hasPromise = + promise && typeof promise.then === 'function' && typeof promise.catch === 'function'; + + /** + * 1. Use promise as the first priority. + * 2. If promise not exist, use callback with warning instead + */ + warning(hasPromise, '`callback` is deprecated. Please return a promise instead.'); + + if (hasPromise) { + (promise as Promise) + .then(() => { + callback(); + }) + .catch(err => { + callback(err); + }); + } }, }; }); From faff0a47dbeeb887b062d4d939907ee4a833c605 Mon Sep 17 00:00:00 2001 From: zombiej Date: Wed, 12 Jun 2019 17:24:55 +0800 Subject: [PATCH 3/5] add test case --- package.json | 1 - src/utils/validateUtil.ts | 19 +++-- tests/common/InfoField.tsx | 31 +++++++ tests/common/timeout.ts | 5 ++ tests/validate.test.js | 164 +++++++++++++++++++++++++++++++++++++ 5 files changed, 211 insertions(+), 9 deletions(-) create mode 100644 tests/common/InfoField.tsx create mode 100644 tests/common/timeout.ts create mode 100644 tests/validate.test.js diff --git a/package.json b/package.json index 8e1e5fb9..e307b3b4 100644 --- a/package.json +++ b/package.json @@ -31,7 +31,6 @@ "build": "father doc build --storybook", "compile": "rc-tools run compile --babel-runtime", "gh-pages": "rc-tools run gh-pages", - "start1": "rc-tools run storybook", "pub": "rc-tools run pub --babel-runtime", "lint": "eslint src/**/*", "test": "father test", diff --git a/src/utils/validateUtil.ts b/src/utils/validateUtil.ts index 2df08d3d..51eefebb 100644 --- a/src/utils/validateUtil.ts +++ b/src/utils/validateUtil.ts @@ -111,14 +111,17 @@ export function validateRules( // Wrap callback only accept when promise not provided const wrappedCallback = (...args: string[]) => { - warning( - !hasPromise, - 'Your validator function has already return a promise. `callback` will be ignored.', - ); - - if (!hasPromise) { - callback(...args); - } + // Wait a tick to make sure return type is a promise + Promise.resolve().then(() => { + warning( + !hasPromise, + 'Your validator function has already return a promise. `callback` will be ignored.', + ); + + if (!hasPromise) { + callback(...args); + } + }); }; // Get promise diff --git a/tests/common/InfoField.tsx b/tests/common/InfoField.tsx new file mode 100644 index 00000000..e6a02d47 --- /dev/null +++ b/tests/common/InfoField.tsx @@ -0,0 +1,31 @@ +import React, { ReactElement } from 'react'; +import { Field } from '../../src'; +import { FieldProps } from '../../src/Field'; + +interface InfoFieldProps extends FieldProps { + children: ReactElement; +} + +/** + * Return a wrapped Field with meta info + */ +const InfoField: React.FC = ({ children, ...props }) => ( + + {(control, { errors }) => ( +
+ {children ? ( + React.cloneElement(children, control) + ) : ( + + )} +
    + {errors.map(error => ( +
  • {error}
    • + ))} +
+
+ )} + +); + +export default InfoField; diff --git a/tests/common/timeout.ts b/tests/common/timeout.ts new file mode 100644 index 00000000..9eecacec --- /dev/null +++ b/tests/common/timeout.ts @@ -0,0 +1,5 @@ +export default (timeout: number = 0) => { + return new Promise(resolve => { + setTimeout(resolve, timeout); + }); +}; diff --git a/tests/validate.test.js b/tests/validate.test.js new file mode 100644 index 00000000..605cfc87 --- /dev/null +++ b/tests/validate.test.js @@ -0,0 +1,164 @@ +import React from 'react'; +import { mount } from 'enzyme'; +import Form from '../src'; +import InfoField from './common/InfoField'; +import timeout from './common/timeout'; + +async function changeValue(wrapper, value) { + wrapper.find('input').simulate('change', { target: { value } }); + await timeout(); + wrapper.update(); +} + +function matchError(wrapper, error) { + if (error) { + expect(wrapper.find('.errors li').length).toBeTruthy(); + } else { + expect(wrapper.find('.errors li').length).toBeFalsy(); + } + + if (error && typeof error !== 'boolean') { + expect(wrapper.find('.errors li').text()).toBe(error); + } +} + +describe('validate', () => { + it('required', async () => { + const wrapper = mount( +
+ + , + ); + + await changeValue(wrapper, ''); + matchError(wrapper, true); + }); + + describe('validateMessages', () => { + function renderForm(messages) { + return mount( +
+ + , + ); + } + + it('template message', async () => { + const wrapper = renderForm({ required: "You miss '${name}'!" }); + + await changeValue(wrapper, ''); + matchError(wrapper, "You miss 'username'!"); + }); + + it('function message', async () => { + const wrapper = renderForm({ required: () => 'Bamboo & Light' }); + + await changeValue(wrapper, ''); + matchError(wrapper, 'Bamboo & Light'); + }); + }); + + it('customize validator', async () => { + const wrapper = mount( +
+ + , + ); + + // Wrong value + await changeValue(wrapper, 'light'); + matchError(wrapper, 'should be bamboo!'); + + // Correct value + await changeValue(wrapper, 'bamboo'); + matchError(wrapper, false); + }); + + it('fail validate if throw', async () => { + const wrapper = mount( +
+ + , + ); + + // Wrong value + await changeValue(wrapper, 'light'); + matchError(wrapper, "Validation error on field 'username'"); + }); + + describe('callback', () => { + it('warning if not return promise', async () => { + const errorSpy = jest.spyOn(console, 'error').mockImplementation(() => {}); + + const wrapper = mount( +
+ + , + ); + + await changeValue(wrapper, 'light'); + expect(errorSpy).toHaveBeenCalledWith( + 'Warning: `callback` is deprecated. Please return a promise instead.', + ); + + errorSpy.mockRestore(); + }); + + it('warning if both promise & callback exist', async () => { + const errorSpy = jest.spyOn(console, 'error').mockImplementation(() => {}); + + const wrapper = mount( +
+ {}); + }, + }, + ]} + /> + , + ); + + await changeValue(wrapper, 'light'); + expect(errorSpy).toHaveBeenCalledWith( + 'Warning: Your validator function has already return a promise. `callback` will be ignored.', + ); + + errorSpy.mockRestore(); + }); + }); +}); From 7c75924903150b63f205219b737294b0ccf8c9f2 Mon Sep 17 00:00:00 2001 From: zombiej Date: Wed, 12 Jun 2019 17:47:17 +0800 Subject: [PATCH 4/5] test context --- package.json | 2 +- tests/common/index.js | 19 +++++++++++++++++++ tests/context.test.js | 20 ++++++++++++++++++++ tests/validate.test.js | 20 +------------------- 4 files changed, 41 insertions(+), 20 deletions(-) create mode 100644 tests/common/index.js create mode 100644 tests/context.test.js diff --git a/package.json b/package.json index e307b3b4..56db8736 100644 --- a/package.json +++ b/package.json @@ -45,7 +45,7 @@ "enzyme": "^3.1.0", "enzyme-adapter-react-16": "^1.0.2", "enzyme-to-json": "^3.1.4", - "father": "^2.6.6", + "father": "^2.7.2", "react": "^16.8.6", "react-dom": "^16.8.6", "react-redux": "^4.4.10", diff --git a/tests/common/index.js b/tests/common/index.js new file mode 100644 index 00000000..df37b29b --- /dev/null +++ b/tests/common/index.js @@ -0,0 +1,19 @@ +import timeout from './timeout'; + +export async function changeValue(wrapper, value) { + wrapper.find('input').simulate('change', { target: { value } }); + await timeout(); + wrapper.update(); +} + +export function matchError(wrapper, error) { + if (error) { + expect(wrapper.find('.errors li').length).toBeTruthy(); + } else { + expect(wrapper.find('.errors li').length).toBeFalsy(); + } + + if (error && typeof error !== 'boolean') { + expect(wrapper.find('.errors li').text()).toBe(error); + } +} diff --git a/tests/context.test.js b/tests/context.test.js new file mode 100644 index 00000000..7fd0f007 --- /dev/null +++ b/tests/context.test.js @@ -0,0 +1,20 @@ +import React from 'react'; +import { mount } from 'enzyme'; +import Form, { FormProvider } from '../src'; +import InfoField from './common/InfoField'; +import { changeValue, matchError } from './common'; + +describe('context', () => { + it('validateMessages', async () => { + const wrapper = mount( + +
+ + +
, + ); + + await changeValue(wrapper, ''); + matchError(wrapper, "I'm global"); + }); +}); diff --git a/tests/validate.test.js b/tests/validate.test.js index 605cfc87..cf4d57cf 100644 --- a/tests/validate.test.js +++ b/tests/validate.test.js @@ -2,25 +2,7 @@ import React from 'react'; import { mount } from 'enzyme'; import Form from '../src'; import InfoField from './common/InfoField'; -import timeout from './common/timeout'; - -async function changeValue(wrapper, value) { - wrapper.find('input').simulate('change', { target: { value } }); - await timeout(); - wrapper.update(); -} - -function matchError(wrapper, error) { - if (error) { - expect(wrapper.find('.errors li').length).toBeTruthy(); - } else { - expect(wrapper.find('.errors li').length).toBeFalsy(); - } - - if (error && typeof error !== 'boolean') { - expect(wrapper.find('.errors li').text()).toBe(error); - } -} +import { changeValue, matchError } from './common'; describe('validate', () => { it('required', async () => { From ee78075de5178717658ef5363a0dc1b015ada8f8 Mon Sep 17 00:00:00 2001 From: zombiej Date: Wed, 12 Jun 2019 18:00:14 +0800 Subject: [PATCH 5/5] update prettier config --- .prettierrc | 6 +++ README.md | 121 ++++++++++++++++++++++++---------------------------- 2 files changed, 62 insertions(+), 65 deletions(-) create mode 100644 .prettierrc diff --git a/.prettierrc b/.prettierrc new file mode 100644 index 00000000..5aab7f9b --- /dev/null +++ b/.prettierrc @@ -0,0 +1,6 @@ +{ + "singleQuote": true, + "trailingComma": "all", + "printWidth": 100, + "proseWrap": "never" +} \ No newline at end of file diff --git a/README.md b/README.md index 88434254..5eae3b63 100644 --- a/README.md +++ b/README.md @@ -2,11 +2,7 @@ React Performance First Form Component. -[![NPM version][npm-image]][npm-url] -[![build status][circleci-image]][circleci-url] -[![Test coverage][coveralls-image]][coveralls-url] -[![node version][node-image]][node-url] -[![npm download][download-image]][download-url] +[![NPM version][npm-image]][npm-url] [![build status][circleci-image]][circleci-url] [![Test coverage][coveralls-image]][coveralls-url] [![node version][node-image]][node-url] [![npm download][download-image]][download-url] [npm-image]: http://img.shields.io/npm/v/rc-field-form.svg?style=flat-square [npm-url]: http://npmjs.org/package/rc-field-form @@ -61,43 +57,40 @@ export default Demo; # API -We use typescript to create the Type definition. You can view directly in IDE. -But you can still check the type definition [here](https://github.com/react-component/field-form/blob/master/src/interface.ts). +We use typescript to create the Type definition. You can view directly in IDE. But you can still check the type definition [here](https://github.com/react-component/field-form/blob/master/src/interface.ts). ## Form -| Prop | Description | Type | Default | -| ---------------- | -------------------------------------------------- | ------------------------------------- | ---------------- | -| fields | Control Form fields status. Only use when in Redux | [FieldData](#fielddata)[] | - | -| form | Set form instance created by `useForm` | [FormInstance](#useform) | `Form.useForm()` | -| initialValues | Initial value of Form | Object | - | -| name | Config name with [FormProvider](#formprovider) | string | - | -| validateMessages | Set validate message template | [ValidateMessages](#validatemessages) | - | -| onFieldsChange | Trigger when any value of Field changed | (changedFields, allFields): void | - | -| onValuesChange | Trigger when any value of Field changed | (changedValues, values): void | - | +| Prop | Description | Type | Default | +| --- | --- | --- | --- | +| fields | Control Form fields status. Only use when in Redux | [FieldData](#fielddata)[] | - | +| form | Set form instance created by `useForm` | [FormInstance](#useform) | `Form.useForm()` | +| initialValues | Initial value of Form | Object | - | +| name | Config name with [FormProvider](#formprovider) | string | - | +| validateMessages | Set validate message template | [ValidateMessages](#validatemessages) | - | +| onFieldsChange | Trigger when any value of Field changed | (changedFields, allFields): void | - | +| onValuesChange | Trigger when any value of Field changed | (changedValues, values): void | - | ## Field -| Prop | Description | Type | Default | -| --------------- | --------------------------------------- | --------------------------------- | -------- | -| name | Field name path | [NamePath](#namepath)[] | - | -| rules | Validate rules | [Rule](#rule)[] | - | -| shouldUpdate | Check if Field should update | (prevValues, nextValues): boolean | - | -| trigger | Collect value update by event trigger | string | onChange | -| validateTrigger | Config trigger point with rule validate | string \| string[] | onChange | +| Prop | Description | Type | Default | +| --- | --- | --- | --- | +| name | Field name path | [NamePath](#namepath)[] | - | +| rules | Validate rules | [Rule](#rule)[] | - | +| shouldUpdate | Check if Field should update | (prevValues, nextValues): boolean | - | +| trigger | Collect value update by event trigger | string | onChange | +| validateTrigger | Config trigger point with rule validate | string \| string[] | onChange | ## List -| Prop | Description | Type | Default | -| -------- | ------------------------------- | ----------------------------------------------------------------------------------------------------- | ------- | -| name | List field name path | [NamePath](#namepath)[] | - | -| children | Render props for listing fields | (fields: { name: [NamePath](#namepath) }[], operations: [ListOperations](#listoperations)): ReactNode | - | +| Prop | Description | Type | Default | +| --- | --- | --- | --- | +| name | List field name path | [NamePath](#namepath)[] | - | +| children | Render props for listing fields | (fields: { name: [NamePath](#namepath) }[], operations: [ListOperations](#listoperations)): ReactNode | - | ## useForm -Form component default create an form instance by `Form.useForm`. -But you can create it and pass to Form also. -This allow you to call some function on the form instance. +Form component default create an form instance by `Form.useForm`. But you can create it and pass to Form also. This allow you to call some function on the form instance. ```jsx const Demo = () => { @@ -120,26 +113,26 @@ class Demo extends React.Component { } ``` -| Prop | Description | Type | -| ----------------- | ------------------------------------------ | -------------------------------------------------------------------------- | -| getFieldValue | Get field value by name path | (name: [NamePath](#namepath)) => any | -| getFieldsValue | Get list of field values by name path list | (nameList?: [NamePath](#namepath)[]) => any | -| getFieldError | Get field errors by name path | (name: [NamePath](#namepath)) => string[] | -| getFieldsError | Get list of field errors by name path list | (nameList?: [NamePath](#namepath)[]) => FieldError[] | -| isFieldsTouched | Check if list of fields are touched | (nameList?: [NamePath](#namepath)[]) => boolean | -| isFieldTouched | Check if a field is touched | (name: [NamePath](#namepath)) => boolean | -| isFieldValidating | Check if a field is validating | (name: [NamePath](#namepath)) => boolean | -| resetFields | Reset fields status | (fields?: [NamePath](#namepath)[]) => void | -| setFields | Set fields status | (fields: FieldData[]) => void | -| setFieldsValue | Set fields value | (values) => void | -| validateFields | Trigger fields to validate | (nameList?: [NamePath](#namepath)[], options?: ValidateOptions) => Promise | +| Prop | Description | Type | +| --- | --- | --- | +| getFieldValue | Get field value by name path | (name: [NamePath](#namepath)) => any | +| getFieldsValue | Get list of field values by name path list | (nameList?: [NamePath](#namepath)[]) => any | +| getFieldError | Get field errors by name path | (name: [NamePath](#namepath)) => string[] | +| getFieldsError | Get list of field errors by name path list | (nameList?: [NamePath](#namepath)[]) => FieldError[] | +| isFieldsTouched | Check if list of fields are touched | (nameList?: [NamePath](#namepath)[]) => boolean | +| isFieldTouched | Check if a field is touched | (name: [NamePath](#namepath)) => boolean | +| isFieldValidating | Check if a field is validating | (name: [NamePath](#namepath)) => boolean | +| resetFields | Reset fields status | (fields?: [NamePath](#namepath)[]) => void | +| setFields | Set fields status | (fields: FieldData[]) => void | +| setFieldsValue | Set fields value | (values) => void | +| validateFields | Trigger fields to validate | (nameList?: [NamePath](#namepath)[], options?: ValidateOptions) => Promise | ## FormProvider -| Prop | Description | Type | Default | -| ---------------- | ----------------------------------------- | ---------------------------------------- | ------- | -| validateMessages | Config global `validateMessages` template | [ValidateMessages](#validatemessages) | - | -| onFormChange | Trigger by named form fields change | (name, { changedFields, forms }) => void | - | +| Prop | Description | Type | Default | +| --- | --- | --- | --- | +| validateMessages | Config global `validateMessages` template | [ValidateMessages](#validatemessages) | - | +| onFormChange | Trigger by named form fields change | (name, { changedFields, forms }) => void | - | ## Interface @@ -161,25 +154,24 @@ class Demo extends React.Component { ### Rule -| Prop | Type | -| --------------- | ----------------------------------------------------------------------------------------------- | -| enum | any[] | -| len | number | -| max | number | -| message | string | -| min | number | -| pattern | RegExp | -| required | boolean | -| transform | (value) => any | -| type | string | -| validator | ([rule](#rule), value, callback: (error?: string) => void, [form](#useform)) => Promise \| void | -| whitespace | boolean | -| validateTrigger | string \| string[] | +| Prop | Type | +| --- | --- | +| enum | any[] | +| len | number | +| max | number | +| message | string | +| min | number | +| pattern | RegExp | +| required | boolean | +| transform | (value) => any | +| type | string | +| validator | ([rule](#rule), value, callback: (error?: string) => void, [form](#useform)) => Promise \| void | +| whitespace | boolean | +| validateTrigger | string \| string[] | #### validator -To keep sync with `rc-form` legacy usage of `validator`, we still provides `callback` to trigger validate finished. -But in `rc-field-form`, we strongly recommend to return a Promise instead. +To keep sync with `rc-form` legacy usage of `validator`, we still provides `callback` to trigger validate finished. But in `rc-field-form`, we strongly recommend to return a Promise instead. ### ListOperations @@ -190,8 +182,7 @@ But in `rc-field-form`, we strongly recommend to return a Promise instead. ### ValidateMessages -Validate Messages provides a list of error template. -You can ref [here](https://github.com/react-component/field-form/blob/master/src/utils/messages.ts) for fully default templates. +Validate Messages provides a list of error template. You can ref [here](https://github.com/react-component/field-form/blob/master/src/utils/messages.ts) for fully default templates. | Prop | Description | | ------- | ------------------- |