Skip to content

Commit db7b0a7

Browse files
committed
fix: make DialogManagerProvider a pure component
1 parent bc1805a commit db7b0a7

File tree

2 files changed

+38
-25
lines changed

2 files changed

+38
-25
lines changed

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

Lines changed: 11 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -64,14 +64,17 @@ const DialogTestComponent = ({ dialogId, managerId }) => {
6464

6565
describe('DialogManagerContext', () => {
6666
describe('DialogManagerProvider', () => {
67-
it('does not create a new dialog manager when no id is provided', () => {
67+
it('creates a new dialog manager when no id is provided with randomly generated id', () => {
6868
render(
6969
<DialogManagerProvider>
7070
<TestComponent />
7171
</DialogManagerProvider>,
7272
);
7373

7474
expect(screen.getByTestId(TEST_IDS.DIALOG_COUNT).textContent).toBe('0');
75+
expect(screen.getByTestId(TEST_IDS.MANAGER_ID_DISPLAY).textContent).toEqual(
76+
expect.any(String),
77+
);
7578
});
7679

7780
it('creates a new dialog manager and adds it to the manager pool when id is provided', () => {
@@ -81,8 +84,9 @@ describe('DialogManagerContext', () => {
8184
</DialogManagerProvider>,
8285
);
8386

84-
const managerId = screen.getByTestId(TEST_IDS.MANAGER_ID_DISPLAY).textContent;
85-
expect(managerId).toBe(TEST_MANAGER_ID);
87+
expect(screen.getByTestId(TEST_IDS.MANAGER_ID_DISPLAY).textContent).toBe(
88+
TEST_MANAGER_ID,
89+
);
8690
expect(screen.getByTestId(TEST_IDS.DIALOG_COUNT).textContent).toBe('0');
8791
});
8892

@@ -93,8 +97,9 @@ describe('DialogManagerContext', () => {
9397
<TestComponent dialogManagerId={MANAGER_2_ID} />
9498
</DialogManagerProvider>,
9599
);
96-
const managerId = screen.getByTestId(TEST_IDS.MANAGER_ID_DISPLAY).textContent;
97-
expect(managerId).toBe(MANAGER_2_ID);
100+
expect(screen.getByTestId(TEST_IDS.MANAGER_ID_DISPLAY).textContent).toBe(
101+
MANAGER_2_ID,
102+
);
98103
expect(screen.getByTestId(TEST_IDS.DIALOG_COUNT).textContent).toBe('0');
99104
});
100105

@@ -120,7 +125,7 @@ describe('DialogManagerContext', () => {
120125
expect(screen.getByTestId(TEST_IDS.DIALOG_COUNT)).toHaveTextContent('0');
121126
});
122127

123-
it('retrieves existing dialog manager and does not create a new dialog manager', () => {
128+
it('retrieves the existing dialog manager and does not create a new dialog manager', () => {
124129
const dialogId = 'shared-dialog';
125130
render(
126131
<DialogManagerProvider id={SHARED_MANAGER_ID}>

src/context/DialogManagerContext.tsx

Lines changed: 27 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -14,19 +14,23 @@ import type { PropsWithChildrenOnly } from '../types/types';
1414
type DialogManagerId = string;
1515

1616
type DialogManagersState = Record<DialogManagerId, DialogManager | undefined>;
17-
const dialogManagersStore: StateStore<DialogManagersState> = new StateStore({});
17+
const dialogManagersRegistry: StateStore<DialogManagersState> = new StateStore({});
1818

1919
const getDialogManager = (id: string): DialogManager | undefined =>
20-
dialogManagersStore.getLatestValue()[id];
20+
dialogManagersRegistry.getLatestValue()[id];
2121

22-
const addDialogManager = (dialogManager: DialogManager) => {
23-
if (getDialogManager(dialogManager.id)) return;
24-
dialogManagersStore.partialNext({ [dialogManager.id]: dialogManager });
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;
2529
};
2630

2731
const removeDialogManager = (id: string) => {
2832
if (!getDialogManager(id)) return;
29-
dialogManagersStore.partialNext({ [id]: undefined });
33+
dialogManagersRegistry.partialNext({ [id]: undefined });
3034
};
3135

3236
type DialogManagerProviderContextValue = {
@@ -47,18 +51,22 @@ export const DialogManagerProvider = ({
4751
children,
4852
id,
4953
}: PropsWithChildren<{ id?: string }>) => {
50-
const dialogManager = useMemo<DialogManager>(
51-
() => (id && getDialogManager(id)) || new DialogManager({ id }),
52-
[id],
53-
);
54+
const [dialogManager, setDialogManager] = useState<DialogManager | null>(() => {
55+
if (id) return getDialogManager(id) ?? null;
56+
return new DialogManager(); // will not be included in the registry
57+
});
5458

55-
addDialogManager(dialogManager);
56-
useEffect(
57-
() => () => {
58-
removeDialogManager(dialogManager.id);
59-
},
60-
[dialogManager],
61-
);
59+
useEffect(() => {
60+
if (!id) return;
61+
setDialogManager(getOrCreateDialogManager(id));
62+
return () => {
63+
removeDialogManager(id);
64+
setDialogManager(null);
65+
};
66+
}, [id]);
67+
68+
// temporarily do not render until a new dialog manager is created
69+
if (!dialogManager) return null;
6270

6371
return (
6472
<DialogManagerProviderContext.Provider value={{ dialogManager }}>
@@ -126,7 +134,7 @@ export const useDialogManager = ({
126134
const { managerInNewState } = getManagerFromStore({
127135
dialogId,
128136
dialogManagerId,
129-
newState: dialogManagersStore.getLatestValue(),
137+
newState: dialogManagersRegistry.getLatestValue(),
130138
previousState: undefined,
131139
});
132140
return managerInNewState
@@ -136,7 +144,7 @@ export const useDialogManager = ({
136144

137145
useEffect(() => {
138146
if (!dialogId && !dialogManagerId) return;
139-
const unsubscribe = dialogManagersStore.subscribeWithSelector(
147+
const unsubscribe = dialogManagersRegistry.subscribeWithSelector(
140148
(state) => state,
141149
(newState, previousState) => {
142150
const { managerInNewState, managerInPrevState } = getManagerFromStore({

0 commit comments

Comments
 (0)