Skip to content

Commit bf8e451

Browse files
committed
fix: Proper async handling for portals
1 parent 650399a commit bf8e451

File tree

2 files changed

+36
-7
lines changed

2 files changed

+36
-7
lines changed

src/internal/portal/__tests__/portal.test.tsx

Lines changed: 25 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,7 @@
22
// SPDX-License-Identifier: Apache-2.0
33

44
import React, { useState } from 'react';
5-
import { act, fireEvent, render, screen } from '@testing-library/react';
5+
import { act, fireEvent, render, screen, waitFor } from '@testing-library/react';
66

77
import { warnOnce } from '../../logging';
88
import Portal, { PortalProps } from '../index';
@@ -108,6 +108,30 @@ describe('Portal', () => {
108108
expect(document.body.contains(container)).toBe(false);
109109
});
110110

111+
test('should support aborting async container setup', async () => {
112+
const container = document.createElement('div');
113+
const onAbort = jest.fn();
114+
const onContinue = jest.fn();
115+
const getContainer: PortalProps['getContainer'] = async ({ abortSignal }) => {
116+
abortSignal.addEventListener('abort', onAbort);
117+
await Promise.resolve();
118+
onContinue(abortSignal.aborted);
119+
return container;
120+
};
121+
const removeContainer = jest.fn();
122+
const { unmount } = renderPortal({
123+
children: <p data-testid="portal-content">Hello!</p>,
124+
getContainer,
125+
removeContainer,
126+
});
127+
unmount();
128+
await waitFor(() => {
129+
expect(onContinue).not.toHaveBeenCalled();
130+
expect(onAbort).toHaveBeenCalled();
131+
expect(removeContainer).toHaveBeenCalledWith(null);
132+
});
133+
});
134+
111135
test('allows conditional change of getContainer/removeContainer', async () => {
112136
function MovablePortal({ getContainer, removeContainer }: Pick<PortalProps, 'getContainer' | 'removeContainer'>) {
113137
const [visible, setVisible] = useState(false);

src/internal/portal/index.tsx

Lines changed: 11 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -8,8 +8,8 @@ import { warnOnce } from '../logging';
88

99
export interface PortalProps {
1010
container?: null | Element;
11-
getContainer?: () => Promise<HTMLElement>;
12-
removeContainer?: (container: HTMLElement) => void;
11+
getContainer?: (options: { abortSignal: AbortSignal }) => Promise<HTMLElement>;
12+
removeContainer?: (container: HTMLElement | null) => void;
1313
children: React.ReactNode;
1414
}
1515

@@ -23,13 +23,17 @@ function manageDefaultContainer(setState: React.Dispatch<React.SetStateAction<El
2323
}
2424

2525
function manageAsyncContainer(
26-
getContainer: () => Promise<HTMLElement>,
27-
removeContainer: (container: HTMLElement) => void,
26+
getContainer: (options: { abortSignal: AbortSignal }) => Promise<HTMLElement>,
27+
removeContainer: (container: HTMLElement | null) => void,
2828
setState: React.Dispatch<React.SetStateAction<Element | null>>
2929
) {
30-
let newContainer: HTMLElement;
31-
getContainer().then(
30+
let newContainer: HTMLElement | null = null;
31+
const abortController = new AbortController();
32+
getContainer({ abortSignal: abortController.signal }).then(
3233
container => {
34+
if (abortController.signal.aborted) {
35+
return;
36+
}
3337
newContainer = container;
3438
setState(container);
3539
},
@@ -38,6 +42,7 @@ function manageAsyncContainer(
3842
}
3943
);
4044
return () => {
45+
abortController.abort();
4146
removeContainer(newContainer);
4247
};
4348
}

0 commit comments

Comments
 (0)