Skip to content

Commit 2c97712

Browse files
elylucasFWeinb
elylucas
and
FWeinb
authored
fix(react): overlay hooks memorised properly to prevent re-renders (#24010)
resolves #23741 Co-authored-by: Fabrice <fabrice@weinberg.me>
1 parent f112ad4 commit 2c97712

File tree

11 files changed

+273
-143
lines changed

11 files changed

+273
-143
lines changed

‎packages/react/package.json

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -52,6 +52,7 @@
5252
"@rollup/plugin-virtual": "^2.0.3",
5353
"@testing-library/jest-dom": "^5.11.6",
5454
"@testing-library/react": "^11.2.2",
55+
"@testing-library/react-hooks": "^7.0.1",
5556
"@types/jest": "^26.0.15",
5657
"@types/node": "^14.0.14",
5758
"@types/react": "16.14.0",

‎packages/react/src/hooks/__tests__/hooks.spec.tsx

Lines changed: 153 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,153 @@
1+
import { alertController, modalController } from '@ionic/core';
2+
3+
import React from 'react';
4+
5+
import { useController } from '../useController';
6+
import { useOverlay } from '../useOverlay';
7+
8+
import { useIonActionSheet } from '../useIonActionSheet';
9+
import type { UseIonActionSheetResult } from '../useIonActionSheet';
10+
import { useIonAlert } from '../useIonAlert';
11+
import type { UseIonAlertResult } from '../useIonAlert';
12+
import { useIonLoading } from '../useIonLoading';
13+
import type { UseIonLoadingResult } from '../useIonLoading';
14+
import { useIonModal } from '../useIonModal';
15+
import type { UseIonModalResult } from '../useIonModal';
16+
import { useIonPicker } from '../useIonPicker';
17+
import type { UseIonPickerResult } from '../useIonPicker';
18+
import { useIonPopover } from '../useIonPopover';
19+
import type { UseIonPopoverResult } from '../useIonPopover';
20+
import { useIonToast } from '../useIonToast';
21+
import type { UseIonToastResult } from '../useIonToast';
22+
23+
import { renderHook } from '@testing-library/react-hooks';
24+
25+
describe('useController', () => {
26+
it('should be memorised', () => {
27+
const { result, rerender } = renderHook(() =>
28+
useController('AlertController', alertController)
29+
);
30+
31+
rerender();
32+
33+
const [
34+
{ present: firstPresent, dismiss: firstDismiss },
35+
{ present: secondPresent, dismiss: secondDismiss },
36+
] = result.all as ReturnType<typeof useController>[];
37+
38+
expect(firstPresent).toBe(secondPresent);
39+
expect(firstDismiss).toBe(secondDismiss);
40+
});
41+
});
42+
43+
describe('useIonActionSheet', () => {
44+
it('should be memorised', () => {
45+
const { result, rerender } = renderHook(() => useIonActionSheet());
46+
47+
rerender();
48+
49+
const [[firstPresent, firstDismiss], [secondPresent, secondDismiss]] =
50+
result.all as UseIonActionSheetResult[];
51+
expect(firstPresent).toBe(secondPresent);
52+
expect(firstDismiss).toBe(secondDismiss);
53+
});
54+
});
55+
56+
describe('useIonAlert', () => {
57+
it('should be memorised', () => {
58+
const { result, rerender } = renderHook(() => useIonAlert());
59+
60+
rerender();
61+
62+
const [[firstPresent, firstDismiss], [secondPresent, secondDismiss]] =
63+
result.all as UseIonAlertResult[];
64+
expect(firstPresent).toBe(secondPresent);
65+
expect(firstDismiss).toBe(secondDismiss);
66+
});
67+
});
68+
69+
describe('useIonLoading', () => {
70+
it('should be memorised', () => {
71+
const { result, rerender } = renderHook(() => useIonLoading());
72+
73+
rerender();
74+
75+
const [[firstPresent, firstDismiss], [secondPresent, secondDismiss]] =
76+
result.all as UseIonLoadingResult[];
77+
expect(firstPresent).toBe(secondPresent);
78+
expect(firstDismiss).toBe(secondDismiss);
79+
});
80+
});
81+
82+
describe('useIonModal', () => {
83+
it('should be memorised', () => {
84+
const ModalComponent = () => <div />;
85+
const { result, rerender } = renderHook(() => useIonModal(ModalComponent, {}));
86+
87+
rerender();
88+
89+
const [[firstPresent, firstDismiss], [secondPresent, secondDismiss]] =
90+
result.all as UseIonModalResult[];
91+
expect(firstPresent).toBe(secondPresent);
92+
expect(firstDismiss).toBe(secondDismiss);
93+
});
94+
});
95+
96+
describe('useIonPicker', () => {
97+
it('should be memorised', () => {
98+
const { result, rerender } = renderHook(() => useIonPicker());
99+
100+
rerender();
101+
102+
const [[firstPresent, firstDismiss], [secondPresent, secondDismiss]] =
103+
result.all as UseIonPickerResult[];
104+
expect(firstPresent).toBe(secondPresent);
105+
expect(firstDismiss).toBe(secondDismiss);
106+
});
107+
});
108+
109+
describe('useIonPopover', () => {
110+
it('should be memorised', () => {
111+
const PopoverComponent = () => <div />;
112+
const { result, rerender } = renderHook(() => useIonPopover(PopoverComponent, {}));
113+
114+
rerender();
115+
116+
const [[firstPresent, firstDismiss], [secondPresent, secondDismiss]] =
117+
result.all as UseIonPopoverResult[];
118+
expect(firstPresent).toBe(secondPresent);
119+
expect(firstDismiss).toBe(secondDismiss);
120+
});
121+
});
122+
123+
describe('useIonToast', () => {
124+
it('should be memorised', () => {
125+
const { result, rerender } = renderHook(() => useIonToast());
126+
127+
rerender();
128+
129+
const [[firstPresent, firstDismiss], [secondPresent, secondDismiss]] =
130+
result.all as UseIonToastResult[];
131+
expect(firstPresent).toBe(secondPresent);
132+
expect(firstDismiss).toBe(secondDismiss);
133+
});
134+
});
135+
136+
describe('useOverlay', () => {
137+
it('should be memorised', () => {
138+
const OverlayComponent = () => <div />;
139+
const { result, rerender } = renderHook(() =>
140+
useOverlay('IonModal', modalController, OverlayComponent, {})
141+
);
142+
143+
rerender();
144+
145+
const [
146+
{ present: firstPresent, dismiss: firstDismiss },
147+
{ present: secondPresent, dismiss: secondDismiss },
148+
] = result.all as ReturnType<typeof useOverlay>[];
149+
150+
expect(firstPresent).toBe(secondPresent);
151+
expect(firstDismiss).toBe(secondDismiss);
152+
});
153+
});

‎packages/react/src/hooks/useController.ts

Lines changed: 38 additions & 56 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
import { OverlayEventDetail } from '@ionic/core';
2-
import { useMemo, useRef } from 'react';
2+
import { useCallback, useMemo, useRef } from 'react';
33

44
import { attachProps } from '../components/utils';
55

@@ -10,71 +10,53 @@ interface OverlayBase extends HTMLElement {
1010
dismiss: (data?: any, role?: string | undefined) => Promise<boolean>;
1111
}
1212

13-
export function useController<
14-
OptionsType,
15-
OverlayType extends OverlayBase
16-
>(
13+
export function useController<OptionsType, OverlayType extends OverlayBase>(
1714
displayName: string,
1815
controller: { create: (options: OptionsType) => Promise<OverlayType> }
1916
) {
2017
const overlayRef = useRef<OverlayType>();
21-
const didDismissEventName = useMemo(
22-
() => `on${displayName}DidDismiss`,
23-
[displayName]
24-
);
25-
const didPresentEventName = useMemo(
26-
() => `on${displayName}DidPresent`,
27-
[displayName]
28-
);
29-
const willDismissEventName = useMemo(
30-
() => `on${displayName}WillDismiss`,
31-
[displayName]
32-
);
33-
const willPresentEventName = useMemo(
34-
() => `on${displayName}WillPresent`,
35-
[displayName]
36-
);
37-
38-
const present = async (options: OptionsType & HookOverlayOptions) => {
39-
if (overlayRef.current) {
40-
return;
41-
}
42-
const {
43-
onDidDismiss,
44-
onWillDismiss,
45-
onDidPresent,
46-
onWillPresent,
47-
...rest
48-
} = options;
18+
const didDismissEventName = useMemo(() => `on${displayName}DidDismiss`, [displayName]);
19+
const didPresentEventName = useMemo(() => `on${displayName}DidPresent`, [displayName]);
20+
const willDismissEventName = useMemo(() => `on${displayName}WillDismiss`, [displayName]);
21+
const willPresentEventName = useMemo(() => `on${displayName}WillPresent`, [displayName]);
4922

50-
const handleDismiss = (event: CustomEvent<OverlayEventDetail<any>>) => {
51-
if (onDidDismiss) {
52-
onDidDismiss(event);
23+
const present = useCallback(
24+
async (options: OptionsType & HookOverlayOptions) => {
25+
if (overlayRef.current) {
26+
return;
5327
}
54-
overlayRef.current = undefined;
55-
}
28+
const { onDidDismiss, onWillDismiss, onDidPresent, onWillPresent, ...rest } = options;
5629

57-
overlayRef.current = await controller.create({
58-
...(rest as any),
59-
});
30+
const handleDismiss = (event: CustomEvent<OverlayEventDetail<any>>) => {
31+
if (onDidDismiss) {
32+
onDidDismiss(event);
33+
}
34+
overlayRef.current = undefined;
35+
};
6036

61-
attachProps(overlayRef.current, {
62-
[didDismissEventName]: handleDismiss,
63-
[didPresentEventName]: (e: CustomEvent) =>
64-
onDidPresent && onDidPresent(e),
65-
[willDismissEventName]: (e: CustomEvent) =>
66-
onWillDismiss && onWillDismiss(e),
67-
[willPresentEventName]: (e: CustomEvent) =>
68-
onWillPresent && onWillPresent(e),
69-
});
37+
overlayRef.current = await controller.create({
38+
...(rest as any),
39+
});
7040

71-
overlayRef.current.present();
72-
};
41+
attachProps(overlayRef.current, {
42+
[didDismissEventName]: handleDismiss,
43+
[didPresentEventName]: (e: CustomEvent) => onDidPresent && onDidPresent(e),
44+
[willDismissEventName]: (e: CustomEvent) => onWillDismiss && onWillDismiss(e),
45+
[willPresentEventName]: (e: CustomEvent) => onWillPresent && onWillPresent(e),
46+
});
7347

74-
const dismiss = async () => {
75-
overlayRef.current && await overlayRef.current.dismiss();
76-
overlayRef.current = undefined;
77-
};
48+
overlayRef.current.present();
49+
},
50+
[controller]
51+
);
52+
53+
const dismiss = useCallback(
54+
() => async () => {
55+
overlayRef.current && (await overlayRef.current.dismiss());
56+
overlayRef.current = undefined;
57+
},
58+
[]
59+
);
7860

7961
return {
8062
present,

‎packages/react/src/hooks/useIonActionSheet.ts

Lines changed: 18 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,5 @@
11
import { ActionSheetButton, ActionSheetOptions, actionSheetController } from '@ionic/core';
2+
import { useCallback } from 'react';
23

34
import { HookOverlayOptions } from './HookOverlayOptions';
45
import { useController } from './useController';
@@ -13,23 +14,24 @@ export function useIonActionSheet(): UseIonActionSheetResult {
1314
actionSheetController
1415
);
1516

16-
function present(buttons: ActionSheetButton[], header?: string): void;
17-
function present(options: ActionSheetOptions & HookOverlayOptions): void;
18-
function present(buttonsOrOptions: ActionSheetButton[] | ActionSheetOptions & HookOverlayOptions, header?: string) {
19-
if (Array.isArray(buttonsOrOptions)) {
20-
controller.present({
21-
buttons: buttonsOrOptions,
22-
header
23-
});
24-
} else {
25-
controller.present(buttonsOrOptions);
26-
}
27-
}
17+
const present = useCallback(
18+
(
19+
buttonsOrOptions: ActionSheetButton[] | (ActionSheetOptions & HookOverlayOptions),
20+
header?: string
21+
) => {
22+
if (Array.isArray(buttonsOrOptions)) {
23+
controller.present({
24+
buttons: buttonsOrOptions,
25+
header,
26+
});
27+
} else {
28+
controller.present(buttonsOrOptions);
29+
}
30+
},
31+
[controller.present]
32+
);
2833

29-
return [
30-
present,
31-
controller.dismiss
32-
];
34+
return [present, controller.dismiss];
3335
}
3436

3537
export type UseIonActionSheetResult = [

‎packages/react/src/hooks/useIonAlert.ts

Lines changed: 16 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,5 @@
11
import { AlertButton, AlertOptions, alertController } from '@ionic/core';
2+
import { useCallback } from 'react';
23

34
import { HookOverlayOptions } from './HookOverlayOptions';
45
import { useController } from './useController';
@@ -8,28 +9,23 @@ import { useController } from './useController';
89
* @returns Returns the present and dismiss methods in an array
910
*/
1011
export function useIonAlert(): UseIonAlertResult {
11-
const controller = useController<AlertOptions, HTMLIonAlertElement>(
12-
'IonAlert',
13-
alertController
14-
);
12+
const controller = useController<AlertOptions, HTMLIonAlertElement>('IonAlert', alertController);
1513

16-
function present(message: string, buttons?: AlertButton[]): void;
17-
function present(options: AlertOptions & HookOverlayOptions): void;
18-
function present(messageOrOptions: string | AlertOptions & HookOverlayOptions, buttons?: AlertButton[]) {
19-
if (typeof messageOrOptions === 'string') {
20-
controller.present({
21-
message: messageOrOptions,
22-
buttons: buttons ?? [{ text: 'Ok' }]
23-
});
24-
} else {
25-
controller.present(messageOrOptions);
26-
}
27-
};
14+
const present = useCallback(
15+
(messageOrOptions: string | (AlertOptions & HookOverlayOptions), buttons?: AlertButton[]) => {
16+
if (typeof messageOrOptions === 'string') {
17+
controller.present({
18+
message: messageOrOptions,
19+
buttons: buttons ?? [{ text: 'Ok' }],
20+
});
21+
} else {
22+
controller.present(messageOrOptions);
23+
}
24+
},
25+
[controller.present]
26+
);
2827

29-
return [
30-
present,
31-
controller.dismiss
32-
];
28+
return [present, controller.dismiss];
3329
}
3430

3531
export type UseIonAlertResult = [

0 commit comments

Comments
 (0)