fix(react): overlay hooks memorised properly to prevent re-renders (#…

…24010) resolves #23741 Co-authored-by: Fabrice <[email protected]>
ionic-team · Oct 5, 2021 · 2c97712 · 2c97712
1 parent f112ad4
commit 2c97712
Show file tree

Hide file tree

Showing 11 changed files with 273 additions and 143 deletions.
diff --git a/packages/react/package.json b/packages/react/package.json
@@ -52,6 +52,7 @@
     "@rollup/plugin-virtual": "^2.0.3",
     "@testing-library/jest-dom": "^5.11.6",
     "@testing-library/react": "^11.2.2",
+    "@testing-library/react-hooks": "^7.0.1",
     "@types/jest": "^26.0.15",
     "@types/node": "^14.0.14",
     "@types/react": "16.14.0",

diff --git a/packages/react/src/hooks/__tests__/hooks.spec.tsx b/packages/react/src/hooks/__tests__/hooks.spec.tsx
@@ -0,0 +1,153 @@
+import { alertController, modalController } from '@ionic/core';
+
+import React from 'react';
+
+import { useController } from '../useController';
+import { useOverlay } from '../useOverlay';
+
+import { useIonActionSheet } from '../useIonActionSheet';
+import type { UseIonActionSheetResult } from '../useIonActionSheet';
+import { useIonAlert } from '../useIonAlert';
+import type { UseIonAlertResult } from '../useIonAlert';
+import { useIonLoading } from '../useIonLoading';
+import type { UseIonLoadingResult } from '../useIonLoading';
+import { useIonModal } from '../useIonModal';
+import type { UseIonModalResult } from '../useIonModal';
+import { useIonPicker } from '../useIonPicker';
+import type { UseIonPickerResult } from '../useIonPicker';
+import { useIonPopover } from '../useIonPopover';
+import type { UseIonPopoverResult } from '../useIonPopover';
+import { useIonToast } from '../useIonToast';
+import type { UseIonToastResult } from '../useIonToast';
+
+import { renderHook } from '@testing-library/react-hooks';
+
+describe('useController', () => {
+  it('should be memorised', () => {
+    const { result, rerender } = renderHook(() =>
+      useController('AlertController', alertController)
+    );
+
+    rerender();
+
+    const [
+      { present: firstPresent, dismiss: firstDismiss },
+      { present: secondPresent, dismiss: secondDismiss },
+    ] = result.all as ReturnType<typeof useController>[];
+
+    expect(firstPresent).toBe(secondPresent);
+    expect(firstDismiss).toBe(secondDismiss);
+  });
+});
+
+describe('useIonActionSheet', () => {
+  it('should be memorised', () => {
+    const { result, rerender } = renderHook(() => useIonActionSheet());
+
+    rerender();
+
+    const [[firstPresent, firstDismiss], [secondPresent, secondDismiss]] =
+      result.all as UseIonActionSheetResult[];
+    expect(firstPresent).toBe(secondPresent);
+    expect(firstDismiss).toBe(secondDismiss);
+  });
+});
+
+describe('useIonAlert', () => {
+  it('should be memorised', () => {
+    const { result, rerender } = renderHook(() => useIonAlert());
+
+    rerender();
+
+    const [[firstPresent, firstDismiss], [secondPresent, secondDismiss]] =
+      result.all as UseIonAlertResult[];
+    expect(firstPresent).toBe(secondPresent);
+    expect(firstDismiss).toBe(secondDismiss);
+  });
+});
+
+describe('useIonLoading', () => {
+  it('should be memorised', () => {
+    const { result, rerender } = renderHook(() => useIonLoading());
+
+    rerender();
+
+    const [[firstPresent, firstDismiss], [secondPresent, secondDismiss]] =
+      result.all as UseIonLoadingResult[];
+    expect(firstPresent).toBe(secondPresent);
+    expect(firstDismiss).toBe(secondDismiss);
+  });
+});
+
+describe('useIonModal', () => {
+  it('should be memorised', () => {
+    const ModalComponent = () => <div />;
+    const { result, rerender } = renderHook(() => useIonModal(ModalComponent, {}));
+
+    rerender();
+
+    const [[firstPresent, firstDismiss], [secondPresent, secondDismiss]] =
+      result.all as UseIonModalResult[];
+    expect(firstPresent).toBe(secondPresent);
+    expect(firstDismiss).toBe(secondDismiss);
+  });
+});
+
+describe('useIonPicker', () => {
+  it('should be memorised', () => {
+    const { result, rerender } = renderHook(() => useIonPicker());
+
+    rerender();
+
+    const [[firstPresent, firstDismiss], [secondPresent, secondDismiss]] =
+      result.all as UseIonPickerResult[];
+    expect(firstPresent).toBe(secondPresent);
+    expect(firstDismiss).toBe(secondDismiss);
+  });
+});
+
+describe('useIonPopover', () => {
+  it('should be memorised', () => {
+    const PopoverComponent = () => <div />;
+    const { result, rerender } = renderHook(() => useIonPopover(PopoverComponent, {}));
+
+    rerender();
+
+    const [[firstPresent, firstDismiss], [secondPresent, secondDismiss]] =
+      result.all as UseIonPopoverResult[];
+    expect(firstPresent).toBe(secondPresent);
+    expect(firstDismiss).toBe(secondDismiss);
+  });
+});
+
+describe('useIonToast', () => {
+  it('should be memorised', () => {
+    const { result, rerender } = renderHook(() => useIonToast());
+
+    rerender();
+
+    const [[firstPresent, firstDismiss], [secondPresent, secondDismiss]] =
+      result.all as UseIonToastResult[];
+    expect(firstPresent).toBe(secondPresent);
+    expect(firstDismiss).toBe(secondDismiss);
+  });
+});
+
+describe('useOverlay', () => {
+  it('should be memorised', () => {
+    const OverlayComponent = () => <div />;
+    const { result, rerender } = renderHook(() =>
+      useOverlay('IonModal', modalController, OverlayComponent, {})
+    );
+
+    rerender();
+
+    const [
+      { present: firstPresent, dismiss: firstDismiss },
+      { present: secondPresent, dismiss: secondDismiss },
+    ] = result.all as ReturnType<typeof useOverlay>[];
+
+    expect(firstPresent).toBe(secondPresent);
+    expect(firstDismiss).toBe(secondDismiss);
+  });
+});
diff --git a/packages/react/src/hooks/useController.ts b/packages/react/src/hooks/useController.ts
@@ -1,5 +1,5 @@
 import { OverlayEventDetail } from '@ionic/core';
-import { useMemo, useRef } from 'react';
+import { useCallback, useMemo, useRef } from 'react';
 
 import { attachProps } from '../components/utils';
 
@@ -10,71 +10,53 @@ interface OverlayBase extends HTMLElement {
   dismiss: (data?: any, role?: string | undefined) => Promise<boolean>;
 }
 
-export function useController<
-  OptionsType,
-  OverlayType extends OverlayBase
->(
+export function useController<OptionsType, OverlayType extends OverlayBase>(
   displayName: string,
   controller: { create: (options: OptionsType) => Promise<OverlayType> }
 ) {
   const overlayRef = useRef<OverlayType>();
-  const didDismissEventName = useMemo(
-    () => `on${displayName}DidDismiss`,
-    [displayName]
-  );
-  const didPresentEventName = useMemo(
-    () => `on${displayName}DidPresent`,
-    [displayName]
-  );
-  const willDismissEventName = useMemo(
-    () => `on${displayName}WillDismiss`,
-    [displayName]
-  );
-  const willPresentEventName = useMemo(
-    () => `on${displayName}WillPresent`,
-    [displayName]
-  );
-
-  const present = async (options: OptionsType & HookOverlayOptions) => {
-    if (overlayRef.current) {
-      return;
-    }
-    const {
-      onDidDismiss,
-      onWillDismiss,
-      onDidPresent,
-      onWillPresent,
-      ...rest
-    } = options;
+  const didDismissEventName = useMemo(() => `on${displayName}DidDismiss`, [displayName]);
+  const didPresentEventName = useMemo(() => `on${displayName}DidPresent`, [displayName]);
+  const willDismissEventName = useMemo(() => `on${displayName}WillDismiss`, [displayName]);
+  const willPresentEventName = useMemo(() => `on${displayName}WillPresent`, [displayName]);
 
-    const handleDismiss = (event: CustomEvent<OverlayEventDetail<any>>) => {
-      if (onDidDismiss) {
-        onDidDismiss(event);
+  const present = useCallback(
+    async (options: OptionsType & HookOverlayOptions) => {
+      if (overlayRef.current) {
+        return;
       }
-      overlayRef.current = undefined;
-    }
+      const { onDidDismiss, onWillDismiss, onDidPresent, onWillPresent, ...rest } = options;
 
-    overlayRef.current = await controller.create({
-      ...(rest as any),
-    });
+      const handleDismiss = (event: CustomEvent<OverlayEventDetail<any>>) => {
+        if (onDidDismiss) {
+          onDidDismiss(event);
+        }
+        overlayRef.current = undefined;
+      };
 
-    attachProps(overlayRef.current, {
-      [didDismissEventName]: handleDismiss,
-      [didPresentEventName]: (e: CustomEvent) =>
-        onDidPresent && onDidPresent(e),
-      [willDismissEventName]: (e: CustomEvent) =>
-        onWillDismiss && onWillDismiss(e),
-      [willPresentEventName]: (e: CustomEvent) =>
-        onWillPresent && onWillPresent(e),
-    });
+      overlayRef.current = await controller.create({
+        ...(rest as any),
+      });
 
-    overlayRef.current.present();
-  };
+      attachProps(overlayRef.current, {
+        [didDismissEventName]: handleDismiss,
+        [didPresentEventName]: (e: CustomEvent) => onDidPresent && onDidPresent(e),
+        [willDismissEventName]: (e: CustomEvent) => onWillDismiss && onWillDismiss(e),
+        [willPresentEventName]: (e: CustomEvent) => onWillPresent && onWillPresent(e),
+      });
 
-  const dismiss = async () => {
-    overlayRef.current && await overlayRef.current.dismiss();
-    overlayRef.current = undefined;
-  };
+      overlayRef.current.present();
+    },
+    [controller]
+  );
+
+  const dismiss = useCallback(
+    () => async () => {
+      overlayRef.current && (await overlayRef.current.dismiss());
+      overlayRef.current = undefined;
+    },
+    []
+  );
 
   return {
     present,

diff --git a/packages/react/src/hooks/useIonActionSheet.ts b/packages/react/src/hooks/useIonActionSheet.ts
@@ -1,4 +1,5 @@
 import { ActionSheetButton, ActionSheetOptions, actionSheetController } from '@ionic/core';
+import { useCallback } from 'react';
 
 import { HookOverlayOptions } from './HookOverlayOptions';
 import { useController } from './useController';
@@ -13,23 +14,24 @@ export function useIonActionSheet(): UseIonActionSheetResult {
     actionSheetController
   );
 
-  function present(buttons: ActionSheetButton[], header?: string): void;
-  function present(options: ActionSheetOptions & HookOverlayOptions): void;
-  function present(buttonsOrOptions: ActionSheetButton[] | ActionSheetOptions & HookOverlayOptions, header?: string) {
-    if (Array.isArray(buttonsOrOptions)) {
-      controller.present({
-        buttons: buttonsOrOptions,
-        header
-      });
-    } else {
-      controller.present(buttonsOrOptions);
-    }
-  }
+  const present = useCallback(
+    (
+      buttonsOrOptions: ActionSheetButton[] | (ActionSheetOptions & HookOverlayOptions),
+      header?: string
+    ) => {
+      if (Array.isArray(buttonsOrOptions)) {
+        controller.present({
+          buttons: buttonsOrOptions,
+          header,
+        });
+      } else {
+        controller.present(buttonsOrOptions);
+      }
+    },
+    [controller.present]
+  );
 
-  return [
-    present,
-    controller.dismiss
-  ];
+  return [present, controller.dismiss];
 }
 
 export type UseIonActionSheetResult = [

diff --git a/packages/react/src/hooks/useIonAlert.ts b/packages/react/src/hooks/useIonAlert.ts
@@ -1,4 +1,5 @@
 import { AlertButton, AlertOptions, alertController } from '@ionic/core';
+import { useCallback } from 'react';
 
 import { HookOverlayOptions } from './HookOverlayOptions';
 import { useController } from './useController';
@@ -8,28 +9,23 @@ import { useController } from './useController';
  * @returns Returns the present and dismiss methods in an array
  */
 export function useIonAlert(): UseIonAlertResult {
-  const controller = useController<AlertOptions, HTMLIonAlertElement>(
-    'IonAlert',
-    alertController
-  );
+  const controller = useController<AlertOptions, HTMLIonAlertElement>('IonAlert', alertController);
 
-  function present(message: string, buttons?: AlertButton[]): void;
-  function present(options: AlertOptions & HookOverlayOptions): void;
-  function present(messageOrOptions: string | AlertOptions & HookOverlayOptions, buttons?: AlertButton[]) {
-    if (typeof messageOrOptions === 'string') {
-      controller.present({
-        message: messageOrOptions,
-        buttons: buttons ?? [{ text: 'Ok' }]
-      });
-    } else {
-      controller.present(messageOrOptions);
-    }
-  };
+  const present = useCallback(
+    (messageOrOptions: string | (AlertOptions & HookOverlayOptions), buttons?: AlertButton[]) => {
+      if (typeof messageOrOptions === 'string') {
+        controller.present({
+          message: messageOrOptions,
+          buttons: buttons ?? [{ text: 'Ok' }],
+        });
+      } else {
+        controller.present(messageOrOptions);
+      }
+    },
+    [controller.present]
+  );
 
-  return [
-    present,
-    controller.dismiss
-  ];
+  return [present, controller.dismiss];
 }
 
 export type UseIonAlertResult = [