fix:Not warning if content do not need key (#397)

* test: test driven

* fix: Not warning of useless component

Author: zombieJ

src/MenuItem.tsx

@@ -28,6 +28,9 @@ export interface MenuItemProps
   /** @private Internal filled key. Do not set it directly */
   eventKey?: string;
 
+  /** @private Do not use. Private warning empty usage */
+  warnKey?: boolean;
+
   disabled?: boolean;
 
   itemIcon?: RenderIconType;
@@ -120,6 +123,11 @@ const InternalMenuItem = (props: MenuItemProps) => {
 
   const connectedKeys = useFullPath(eventKey);
 
+  // ================================ Warn ================================
+  if (process.env.NODE_ENV !== 'production' && props.warnKey) {
+    warning(false, 'MenuItem should not leave undefined `key`.');
+  }
+
   // ============================= Info =============================
   const getEventInfo = (
     e: React.MouseEvent<HTMLElement> | React.KeyboardEvent<HTMLElement>,

src/SubMenu/index.tsx

@@ -1,6 +1,7 @@
 import * as React from 'react';
 import classNames from 'classnames';
 import Overflow from 'rc-overflow';
+import warning from 'rc-util/lib/warning';
 import SubMenuList from './SubMenuList';
 import { parseChildren } from '../utils/nodeUtil';
 import type {
@@ -40,6 +41,9 @@ export interface SubMenuProps {
   /** @private Internal filled key. Do not set it directly */
   eventKey?: string;
 
+  /** @private Do not use. Private warning empty usage */
+  warnKey?: boolean;
+
   // >>>>> Icon
   itemIcon?: RenderIconType;
   expandIcon?: RenderIconType;
@@ -130,6 +134,11 @@ const InternalSubMenu = (props: SubMenuProps) => {
   const elementRef = React.useRef<HTMLDivElement>();
   const popupRef = React.useRef<HTMLUListElement>();
 
+  // ================================ Warn ================================
+  if (process.env.NODE_ENV !== 'production' && props.warnKey) {
+    warning(false, 'SubMenu should not leave undefined `key`.');
+  }
+
   // ================================ Icon ================================
   const mergedItemIcon = itemIcon || contextItemIcon;
   const mergedExpandIcon = expandIcon || contextExpandIcon;
@@ -158,23 +167,25 @@ const InternalSubMenu = (props: SubMenuProps) => {
     }
   };
 
-  const onInternalMouseEnter: React.MouseEventHandler<HTMLLIElement> = domEvent => {
-    triggerChildrenActive(true);
-
-    onMouseEnter?.({
-      key: eventKey,
-      domEvent,
-    });
-  };
-
-  const onInternalMouseLeave: React.MouseEventHandler<HTMLLIElement> = domEvent => {
-    triggerChildrenActive(false);
-
-    onMouseLeave?.({
-      key: eventKey,
-      domEvent,
-    });
-  };
+  const onInternalMouseEnter: React.MouseEventHandler<HTMLLIElement> =
+    domEvent => {
+      triggerChildrenActive(true);
+
+      onMouseEnter?.({
+        key: eventKey,
+        domEvent,
+      });
+    };
+
+  const onInternalMouseLeave: React.MouseEventHandler<HTMLLIElement> =
+    domEvent => {
+      triggerChildrenActive(false);
+
+      onMouseLeave?.({
+        key: eventKey,
+        domEvent,
+      });
+    };
 
   const mergedActive = React.useMemo(() => {
     if (active) {

src/utils/nodeUtil.ts

@@ -1,26 +1,28 @@
 import * as React from 'react';
-import warning from 'rc-util/lib/warning';
 import toArray from 'rc-util/lib/Children/toArray';
 
 export function parseChildren(children: React.ReactNode, keyPath: string[]) {
   return toArray(children).map((child, index) => {
     if (React.isValidElement(child)) {
-      let { key } = child;
-      if (key === null || key === undefined) {
-        key = `tmp_key-${[...keyPath, index].join('-')}`;
+      const { key } = child;
+      let eventKey = (child.props as any)?.eventKey ?? key;
 
-        if (process.env.NODE_ENV !== 'production') {
-          warning(
-            false,
-            'MenuItem or SubMenu should not leave undefined `key`.',
-          );
-        }
+      const emptyKey = eventKey === null || eventKey === undefined;
+
+      if (emptyKey) {
+        eventKey = `tmp_key-${[...keyPath, index].join('-')}`;
+      }
+
+      const cloneProps = {
+        key: eventKey,
+        eventKey,
+      } as any;
+
+      if (process.env.NODE_ENV !== 'production' && emptyKey) {
+        cloneProps.warnKey = true;
       }
 
-      return React.cloneElement(child, {
-        key,
-        eventKey: key,
-      } as any);
+      return React.cloneElement(child, cloneProps);
     }
 
     return child;

tests/SubMenu.spec.js

@@ -3,6 +3,7 @@ import React from 'react';
 import { mount } from 'enzyme';
 import { act } from 'react-dom/test-utils';
 import Menu, { MenuItem, SubMenu } from '../src';
+import { resetWarned } from 'rc-util/lib/warning';
 
 describe('SubMenu', () => {
   beforeEach(() => {
@@ -160,8 +161,6 @@ describe('SubMenu', () => {
   });
 
   it('fires openChange event', () => {
-    const errorSpy = jest.spyOn(console, 'error').mockImplementation(() => {});
-
     const handleOpenChange = jest.fn();
     const wrapper = mount(
       <Menu onOpenChange={handleOpenChange}>
@@ -193,12 +192,66 @@ describe('SubMenu', () => {
       'tmp_key-1',
       'tmp_key-tmp_key-1-1',
     ]);
+  });
 
-    expect(errorSpy).toHaveBeenCalledWith(
-      'Warning: MenuItem or SubMenu should not leave undefined `key`.',
-    );
+  describe('undefined key', () => {
+    it('warning item', () => {
+      resetWarned();
+
+      const errorSpy = jest
+        .spyOn(console, 'error')
+        .mockImplementation(() => {});
+
+      mount(
+        <Menu>
+          <MenuItem>1</MenuItem>
+        </Menu>,
+      );
 
-    errorSpy.mockRestore();
+      expect(errorSpy).toHaveBeenCalledWith(
+        'Warning: MenuItem should not leave undefined `key`.',
+      );
+
+      errorSpy.mockRestore();
+    });
+
+    it('warning sub menu', () => {
+      resetWarned();
+
+      const errorSpy = jest
+        .spyOn(console, 'error')
+        .mockImplementation(() => {});
+
+      mount(
+        <Menu>
+          <SubMenu />
+        </Menu>,
+      );
+
+      expect(errorSpy).toHaveBeenCalledWith(
+        'Warning: SubMenu should not leave undefined `key`.',
+      );
+
+      errorSpy.mockRestore();
+    });
+
+    it('should not warning', () => {
+      resetWarned();
+
+      const errorSpy = jest
+        .spyOn(console, 'error')
+        .mockImplementation(() => {});
+
+      mount(
+        <Menu>
+          <Menu.Divider />
+        </Menu>,
+      );
+
+      expect(errorSpy).not.toHaveBeenCalled();
+
+      errorSpy.mockRestore();
+    });
   });
 
   describe('mouse events', () => {