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
2024-11-24 19:19:57 +08:00 · 2020-01-17 11:50:06 +08:00 · 2020-01-17 11:50:06 +08:00 · b67a1bee7d
commit b67a1bee7d
parent 55bbe3d113
4 changed files with 202 additions and 125 deletions
--- 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<FieldProps, 'children'>;
+type ChildrenType = React.ReactElement | RenderChildren | React.ReactElement[] | null;
+type ChildrenNodeType = Exclude<ChildrenType, RenderChildren>;

 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<FormItemProps> = (props: FormItemProps) => {
+function FormItem(props: FormItemProps): React.ReactNode {
  const {
    name,
    fieldKey,
@ -77,6 +80,119 @@ const FormItem: React.FC<FormItemProps> = (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 (
+      <Row
+        className={classNames(itemClassName)}
+        style={style}
+        key="row"
+        {...omit(restProps, [
+          'colon',
+          'extra',
+          'getValueFromEvent',
+          'hasFeedback',
+          'help',
+          'htmlFor',
+          'id', // It is deprecated because `htmlFor` is its replacement.
+          'label',
+          'labelAlign',
+          'labelCol',
+          'normalize',
+          'required',
+          'validateStatus',
+          'valuePropName',
+          'wrapperCol',
+        ])}
+      >
+        {/* Label */}
+        <FormItemLabel htmlFor={fieldId} {...props} required={isRequired} prefixCls={prefixCls} />
+        {/* Input Group */}
+        <FormItemInput
+          {...props}
+          {...meta}
+          errors={mergedErrors}
+          prefixCls={prefixCls}
+          onDomErrorVisibleChange={setDomErrorVisible}
+          validateStatus={mergedValidateStatus}
+        >
+          <FormItemContext.Provider value={{ updateItemErrors: updateChildItemErrors }}>
+            {baseChildren}
+          </FormItemContext.Provider>
+        </FormItemInput>
+      </Row>
+    );
+  }
+
+  const isRenderProps = typeof children === 'function';
+
+  if (!name && !isRenderProps && !dependencies) {
+    return renderLayout(children as ChildrenNodeType);
+  }
+
  return (
    <Field
      {...props}
@ -87,21 +203,10 @@ const FormItem: React.FC<FormItemProps> = (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<FormItemProps> = (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<FormItemProps> = (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<FormItemProps> = (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<FormItemProps> = (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 (
-          <Row
-            className={classNames(itemClassName)}
-            style={style}
-            key="row"
-            {...omit(restProps, [
-              'colon',
-              'extra',
-              'getValueFromEvent',
-              'hasFeedback',
-              'help',
-              'htmlFor',
-              'id', // It is deprecated because `htmlFor` is its replacement.
-              'label',
-              'labelAlign',
-              'labelCol',
-              'normalize',
-              'required',
-              'validateStatus',
-              'valuePropName',
-              'wrapperCol',
-            ])}
-          >
-            {/* Label */}
-            <FormItemLabel
-              htmlFor={fieldId}
-              {...props}
-              required={isRequired}
-              prefixCls={prefixCls}
-            />
-            {/* Input Group */}
-            <FormItemInput
-              {...props}
-              {...meta}
-              errors={mergedErrors}
-              prefixCls={prefixCls}
-              onDomErrorVisibleChange={setDomErrorVisible}
-              validateStatus={mergedValidateStatus}
-            >
-              <FormItemContext.Provider value={{ updateItemErrors: updateChildItemErrors }}>
-                {childNode}
-              </FormItemContext.Provider>
-            </FormItemInput>
-          </Row>
-        );
+        return renderLayout(childNode, fieldId, meta, isRequired);
      }}
    </Field>
  );
-};
+}

 export default FormItem;
--- 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<FormItemInputProps & FormItemInputMiscProps> = ({

  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,
--- a/components/form/FormItemLabel.tsx
+++ b/components/form/FormItemLabel.tsx
@ -12,7 +12,7 @@ export interface FormItemLabelProps {
  labelCol?: ColProps;
 }

-const FormItemLabel: React.FC<FormItemLabelProps & { required: boolean; prefixCls: string }> = ({
+const FormItemLabel: React.FC<FormItemLabelProps & { required?: boolean; prefixCls: string }> = ({
  prefixCls,
  label,
  htmlFor,
--- 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(<App />);
    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(
+      <Form>
+        <Form.Item dependencies={[]}>text</Form.Item>
+      </Form>,
+    );
+    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 <Input />;
+    };
+
+    const shouldRender = jest.fn();
+    const DynamicInput = () => {
+      shouldRender();
+      return <Input />;
+    };
+
+    const formRef = React.createRef();
+
+    mount(
+      <div>
+        <Form ref={formRef}>
+          <Form.Item>
+            <StaticInput />
+          </Form.Item>
+          <Form.Item name="light">
+            <DynamicInput />
+          </Form.Item>
+        </Form>
+      </div>,
+    );
+
+    expect(shouldNotRender).toHaveBeenCalledTimes(1);
+    expect(shouldRender).toHaveBeenCalledTimes(1);
+
+    formRef.current.setFieldsValue({ light: 'bamboo' });
+    await Promise.resolve();
+    expect(shouldNotRender).toHaveBeenCalledTimes(1);
+    expect(shouldRender).toHaveBeenCalledTimes(2);
+  });
 });