Skip to content

Commit 6d4ccaf

Browse files
committed
fix(dialog): keep DialogManagerProvider mounted on first render and stabilize manager identity
1 parent 443b9a8 commit 6d4ccaf

File tree

2 files changed

+53
-25
lines changed

2 files changed

+53
-25
lines changed

src/components/Dialog/__tests__/DialogManagerContext.test.tsx

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,12 +1,18 @@
11
import React from 'react';
22
import { act, fireEvent, render, screen, waitFor } from '@testing-library/react';
3+
import { renderToStaticMarkup } from 'react-dom/server';
4+
import { vi } from 'vitest';
35
import {
46
DialogManagerProvider,
57
useDialogManager,
68
} from '../../../context/DialogManagerContext';
79

810
import { useDialogIsOpen, useOpenedDialogCount } from '../hooks';
911

12+
vi.mock('../../../components/Dialog/service/DialogPortal', () => ({
13+
DialogPortalDestination: () => null,
14+
}));
15+
1016
const TEST_IDS = {
1117
CLOSE_DIALOG: 'close-dialog',
1218
DIALOG_COUNT: 'dialog-count',
@@ -89,6 +95,16 @@ describe('DialogManagerContext', () => {
8995
expect(screen.getByTestId(TEST_IDS.DIALOG_COUNT).textContent).toBe('0');
9096
});
9197

98+
it('renders children during SSR when id is provided', () => {
99+
const html = renderToStaticMarkup(
100+
<DialogManagerProvider id={TEST_MANAGER_ID}>
101+
<div>server-rendered-child</div>
102+
</DialogManagerProvider>,
103+
);
104+
105+
expect(html).toContain('server-rendered-child');
106+
});
107+
92108
it('provides dialog manager to non-child components', () => {
93109
render(
94110
<DialogManagerProvider id={MANAGER_1_ID}>

src/context/DialogManagerContext.tsx

Lines changed: 37 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -15,27 +15,12 @@ type DialogManagerId = string;
1515

1616
type DialogManagersState = Record<DialogManagerId, DialogManager | undefined>;
1717
const dialogManagersRegistry: StateStore<DialogManagersState> = new StateStore({});
18+
const pendingDialogManagersById: Partial<Record<DialogManagerId, DialogManager>> = {};
19+
const dialogManagerMountCountsById: Partial<Record<DialogManagerId, number>> = {};
1820

1921
const getDialogManager = (id: string): DialogManager | undefined =>
2022
dialogManagersRegistry.getLatestValue()[id];
2123

22-
const getOrCreateDialogManager = ({
23-
closeOnClickOutside,
24-
id,
25-
}: {
26-
closeOnClickOutside?: boolean;
27-
id: string;
28-
}) => {
29-
let manager = getDialogManager(id);
30-
if (!manager) {
31-
manager = new DialogManager({ closeOnClickOutside, id });
32-
dialogManagersRegistry.partialNext({ [id]: manager });
33-
} else if (typeof closeOnClickOutside === 'boolean') {
34-
manager.closeOnClickOutside = closeOnClickOutside;
35-
}
36-
return manager;
37-
};
38-
3924
const removeDialogManager = (id: string) => {
4025
if (!getDialogManager(id)) return;
4126
dialogManagersRegistry.partialNext({ [id]: undefined });
@@ -67,23 +52,50 @@ export const DialogManagerProvider = ({
6752
closeOnClickOutside,
6853
id,
6954
}: DialogManagerProviderProps) => {
70-
const [dialogManager, setDialogManager] = useState<DialogManager | null>(() => {
71-
if (id) return getDialogManager(id) ?? null;
55+
const [dialogManager, setDialogManager] = useState<DialogManager>(() => {
56+
if (id) {
57+
const manager =
58+
getDialogManager(id) ??
59+
pendingDialogManagersById[id] ??
60+
new DialogManager({ closeOnClickOutside, id });
61+
62+
pendingDialogManagersById[id] = manager;
63+
return manager;
64+
}
7265
return new DialogManager({ closeOnClickOutside }); // will not be included in the registry
7366
});
7467

7568
useEffect(() => {
7669
if (!id) return;
77-
setDialogManager(getOrCreateDialogManager({ closeOnClickOutside, id }));
70+
const manager =
71+
getDialogManager(id) ??
72+
pendingDialogManagersById[id] ??
73+
new DialogManager({ closeOnClickOutside, id });
74+
75+
if (typeof closeOnClickOutside === 'boolean') {
76+
manager.closeOnClickOutside = closeOnClickOutside;
77+
}
78+
79+
if (!getDialogManager(id)) {
80+
dialogManagersRegistry.partialNext({ [id]: manager });
81+
}
82+
delete pendingDialogManagersById[id];
83+
84+
setDialogManager((prev) => (prev === manager ? prev : manager));
85+
dialogManagerMountCountsById[id] = (dialogManagerMountCountsById[id] ?? 0) + 1;
86+
7887
return () => {
88+
const nextMountCount = (dialogManagerMountCountsById[id] ?? 1) - 1;
89+
if (nextMountCount > 0) {
90+
dialogManagerMountCountsById[id] = nextMountCount;
91+
return;
92+
}
93+
94+
delete dialogManagerMountCountsById[id];
7995
removeDialogManager(id);
80-
setDialogManager(null);
8196
};
8297
}, [closeOnClickOutside, id]);
8398

84-
// temporarily do not render until a new dialog manager is created
85-
if (!dialogManager) return null;
86-
8799
return (
88100
<DialogManagerProviderContext.Provider value={{ dialogManager }}>
89101
{children}
@@ -172,7 +184,7 @@ export const useDialogManager = ({
172184

173185
if (!managerInPrevState || managerInNewState?.id !== managerInPrevState.id) {
174186
setDialogManagerContext((prevState) => {
175-
if (prevState?.dialogManager.id === managerInNewState?.id) return prevState;
187+
if (prevState?.dialogManager === managerInNewState) return prevState;
176188
// fixme: need to handle the possibility that the dialogManager is undefined
177189
return {
178190
dialogManager:

0 commit comments

Comments
 (0)