Skip to content

Commit 79ff330

Browse files
committed
fix(dialog): keep DialogManagerProvider mounted on first render and stabilize manager identity
1 parent f44c07b commit 79ff330

4 files changed

Lines changed: 52 additions & 19 deletions

File tree

src/components/ChannelList/hooks/useSelectedChannelState.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -45,5 +45,5 @@ export function useSelectedChannelState<O>({
4545
return selector(channel);
4646
}, [channel, selector]);
4747

48-
return useSyncExternalStore(subscribe, getSnapshot);
48+
return useSyncExternalStore(subscribe, getSnapshot, getSnapshot);
4949
}

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

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,6 @@
11
import React from 'react';
22
import { act, fireEvent, render, screen, waitFor } from '@testing-library/react';
3+
import { renderToStaticMarkup } from 'react-dom/server';
34
import {
45
DialogManagerProvider,
56
useDialogManager,
@@ -8,6 +9,10 @@ import {
89
import '@testing-library/jest-dom';
910
import { useDialogIsOpen, useOpenedDialogCount } from '../hooks';
1011

12+
jest.mock('../../../components/Dialog/DialogPortal', () => ({
13+
DialogPortalDestination: () => null,
14+
}));
15+
1116
const TEST_IDS = {
1217
CLOSE_DIALOG: 'close-dialog',
1318
DIALOG_COUNT: 'dialog-count',
@@ -90,6 +95,16 @@ describe('DialogManagerContext', () => {
9095
expect(screen.getByTestId(TEST_IDS.DIALOG_COUNT).textContent).toBe('0');
9196
});
9297

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+
93108
it('provides dialog manager to non-child components', () => {
94109
render(
95110
<DialogManagerProvider id={MANAGER_1_ID}>

src/context/DialogManagerContext.tsx

Lines changed: 31 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -15,19 +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 = (id: string) => {
23-
let manager = getDialogManager(id);
24-
if (!manager) {
25-
manager = new DialogManager({ id });
26-
dialogManagersRegistry.partialNext({ [id]: manager });
27-
}
28-
return manager;
29-
};
30-
3124
const removeDialogManager = (id: string) => {
3225
if (!getDialogManager(id)) return;
3326
dialogManagersRegistry.partialNext({ [id]: undefined });
@@ -51,23 +44,44 @@ export const DialogManagerProvider = ({
5144
children,
5245
id,
5346
}: PropsWithChildren<{ id?: string }>) => {
54-
const [dialogManager, setDialogManager] = useState<DialogManager | null>(() => {
55-
if (id) return getDialogManager(id) ?? null;
47+
const [dialogManager, setDialogManager] = useState<DialogManager>(() => {
48+
if (id) {
49+
const manager =
50+
getDialogManager(id) ??
51+
pendingDialogManagersById[id] ??
52+
new DialogManager({ id });
53+
pendingDialogManagersById[id] = manager;
54+
return manager;
55+
}
56+
5657
return new DialogManager(); // will not be included in the registry
5758
});
5859

5960
useEffect(() => {
6061
if (!id) return;
61-
setDialogManager(getOrCreateDialogManager(id));
62+
const manager =
63+
getDialogManager(id) ?? pendingDialogManagersById[id] ?? new DialogManager({ id });
64+
65+
if (!getDialogManager(id)) {
66+
dialogManagersRegistry.partialNext({ [id]: manager });
67+
}
68+
delete pendingDialogManagersById[id];
69+
70+
setDialogManager((prev) => (prev === manager ? prev : manager));
71+
dialogManagerMountCountsById[id] = (dialogManagerMountCountsById[id] ?? 0) + 1;
72+
6273
return () => {
74+
const nextMountCount = (dialogManagerMountCountsById[id] ?? 1) - 1;
75+
if (nextMountCount > 0) {
76+
dialogManagerMountCountsById[id] = nextMountCount;
77+
return;
78+
}
79+
80+
delete dialogManagerMountCountsById[id];
6381
removeDialogManager(id);
64-
setDialogManager(null);
6582
};
6683
}, [id]);
6784

68-
// temporarily do not render until a new dialog manager is created
69-
if (!dialogManager) return null;
70-
7185
return (
7286
<DialogManagerProviderContext.Provider value={{ dialogManager }}>
7387
{children}
@@ -156,7 +170,7 @@ export const useDialogManager = ({
156170

157171
if (!managerInPrevState || managerInNewState?.id !== managerInPrevState.id) {
158172
setDialogManagerContext((prevState) => {
159-
if (prevState?.dialogManager.id === managerInNewState?.id) return prevState;
173+
if (prevState?.dialogManager === managerInNewState) return prevState;
160174
// fixme: need to handle the possibility that the dialogManager is undefined
161175
return {
162176
dialogManager:

src/store/hooks/useStateStore.ts

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -59,7 +59,11 @@ export function useStateStore<
5959
};
6060
}, [store, selector]);
6161

62-
const state = useSyncExternalStore(wrappedSubscription, wrappedSnapshot);
62+
const state = useSyncExternalStore(
63+
wrappedSubscription,
64+
wrappedSnapshot,
65+
wrappedSnapshot,
66+
);
6367

6468
return state;
6569
}

0 commit comments

Comments
 (0)