fix: auto close with maxCount not work (#136)

* fix: Notice close should keep new key

* test: move pos

* test: Update test case

Author: zombieJ

src/Notice.tsx

@@ -15,11 +15,13 @@ export interface NoticeProps {
   duration?: number | null;
   children?: React.ReactNode;
   updateMark?: string;
+  /** Mark as final key since set maxCount may keep the key but user pass key is different */
+  noticeKey: React.Key;
   closeIcon?: React.ReactNode;
   closable?: boolean;
   props?: DivProps;
   onClick?: React.MouseEventHandler<HTMLDivElement>;
-  onClose?: () => void;
+  onClose?: (key: React.Key) => void;
 
   /** @private Only for internal usage. We don't promise that we will refactor this */
   holder?: HTMLDivElement;
@@ -55,9 +57,9 @@ export default class Notice extends Component<NoticeProps> {
       e.stopPropagation();
     }
     this.clearCloseTimer();
-    const { onClose } = this.props;
+    const { onClose, noticeKey } = this.props;
     if (onClose) {
-      onClose();
+      onClose(noticeKey);
     }
   };
 

src/Notification.tsx

@@ -3,7 +3,6 @@ import { Component, ReactText } from 'react';
 import ReactDOM from 'react-dom';
 import classNames from 'classnames';
 import { CSSMotionList } from 'rc-motion';
-import createChainedFunction from 'rc-util/lib/createChainedFunction';
 import Notice, { NoticeProps } from './Notice';
 import useNotification from './useNotification';
 
@@ -16,11 +15,13 @@ function getUuid() {
   return `rcNotification_${now}_${id}`;
 }
 
-export interface NoticeContent extends Omit<NoticeProps, 'prefixCls' | 'children'> {
+export interface NoticeContent
+  extends Omit<NoticeProps, 'prefixCls' | 'children' | 'noticeKey' | 'onClose'> {
   prefixCls?: string;
   key?: React.Key;
   updateMark?: string;
   content?: React.ReactNode;
+  onClose?: () => void;
 }
 
 export type NoticeFunc = (noticeProps: NoticeContent) => void;
@@ -94,9 +95,9 @@ class Notification extends Component<NotificationProps, NotificationState> {
       key,
     };
     const { maxCount } = this.props;
-    this.setState((previousState) => {
+    this.setState(previousState => {
       const { notices } = previousState;
-      const noticeIndex = notices.map((v) => v.notice.key).indexOf(key);
+      const noticeIndex = notices.map(v => v.notice.key).indexOf(key);
       const updatedNotices = notices.concat();
       if (noticeIndex !== -1) {
         updatedNotices.splice(noticeIndex, 1, { notice, holderCallback });
@@ -155,18 +156,20 @@ class Notification extends Component<NotificationProps, NotificationState> {
 
     notices.forEach(({ notice, holderCallback }, index) => {
       const updateMark = index === notices.length - 1 ? notice.updateMark : undefined;
-      const { key } = notice;
-
-      const onClose = createChainedFunction(this.remove.bind(this, key), notice.onClose) as any;
+      const { key, userPassKey } = notice;
 
       const noticeProps = {
         prefixCls,
         closeIcon,
         ...notice,
         ...notice.props,
         key,
+        noticeKey: userPassKey || key,
         updateMark,
-        onClose,
+        onClose: (noticeKey: React.Key) => {
+          this.remove(noticeKey);
+          notice.onClose?.();
+        },
         onClick: notice.onClick,
         children: notice.content,
       } as NoticeProps & { key: ReactText };
@@ -195,7 +198,7 @@ class Notification extends Component<NotificationProps, NotificationState> {
                   key={key}
                   className={classNames(motionClassName, `${prefixCls}-hook-holder`)}
                   style={{ ...motionStyle }}
-                  ref={(div) => {
+                  ref={div => {
                     if (typeof key === 'undefined') {
                       return;
                     }

tests/index.test.js

@@ -176,46 +176,6 @@ describe('Notification.Basic', () => {
     );
   });
 
-  it('remove work when maxCount set', done => {
-    let wrapper;
-
-    Notification.newInstance(
-      {
-        TEST_RENDER: node => {
-          wrapper = mount(<div>{node}</div>);
-        },
-        maxCount: 1,
-      },
-      notification => {
-        // First
-        notification.notice({
-          content: <div className="max-count">bamboo</div>,
-          key: 'bamboo',
-          duration: 0,
-        });
-
-        // Next
-        notification.notice({
-          content: <div className="max-count">bamboo</div>,
-          key: 'bamboo',
-          duration: 0,
-        });
-
-        setTimeout(() => {
-          expect(wrapper.find('.max-count')).toHaveLength(1);
-          notification.removeNotice('bamboo');
-
-          setTimeout(() => {
-            wrapper.update();
-            expect(wrapper.find('.max-count')).toHaveLength(0);
-            notification.destroy();
-            done();
-          }, 500);
-        }, 10);
-      },
-    );
-  });
-
   it('update notification by key with multi instance', done => {
     let wrapper;
 
@@ -341,47 +301,133 @@ describe('Notification.Basic', () => {
     mount(<Test />, container);
   });
 
-  it('drop first notice when items limit exceeds', () => {
-    jest.useFakeTimers();
+  describe('maxCount', () => {
+    it('remove work when maxCount set', done => {
+      let wrapper;
 
-    let wrapper;
-
-    let notificationInstance;
-    Notification.newInstance(
-      {
-        maxCount: 1,
-        TEST_RENDER: node => {
-          wrapper = mount(<div>{node}</div>);
+      Notification.newInstance(
+        {
+          TEST_RENDER: node => {
+            wrapper = mount(<div>{node}</div>);
+          },
+          maxCount: 1,
         },
-      },
-      notification => {
-        notificationInstance = notification;
-      },
-    );
+        notification => {
+          // First
+          notification.notice({
+            content: <div className="max-count">bamboo</div>,
+            key: 'bamboo',
+            duration: 0,
+          });
 
-    const value = 'updated last';
-    notificationInstance.notice({
-      content: <span className="test-maxcount">simple show</span>,
-      duration: 0,
-    });
-    notificationInstance.notice({
-      content: <span className="test-maxcount">simple show</span>,
-      duration: 0,
-    });
-    notificationInstance.notice({
-      content: <span className="test-maxcount">{value}</span>,
-      duration: 0,
+          // Next
+          notification.notice({
+            content: <div className="max-count">bamboo</div>,
+            key: 'bamboo',
+            duration: 0,
+          });
+
+          setTimeout(() => {
+            expect(wrapper.find('.max-count')).toHaveLength(1);
+            notification.removeNotice('bamboo');
+
+            setTimeout(() => {
+              wrapper.update();
+              expect(wrapper.find('.max-count')).toHaveLength(0);
+              notification.destroy();
+              done();
+            }, 500);
+          }, 10);
+        },
+      );
     });
 
-    act(() => {
-      jest.runAllTimers();
-      wrapper.update();
+    it('drop first notice when items limit exceeds', () => {
+      jest.useFakeTimers();
+
+      let wrapper;
+
+      let notificationInstance;
+      Notification.newInstance(
+        {
+          maxCount: 1,
+          TEST_RENDER: node => {
+            wrapper = mount(<div>{node}</div>);
+          },
+        },
+        notification => {
+          notificationInstance = notification;
+        },
+      );
+
+      const value = 'updated last';
+      notificationInstance.notice({
+        content: <span className="test-maxcount">simple show</span>,
+        duration: 0,
+      });
+      notificationInstance.notice({
+        content: <span className="test-maxcount">simple show</span>,
+        duration: 0,
+      });
+      notificationInstance.notice({
+        content: <span className="test-maxcount">{value}</span>,
+        duration: 0,
+      });
+
+      act(() => {
+        jest.runAllTimers();
+        wrapper.update();
+      });
+
+      expect(wrapper.find('.test-maxcount')).toHaveLength(1);
+      expect(wrapper.find('.test-maxcount').text()).toEqual(value);
+
+      jest.useRealTimers();
     });
 
-    expect(wrapper.find('.test-maxcount')).toHaveLength(1);
-    expect(wrapper.find('.test-maxcount').text()).toEqual(value);
+    it('duration should work', done => {
+      let wrapper;
+
+      let notificationInstance;
+      Notification.newInstance(
+        {
+          maxCount: 1,
+          TEST_RENDER: node => {
+            wrapper = mount(<div>{node}</div>);
+          },
+        },
+        notification => {
+          notificationInstance = notification;
+
+          notificationInstance.notice({
+            content: <span className="auto-remove">bamboo</span>,
+            duration: 99,
+          });
+
+          setTimeout(() => {
+            wrapper.update();
+            expect(wrapper.find('.auto-remove').text()).toEqual('bamboo');
+
+            notificationInstance.notice({
+              content: <span className="auto-remove">light</span>,
+              duration: 0.5,
+            });
 
-    jest.useRealTimers();
+            setTimeout(() => {
+              wrapper.update();
+              expect(wrapper.find('.auto-remove').text()).toEqual('light');
+
+              setTimeout(() => {
+                wrapper.update();
+                expect(wrapper.find('.auto-remove')).toHaveLength(0);
+                notification.destroy();
+                done();
+              }, 500);
+            }, 10);
+          }, 10);
+        },
+      );
+    });
   });
 
   it('onClick trigger', done => {