From 2c977126012ae0231d4e4fa63cc76a528bde699b Mon Sep 17 00:00:00 2001 From: Ely Lucas Date: Tue, 5 Oct 2021 06:44:40 -0600 Subject: [PATCH] fix(react): overlay hooks memorised properly to prevent re-renders (#24010) resolves #23741 Co-authored-by: Fabrice --- packages/react/package.json | 1 + .../react/src/hooks/__tests__/hooks.spec.tsx | 153 ++++++++++++++++++ packages/react/src/hooks/useController.ts | 100 +++++------- packages/react/src/hooks/useIonActionSheet.ts | 34 ++-- packages/react/src/hooks/useIonAlert.ts | 36 ++--- packages/react/src/hooks/useIonLoading.tsx | 40 +++-- packages/react/src/hooks/useIonModal.ts | 20 ++- packages/react/src/hooks/useIonPicker.tsx | 16 +- packages/react/src/hooks/useIonPopover.ts | 5 +- packages/react/src/hooks/useIonToast.ts | 7 +- packages/react/src/hooks/useOverlay.ts | 10 +- 11 files changed, 276 insertions(+), 146 deletions(-) create mode 100644 packages/react/src/hooks/__tests__/hooks.spec.tsx diff --git a/packages/react/package.json b/packages/react/package.json index 875adc9eba..4b209ab97e 100644 --- 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 new file mode 100644 index 0000000000..28a570b490 --- /dev/null +++ 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[]; + + 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 = () =>
; + 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 = () =>
; + 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 = () =>
; + const { result, rerender } = renderHook(() => + useOverlay('IonModal', modalController, OverlayComponent, {}) + ); + + rerender(); + + const [ + { present: firstPresent, dismiss: firstDismiss }, + { present: secondPresent, dismiss: secondDismiss }, + ] = result.all as ReturnType[]; + + expect(firstPresent).toBe(secondPresent); + expect(firstDismiss).toBe(secondDismiss); + }); +}); diff --git a/packages/react/src/hooks/useController.ts b/packages/react/src/hooks/useController.ts index fa596f3704..39da229483 100644 --- 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; } -export function useController< - OptionsType, - OverlayType extends OverlayBase ->( +export function useController( displayName: string, controller: { create: (options: OptionsType) => Promise } ) { const overlayRef = useRef(); - 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 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 handleDismiss = (event: CustomEvent>) => { - if (onDidDismiss) { - onDidDismiss(event); + const present = useCallback( + async (options: OptionsType & HookOverlayOptions) => { + if (overlayRef.current) { + return; } + const { onDidDismiss, onWillDismiss, onDidPresent, onWillPresent, ...rest } = options; + + const handleDismiss = (event: CustomEvent>) => { + if (onDidDismiss) { + onDidDismiss(event); + } + overlayRef.current = undefined; + }; + + overlayRef.current = await controller.create({ + ...(rest as any), + }); + + 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.present(); + }, + [controller] + ); + + const dismiss = useCallback( + () => async () => { + overlayRef.current && (await overlayRef.current.dismiss()); overlayRef.current = undefined; - } - - overlayRef.current = await controller.create({ - ...(rest as any), - }); - - 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.present(); - }; - - const dismiss = 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 index cbd045f4ef..e0084e9634 100644 --- 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 index 3f1f7cf7fc..2303de3dc7 100644 --- 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( - 'IonAlert', - alertController + const controller = useController('IonAlert', alertController); + + 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] ); - 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); - } - }; - - return [ - present, - controller.dismiss - ]; + return [present, controller.dismiss]; } export type UseIonAlertResult = [ diff --git a/packages/react/src/hooks/useIonLoading.tsx b/packages/react/src/hooks/useIonLoading.tsx index 779a413c18..aea4d89f07 100644 --- a/packages/react/src/hooks/useIonLoading.tsx +++ b/packages/react/src/hooks/useIonLoading.tsx @@ -1,4 +1,5 @@ import { LoadingOptions, SpinnerTypes, loadingController } from '@ionic/core'; +import { useCallback } from 'react'; import { HookOverlayOptions } from './HookOverlayOptions'; import { useController } from './useController'; @@ -13,27 +14,24 @@ export function useIonLoading(): UseIonLoadingResult { loadingController ); - function present( - message?: string, - duration?: number, - spinner?: SpinnerTypes - ): void; - function present(options: LoadingOptions & HookOverlayOptions): void; - function present( - messageOrOptions: string | (LoadingOptions & HookOverlayOptions) = '', - duration?: number, - spinner?: SpinnerTypes - ) { - if (typeof messageOrOptions === 'string') { - controller.present({ - message: messageOrOptions, - duration, - spinner: spinner ?? 'lines', - }); - } else { - controller.present(messageOrOptions); - } - } + const present = useCallback( + ( + messageOrOptions: string | (LoadingOptions & HookOverlayOptions) = '', + duration?: number, + spinner?: SpinnerTypes + ) => { + if (typeof messageOrOptions === 'string') { + controller.present({ + message: messageOrOptions, + duration, + spinner: spinner ?? 'lines', + }); + } else { + controller.present(messageOrOptions); + } + }, + [controller.present] + ); return [present, controller.dismiss]; } diff --git a/packages/react/src/hooks/useIonModal.ts b/packages/react/src/hooks/useIonModal.ts index 4a84969aca..dc6a101880 100644 --- a/packages/react/src/hooks/useIonModal.ts +++ b/packages/react/src/hooks/useIonModal.ts @@ -1,4 +1,5 @@ import { ModalOptions, modalController } from '@ionic/core'; +import { useCallback } from 'react'; import { HookOverlayOptions } from './HookOverlayOptions'; import { ReactComponentOrElement, useOverlay } from './useOverlay'; @@ -9,7 +10,10 @@ import { ReactComponentOrElement, useOverlay } from './useOverlay'; * @param componentProps The props that will be passed to the component, if required * @returns Returns the present and dismiss methods in an array */ -export function useIonModal(component: ReactComponentOrElement, componentProps?: any): UseIonModalResult { +export function useIonModal( + component: ReactComponentOrElement, + componentProps?: any +): UseIonModalResult { const controller = useOverlay( 'IonModal', modalController, @@ -17,14 +21,14 @@ export function useIonModal(component: ReactComponentOrElement, componentProps?: componentProps ); - function present(options: Omit & HookOverlayOptions = {}) { - controller.present(options as any); - }; + const present = useCallback( + (options: Omit & HookOverlayOptions = {}) => { + controller.present(options as any); + }, + [controller.present] + ); - return [ - present, - controller.dismiss - ]; + return [present, controller.dismiss]; } export type UseIonModalResult = [ diff --git a/packages/react/src/hooks/useIonPicker.tsx b/packages/react/src/hooks/useIonPicker.tsx index 36473862aa..130981142c 100644 --- a/packages/react/src/hooks/useIonPicker.tsx +++ b/packages/react/src/hooks/useIonPicker.tsx @@ -1,9 +1,5 @@ -import { - PickerButton, - PickerColumn, - PickerOptions, - pickerController, -} from '@ionic/core'; +import { PickerButton, PickerColumn, PickerOptions, pickerController } from '@ionic/core'; +import { useCallback } from 'react'; import { HookOverlayOptions } from './HookOverlayOptions'; import { useController } from './useController'; @@ -18,12 +14,10 @@ export function useIonPicker(): UseIonPickerResult { pickerController ); - function present(columns: PickerColumn[], buttons?: PickerButton[]): void; - function present(options: PickerOptions & HookOverlayOptions): void; - function present( + const present = useCallback(( columnsOrOptions: PickerColumn[] | (PickerOptions & HookOverlayOptions), buttons?: PickerButton[] - ) { + ) => { if (Array.isArray(columnsOrOptions)) { controller.present({ columns: columnsOrOptions, @@ -32,7 +26,7 @@ export function useIonPicker(): UseIonPickerResult { } else { controller.present(columnsOrOptions); } - } + }, [controller.present]); return [present, controller.dismiss]; } diff --git a/packages/react/src/hooks/useIonPopover.ts b/packages/react/src/hooks/useIonPopover.ts index 42ecf6600c..ca2b369781 100644 --- a/packages/react/src/hooks/useIonPopover.ts +++ b/packages/react/src/hooks/useIonPopover.ts @@ -1,4 +1,5 @@ import { PopoverOptions, popoverController } from '@ionic/core'; +import { useCallback } from 'react'; import { HookOverlayOptions } from './HookOverlayOptions'; import { ReactComponentOrElement, useOverlay } from './useOverlay'; @@ -17,9 +18,9 @@ export function useIonPopover(component: ReactComponentOrElement, componentProps componentProps ); - function present(options: Omit & HookOverlayOptions = {}) { + const present = useCallback((options: Omit & HookOverlayOptions = {}) => { controller.present(options as any); - }; + }, [controller.present]); return [ present, diff --git a/packages/react/src/hooks/useIonToast.ts b/packages/react/src/hooks/useIonToast.ts index 258d5b2bc8..5f6578ad6d 100644 --- a/packages/react/src/hooks/useIonToast.ts +++ b/packages/react/src/hooks/useIonToast.ts @@ -1,4 +1,5 @@ import { ToastOptions, toastController } from '@ionic/core'; +import { useCallback } from 'react'; import { HookOverlayOptions } from './HookOverlayOptions'; import { useController } from './useController'; @@ -13,9 +14,7 @@ export function useIonToast(): UseIonToastResult { toastController ); - function present(message: string, duration?: number): void; - function present(options: ToastOptions & HookOverlayOptions): void; - function present(messageOrOptions: string | ToastOptions & HookOverlayOptions, duration?: number) { + const present = useCallback((messageOrOptions: string | ToastOptions & HookOverlayOptions, duration?: number) => { if (typeof messageOrOptions === 'string') { controller.present({ message: messageOrOptions, @@ -24,7 +23,7 @@ export function useIonToast(): UseIonToastResult { } else { controller.present(messageOrOptions); } - }; + }, [controller.present]); return [ present, diff --git a/packages/react/src/hooks/useOverlay.ts b/packages/react/src/hooks/useOverlay.ts index 6365bc4829..6292d6d689 100644 --- a/packages/react/src/hooks/useOverlay.ts +++ b/packages/react/src/hooks/useOverlay.ts @@ -1,5 +1,5 @@ import { OverlayEventDetail } from '@ionic/core'; -import React, { useEffect, useMemo, useRef, useState } from 'react'; +import React, { useCallback, useEffect, useMemo, useRef, useState } from 'react'; import ReactDOM from 'react-dom'; import { attachProps } from '../components/utils'; @@ -52,7 +52,7 @@ export function useOverlay< } }, [component, containerElRef.current, isOpen, componentProps]); - const present = async (options: OptionsType & HookOverlayOptions) => { + const present = useCallback(async (options: OptionsType & HookOverlayOptions) => { if (overlayRef.current) { return; } @@ -96,13 +96,13 @@ export function useOverlay< containerElRef.current = undefined; setIsOpen(false); } - }; + }, []); - const dismiss = async () => { + const dismiss = useCallback(async () => { overlayRef.current && await overlayRef.current.dismiss(); overlayRef.current = undefined; containerElRef.current = undefined; - }; + }, []); return { present,