chore: Should not re-render when dialog is invisible (#212)

* fix: Dialog should not update children when !visible

* chore: test to tsx

* test: fix test case

Author: zombieJ

examples/bootstrap.tsx

@@ -3,6 +3,12 @@ import '../assets/bootstrap.less';
 import * as React from 'react';
 import Dialog from '../src/DialogWrap';
 
+// Check for memo update should work
+const InnerRender = () => {
+  console.log('Updated...', Date.now());
+  return null;
+};
+
 const MyControl = () => {
   const [visible, setVisible] = React.useState(false);
   const [destroyOnClose, setDestroyOnClose] = React.useState(false);
@@ -37,6 +43,7 @@ const MyControl = () => {
         </button>,
       ]}
     >
+      <InnerRender />
       <h4>Text in a modal</h4>
       <p>Duis mollis, est non commodo luctus, nisi erat porttitor ligula.</p>
       <hr />

src/Dialog/Content/MemoChildren.tsx

@@ -0,0 +1,11 @@
+import * as React from 'react';
+
+export interface MemoChildrenProps {
+  visible: boolean;
+  children: React.ReactNode;
+}
+
+export default React.memo(
+  ({ children }: MemoChildrenProps) => children as React.ReactElement,
+  (_, { visible }) => !visible,
+);

src/Dialog/Content.tsx renamed to src/Dialog/Content/index.tsx

@@ -2,8 +2,9 @@ import * as React from 'react';
 import { useRef } from 'react';
 import classNames from 'classnames';
 import CSSMotion from 'rc-motion';
-import { IDialogChildProps } from '.';
-import { offset } from '../util';
+import { IDialogChildProps } from '..';
+import { offset } from '../../util';
+import MemoChildren from './MemoChildren';
 
 const sentinelStyle = { width: 0, height: 0, overflow: 'hidden', outline: 'none' };
 
@@ -147,7 +148,9 @@ const Content = React.forwardRef<ContentRef, ContentProps>((props, ref) => {
           onClick={onClick}
         >
           <div tabIndex={0} ref={sentinelStartRef} style={sentinelStyle} aria-hidden="true" />
-          {modalRender ? modalRender(content) : content}
+          <MemoChildren visible={visible}>
+            {modalRender ? modalRender(content) : content}
+          </MemoChildren>
           <div tabIndex={0} ref={sentinelEndRef} style={sentinelStyle} aria-hidden="true" />
         </div>
       )}

tests/index.spec.js renamed to tests/index.spec.tsx

@@ -90,8 +90,11 @@ describe('dialog', () => {
       jest.runAllTimers();
       wrapper.update();
 
-      document.getElementsByClassName('.test-input').value = 'test';
-      expect(document.getElementsByClassName('.test-input').value).toBe('test');
+      ((document.getElementsByClassName('.test-input') as unknown) as HTMLInputElement).value =
+        'test';
+      expect(
+        ((document.getElementsByClassName('.test-input') as unknown) as HTMLInputElement).value,
+      ).toBe('test');
 
       // Hide
       wrapper.setProps({ visible: false });
@@ -103,7 +106,9 @@ describe('dialog', () => {
       jest.runAllTimers();
       wrapper.update();
 
-      expect(document.getElementsByClassName('.test-input').value).toBeUndefined();
+      expect(
+        ((document.getElementsByClassName('.test-input') as unknown) as HTMLInputElement).value,
+      ).toBeUndefined();
       wrapper.unmount();
     });
   });
@@ -201,7 +206,9 @@ describe('dialog', () => {
   describe('Tab should keep focus in dialog', () => {
     it('basic tabbing', () => {
       const wrapper = mount(<Dialog visible />, { attachTo: document.body });
-      const sentinelEnd = document.querySelectorAll('.rc-dialog-content + div')[0];
+      const sentinelEnd = (document.querySelectorAll(
+        '.rc-dialog-content + div',
+      )[0] as unknown) as HTMLDivElement;
       sentinelEnd.focus();
 
       wrapper.find('.rc-dialog-wrap').simulate('keyDown', {
@@ -237,16 +244,13 @@ describe('dialog', () => {
     // Trigger position align
     act(() => {
       wrapper
-        .find('Content CSSMotion')
+        .find<any>('Content CSSMotion' as any)
         .props()
         .onAppearPrepare();
     });
 
     expect(
-      wrapper
-        .find('.rc-dialog')
-        .at(0)
-        .getDOMNode().style['transform-origin'],
+      (wrapper.find('.rc-dialog').at(0).getDOMNode() as HTMLDivElement).style['transform-origin'],
     ).toBeTruthy();
   });
 
@@ -318,7 +322,7 @@ describe('dialog', () => {
     }
 
     const wrapper = mount(
-      <DialogWrapTest visible forceRender>
+      <DialogWrapTest>
         <div>Show dialog with forceRender and visible is true</div>
       </DialogWrapTest>,
     );
@@ -331,7 +335,7 @@ describe('dialog', () => {
     const modalRender = mount(
       <Dialog
         visible
-        modalRender={node =>
+        modalRender={(node: React.ReactElement) =>
           cloneElement(node, { ...node.props, style: { background: '#1890ff' } })
         }
       />,
@@ -350,4 +354,24 @@ describe('dialog', () => {
       expect(wrapper.find('.rc-dialog').props().style.height).toEqual(903);
     });
   });
+
+  it('should not re-render when visible changed', () => {
+    let renderTimes = 0;
+    const RenderChecker = () => {
+      renderTimes += 1;
+      return null;
+    };
+
+    const wrapper = mount(
+      <Dialog visible>
+        <RenderChecker />
+      </Dialog>,
+    );
+
+    expect(renderTimes).toEqual(1);
+
+    // Hidden should not trigger render
+    wrapper.setProps({ visible: false });
+    expect(renderTimes).toEqual(1);
+  });
 });