Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Merged
merged 1 commit into from
Oct 5, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions packages/react/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -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",
Expand Down
153 changes: 153 additions & 0 deletions packages/react/src/hooks/__tests__/hooks.spec.tsx
Original file line number Diff line number Diff line change
@@ -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);
});
});
94 changes: 38 additions & 56 deletions packages/react/src/hooks/useController.ts
Original file line number Diff line number Diff line change
@@ -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';

Expand All @@ -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,
Expand Down
34 changes: 18 additions & 16 deletions packages/react/src/hooks/useIonActionSheet.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
import { ActionSheetButton, ActionSheetOptions, actionSheetController } from '@ionic/core';
import { useCallback } from 'react';

import { HookOverlayOptions } from './HookOverlayOptions';
import { useController } from './useController';
Expand All @@ -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 = [
Expand Down
36 changes: 16 additions & 20 deletions packages/react/src/hooks/useIonAlert.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
import { AlertButton, AlertOptions, alertController } from '@ionic/core';
import { useCallback } from 'react';

import { HookOverlayOptions } from './HookOverlayOptions';
import { useController } from './useController';
Expand All @@ -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 = [
Expand Down
Loading