From b67a1bee7d554938e83a04608a522863c0a17d46 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E4=BA=8C=E8=B4=A7=E6=9C=BA=E5=99=A8=E4=BA=BA?= Date: Fri, 17 Jan 2020 11:50:06 +0800 Subject: [PATCH] fix: Form.Item keep render even it's not a Field (#20963) * Not use Field when Form.Item is pure * Add test case * clean up * add delay update for test --- components/form/FormItem.tsx | 249 +++++++++++++----------- components/form/FormItemInput.tsx | 18 +- components/form/FormItemLabel.tsx | 2 +- components/form/__tests__/index.test.js | 58 +++++- 4 files changed, 202 insertions(+), 125 deletions(-) diff --git a/components/form/FormItem.tsx b/components/form/FormItem.tsx index 19f32fc75c..f09e718861 100644 --- a/components/form/FormItem.tsx +++ b/components/form/FormItem.tsx @@ -3,6 +3,7 @@ import isEqual from 'lodash/isEqual'; import classNames from 'classnames'; import { Field, FormInstance } from 'rc-field-form'; import { FieldProps } from 'rc-field-form/lib/Field'; +import { Meta } from 'rc-field-form/lib/interface'; import omit from 'omit.js'; import Row from '../grid/row'; import { ConfigContext } from '../config-provider'; @@ -18,6 +19,8 @@ export type ValidateStatus = typeof ValidateStatuses[number]; type RenderChildren = (form: FormInstance) => React.ReactElement; type RcFieldProps = Omit; +type ChildrenType = React.ReactElement | RenderChildren | React.ReactElement[] | null; +type ChildrenNodeType = Exclude; export interface FormItemProps extends FormItemLabelProps, @@ -27,7 +30,7 @@ export interface FormItemProps noStyle?: boolean; style?: React.CSSProperties; className?: string; - children: React.ReactElement | RenderChildren | React.ReactElement[]; + children: ChildrenType; id?: string; hasFeedback?: boolean; validateStatus?: ValidateStatus; @@ -37,7 +40,7 @@ export interface FormItemProps fieldKey?: number; } -const FormItem: React.FC = (props: FormItemProps) => { +function FormItem(props: FormItemProps): React.ReactNode { const { name, fieldKey, @@ -77,6 +80,119 @@ const FormItem: React.FC = (props: FormItemProps) => { const prefixCls = getPrefixCls('form', customizePrefixCls); + // ======================== Errors ======================== + // Collect noStyle Field error to the top FormItem + const updateChildItemErrors = noStyle + ? updateItemErrors + : (subName: string, subErrors: string[]) => { + if (!isEqual(inlineErrors[subName], subErrors)) { + setInlineErrors({ + ...inlineErrors, + [subName]: subErrors, + }); + } + }; + + function renderLayout( + baseChildren: ChildrenNodeType, + fieldId?: string, + meta?: Meta, + isRequired?: boolean, + ) { + if (noStyle) { + return baseChildren; + } + + // ======================== Errors ======================== + let mergedErrors: React.ReactNode[]; + if (help) { + mergedErrors = toArray(help); + } else { + mergedErrors = meta ? meta.errors : []; + Object.keys(inlineErrors).forEach(subName => { + const subErrors = inlineErrors[subName] || []; + if (subErrors.length) { + mergedErrors = [...mergedErrors, ...subErrors]; + } + }); + } + + // ======================== Status ======================== + let mergedValidateStatus: ValidateStatus = ''; + if (validateStatus !== undefined) { + mergedValidateStatus = validateStatus; + } else if (meta && meta.validating) { + mergedValidateStatus = 'validating'; + } else if (!help && mergedErrors.length) { + mergedValidateStatus = 'error'; + } else if (meta && meta.touched) { + mergedValidateStatus = 'success'; + } + + const itemClassName = { + [`${prefixCls}-item`]: true, + [`${prefixCls}-item-with-help`]: domErrorVisible || help, + [`${className}`]: !!className, + + // Status + [`${prefixCls}-item-has-feedback`]: mergedValidateStatus && hasFeedback, + [`${prefixCls}-item-has-success`]: mergedValidateStatus === 'success', + [`${prefixCls}-item-has-warning`]: mergedValidateStatus === 'warning', + [`${prefixCls}-item-has-error`]: mergedValidateStatus === 'error', + [`${prefixCls}-item-has-error-leave`]: + !help && domErrorVisible && mergedValidateStatus !== 'error', + [`${prefixCls}-item-is-validating`]: mergedValidateStatus === 'validating', + }; + + // ======================= Children ======================= + return ( + + {/* Label */} + + {/* Input Group */} + + + {baseChildren} + + + + ); + } + + const isRenderProps = typeof children === 'function'; + + if (!name && !isRenderProps && !dependencies) { + return renderLayout(children as ChildrenNodeType); + } + return ( = (props: FormItemProps) => { }} > {(control, meta, context) => { - const { errors, name: metaName } = meta; - const mergedName = toArray(name).length ? metaName : []; + const { errors } = meta; - // ======================== Errors ======================== - // Collect noStyle Field error to the top FormItem - const updateChildItemErrors = noStyle - ? updateItemErrors - : (subName: string, subErrors: string[]) => { - if (!isEqual(inlineErrors[subName], subErrors)) { - setInlineErrors({ - ...inlineErrors, - [subName]: subErrors, - }); - } - }; + const mergedName = toArray(name).length && meta ? meta.name : []; + const fieldId = getFieldId(mergedName, formName); if (noStyle) { nameRef.current = [...mergedName]; @@ -111,47 +216,6 @@ const FormItem: React.FC = (props: FormItemProps) => { updateItemErrors(nameRef.current.join('__SPLIT__'), errors); } - let mergedErrors: React.ReactNode[]; - if (help) { - mergedErrors = toArray(help); - } else { - mergedErrors = errors; - Object.keys(inlineErrors).forEach(subName => { - const subErrors = inlineErrors[subName] || []; - if (subErrors.length) { - mergedErrors = [...mergedErrors, ...subErrors]; - } - }); - } - - // ======================== Status ======================== - let mergedValidateStatus: ValidateStatus = ''; - if (validateStatus !== undefined) { - mergedValidateStatus = validateStatus; - } else if (meta.validating) { - mergedValidateStatus = 'validating'; - } else if (!help && mergedErrors.length) { - mergedValidateStatus = 'error'; - } else if (meta.touched) { - mergedValidateStatus = 'success'; - } - - // ====================== Class Name ====================== - const itemClassName = { - [`${prefixCls}-item`]: true, - [`${prefixCls}-item-with-help`]: domErrorVisible || help, - [`${className}`]: !!className, - - // Status - [`${prefixCls}-item-has-feedback`]: mergedValidateStatus && hasFeedback, - [`${prefixCls}-item-has-success`]: mergedValidateStatus === 'success', - [`${prefixCls}-item-has-warning`]: mergedValidateStatus === 'warning', - [`${prefixCls}-item-has-error`]: mergedValidateStatus === 'error', - [`${prefixCls}-item-has-error-leave`]: - !help && domErrorVisible && mergedValidateStatus !== 'error', - [`${prefixCls}-item-is-validating`]: mergedValidateStatus === 'validating', - }; - const isRequired = required !== undefined ? required @@ -170,17 +234,16 @@ const FormItem: React.FC = (props: FormItemProps) => { ); // ======================= Children ======================= - const fieldId = getFieldId(mergedName, formName); const mergedControl: typeof control = { ...control, id: fieldId, }; - let childNode; + let childNode: ChildrenNodeType = null; if (Array.isArray(children) && !!name) { warning(false, 'Form.Item', '`children` is array of render props cannot have `name`.'); childNode = children; - } else if (typeof children === 'function' && (!shouldUpdate || !!name)) { + } else if (isRenderProps && (!shouldUpdate || !!name)) { warning( !!shouldUpdate, 'Form.Item', @@ -191,8 +254,12 @@ const FormItem: React.FC = (props: FormItemProps) => { 'Form.Item', "Do not use `name` with `children` of render props since it's not a field.", ); - } else if (!mergedName.length && !shouldUpdate && !dependencies) { - childNode = children; + } else if (dependencies && !isRenderProps && !name) { + warning( + false, + 'Form.Item', + 'Must set `name` or use render props when `dependencies` is set.', + ); } else if (React.isValidElement(children)) { const childProps = { ...children.props, ...mergedControl }; @@ -212,69 +279,21 @@ const FormItem: React.FC = (props: FormItemProps) => { }); childNode = React.cloneElement(children, childProps); - } else if (typeof children === 'function' && shouldUpdate && !name) { - childNode = children(context); + } else if (isRenderProps && shouldUpdate && !name) { + childNode = (children as RenderChildren)(context); } else { warning( !mergedName.length, 'Form.Item', '`name` is only used for validate React element. If you are using Form.Item as layout display, please remove `name` instead.', ); - childNode = children; + childNode = children as any; } - if (noStyle) { - return childNode; - } - - return ( - - {/* Label */} - - {/* Input Group */} - - - {childNode} - - - - ); + return renderLayout(childNode, fieldId, meta, isRequired); }} ); -}; +} export default FormItem; diff --git a/components/form/FormItemInput.tsx b/components/form/FormItemInput.tsx index aa534495cb..0fe60539bf 100644 --- a/components/form/FormItemInput.tsx +++ b/components/form/FormItemInput.tsx @@ -18,8 +18,6 @@ interface FormItemInputMiscProps { prefixCls: string; children: React.ReactNode; errors: React.ReactNode[]; - touched: boolean; - validating: boolean; hasFeedback?: boolean; validateStatus?: ValidateStatus; onDomErrorVisibleChange: (visible: boolean) => void; @@ -59,12 +57,16 @@ const FormItemInput: React.FC = ({ const className = classNames(`${baseClassName}-control`, mergedWrapperCol.className); - const [visible, cacheErrors] = useCacheErrors(errors, changedVisible => { - if (changedVisible) { - onDomErrorVisibleChange(true); - } - forceUpdate({}); - }, !!help); + const [visible, cacheErrors] = useCacheErrors( + errors, + changedVisible => { + if (changedVisible) { + onDomErrorVisibleChange(true); + } + forceUpdate({}); + }, + !!help, + ); const memoErrors = useMemo( () => cacheErrors, diff --git a/components/form/FormItemLabel.tsx b/components/form/FormItemLabel.tsx index 25764bf34a..ae4ef3fbc4 100644 --- a/components/form/FormItemLabel.tsx +++ b/components/form/FormItemLabel.tsx @@ -12,7 +12,7 @@ export interface FormItemLabelProps { labelCol?: ColProps; } -const FormItemLabel: React.FC = ({ +const FormItemLabel: React.FC = ({ prefixCls, label, htmlFor, diff --git a/components/form/__tests__/index.test.js b/components/form/__tests__/index.test.js index 121cc313d2..5474e91015 100644 --- a/components/form/__tests__/index.test.js +++ b/components/form/__tests__/index.test.js @@ -89,9 +89,11 @@ describe('Form', () => { expect(wrapper.find(Input).length).toBe(2); await change(wrapper, 1, ''); + wrapper.update(); expect(wrapper.find('.ant-form-item-explain').length).toBe(1); await operate('.remove'); + wrapper.update(); expect(wrapper.find(Input).length).toBe(1); expect(wrapper.find('.ant-form-item-explain').length).toBe(0); }); @@ -341,7 +343,61 @@ describe('Form', () => { const wrapper = mount(); wrapper.find('button').simulate('click'); - expect(wrapper.find('.ant-form-item').first().hasClass('ant-form-item-with-help')).toBeTruthy(); + expect( + wrapper + .find('.ant-form-item') + .first() + .hasClass('ant-form-item-with-help'), + ).toBeTruthy(); expect(wrapper.find('.ant-form-item-explain').text()).toEqual('bamboo'); }); + + it('warning when use `dependencies` but `name` is empty & children is not a render props', () => { + mount( +
+ text +
, + ); + expect(errorSpy).toHaveBeenCalledWith( + 'Warning: [antd: Form.Item] Must set `name` or use render props when `dependencies` is set.', + ); + }); + + // https://github.com/ant-design/ant-design/issues/20948 + it('not repeat render when Form.Item is not a real Field', async () => { + const shouldNotRender = jest.fn(); + const StaticInput = () => { + shouldNotRender(); + return ; + }; + + const shouldRender = jest.fn(); + const DynamicInput = () => { + shouldRender(); + return ; + }; + + const formRef = React.createRef(); + + mount( +
+
+ + + + + + +
+
, + ); + + expect(shouldNotRender).toHaveBeenCalledTimes(1); + expect(shouldRender).toHaveBeenCalledTimes(1); + + formRef.current.setFieldsValue({ light: 'bamboo' }); + await Promise.resolve(); + expect(shouldNotRender).toHaveBeenCalledTimes(1); + expect(shouldRender).toHaveBeenCalledTimes(2); + }); });