Stabilise context value consumed by useDialog (#175)

a16n-dev · web-flow · commit 1eaa63763045 · 2025-08-26T10:42:26.000+10:00
Co-authored-by: Alex Nicholson &lt;alex.n@clove.kitchen&gt;
diff --git a/packages/react-dialog-async/package.json b/packages/react-dialog-async/package.json
@@ -2,7 +2,7 @@
   "name": "react-dialog-async",
   "description": "A promise-based way to show dialogs in React",
   "type": "module",
-  "version": "2.5.0",
+  "version": "2.5.1",
   "sideEffects": false,
   "main": "index.js",
   "module": "index.esm.js",
diff --git a/packages/react-dialog-async/src/DialogProvider/DialogProvider.tsx b/packages/react-dialog-async/src/DialogProvider/DialogProvider.tsx
@@ -1,4 +1,4 @@
-import React, { useEffect, useRef } from 'react';
+import React, { useContext, useEffect, useMemo, useRef } from 'react';
 import { PropsWithChildren, useCallback, useState } from 'react';
 import DialogContext, { dialogContextState } from '../DialogContext';
 import { DialogComponent } from '../types';
@@ -16,6 +16,7 @@ const DialogProvider = ({
   defaultUnmountDelayInMs,
   children,
 }: DialogProviderProps) => {
+  // This ref tracks timers for unmount dialogs after they're closed
   const unmountDelayTimeoutRefs = useRef<{ [key: string]: any }>({});
 
   const [dialogState, setDialogState] = useState<dialogsStateData>({});
@@ -24,39 +25,36 @@ const DialogProvider = ({
   /**
    * Force closes the dialog with the given id.
    */
-  const hide = useCallback(
-    (id: string) => {
-      setDialogState((state) => {
-        if (!state[id]) return state;
+  const hide = useCallback((id: string) => {
+    setDialogState((state) => {
+      if (!state[id]) return state;
 
-        if (!state[id].open) return state; // don't do anything if the dialog is already closed
+      if (!state[id].open) return state; // don't do anything if the dialog is already closed
 
-        state[id].resolve?.(undefined);
+      state[id].resolve?.(undefined);
 
-        if (state[id].unmountDelay) {
-          if (unmountDelayTimeoutRefs.current[id] !== undefined) {
-            clearTimeout(unmountDelayTimeoutRefs.current[id]);
-          }
+      if (state[id].unmountDelay) {
+        if (unmountDelayTimeoutRefs.current[id] !== undefined) {
+          clearTimeout(unmountDelayTimeoutRefs.current[id]);
+        }
 
-          // start the delay
-          unmountDelayTimeoutRefs.current[id] = setTimeout(() => {
-            setDialogState((state) => removeKey(state, id));
-          }, state[id].unmountDelay);
+        // start the delay
+        unmountDelayTimeoutRefs.current[id] = setTimeout(() => {
+          setDialogState((state) => removeKey(state, id));
+        }, state[id].unmountDelay);
 
-          return {
-            ...state,
-            [id]: {
-              open: false,
-              dialog: state[id].dialog,
-              data: state[id].data,
-            },
-          };
-        }
-        return removeKey(state, id);
-      });
-    },
-    [setDialogState],
-  );
+        return {
+          ...state,
+          [id]: {
+            open: false,
+            dialog: state[id].dialog,
+            data: state[id].data,
+          },
+        };
+      }
+      return removeKey(state, id);
+    });
+  }, []);
 
   const show = useCallback(
     (
@@ -66,9 +64,6 @@ const DialogProvider = ({
       data: unknown,
       unmountDelay?: number,
     ): Promise<unknown> => {
-      // if already open, do nothing
-      if (dialogState[id]?.open) return Promise.resolve();
-
       return new Promise((resolve) => {
         if (unmountDelayTimeoutRefs.current[id] !== undefined) {
           clearTimeout(unmountDelayTimeoutRefs.current[id]);
@@ -79,71 +74,70 @@ const DialogProvider = ({
           hide(id);
         };
 
-        setDialogState((state) => ({
-          ...state,
-          [id]: {
-            dialog: dialog,
-            open: true,
-            hash,
-            data,
-            resolve: resolveFn,
-            unmountDelay: unmountDelay ?? defaultUnmountDelayInMs,
-          },
-        }));
-      });
-    },
-    [setDialogState, hide, defaultUnmountDelayInMs],
-  );
+        setDialogState((state) => {
+          if (state[id]?.open) {
+            resolve(undefined);
+            return state;
+          }
 
-  /**
-   * Updates the data of the given dialog
-   */
-  const updateData = useCallback(
-    (id: string, data: unknown): void => {
-      setDialogState((state) => {
-        if (state[id]) {
           return {
             ...state,
-            [id]: { ...state[id], data },
+            [id]: {
+              dialog: dialog,
+              open: true,
+              hash,
+              data,
+              resolve: resolveFn,
+              unmountDelay: unmountDelay ?? defaultUnmountDelayInMs,
+            },
           };
-        }
-        return state;
+        });
       });
     },
-    [setDialogState],
+    [hide, defaultUnmountDelayInMs],
   );
 
-  const dialogComponents = useRenderDialogs(dialogState);
-
-  if (
-    process.env.NODE_ENV !== 'production' &&
-    !usingOutlet &&
-    dialogComponents.length > 0
-  ) {
-    console.warn(
-      'Rendering a dialog without a <DialogOutlet/>. Please include a <DialogOutlet/> as a child of <DialogProvider/> to ensure dialogs are rendered within the correct contexts - this will be required in the next major version of react-dialog-async. See https://react-dialog-async.a16n.dev/API/dialog-outlet for more details. This warning is only present in development',
-    );
-  }
+  /**
+   * Updates the data of the given dialog
+   */
+  const updateData = useCallback((id: string, data: unknown): void => {
+    setDialogState((state) => {
+      if (state[id]) {
+        return {
+          ...state,
+          [id]: { ...state[id], data },
+        };
+      }
+      return state;
+    });
+  }, []);
 
   useEffect(() => {
     return () => {
       Object.values(unmountDelayTimeoutRefs.current).forEach(clearTimeout);
     };
   }, []);
 
-  const ctx: dialogContextState = {
-    show,
-    hide,
-    updateData,
-  };
+  /**
+   * To prevent unnecessary re-renders, be careful to ensure that this state has
+   * no transitive dependencies to `dialogState`
+   */
+  const ctx: dialogContextState = useMemo(
+    () => ({
+      show,
+      hide,
+      updateData,
+    }),
+    [show, hide, updateData],
+  );
 
   return (
     <DialogStateContext.Provider
       value={{ dialogs: dialogState, setIsUsingOutlet: setUsingOutlet }}
     >
       <DialogContext.Provider value={ctx}>
         {children}
-        {!usingOutlet && dialogComponents}
+        {!usingOutlet && <InternalDialogOutlet />}
       </DialogContext.Provider>
     </DialogStateContext.Provider>
   );
@@ -158,3 +152,23 @@ function removeKey<
   const { [key]: _, ...rest } = data;
   return rest;
 }
+
+const InternalDialogOutlet = () => {
+  const dialogState = useContext(DialogStateContext);
+
+  if (!dialogState) {
+    throw new Error(
+      'Dialog context not found. You likely forgot to wrap your app in a <DialogProvider/> (https://react-dialog-async.a16n.dev/installation)',
+    );
+  }
+
+  const dialogComponents = useRenderDialogs(dialogState.dialogs);
+
+  if (process.env.NODE_ENV !== 'production' && dialogComponents.length > 0) {
+    console.warn(
+      'Rendering a dialog without a <DialogOutlet/>. Please include a <DialogOutlet/> as a child of <DialogProvider/> to ensure dialogs are rendered within the correct contexts - this will be required in the next major version of react-dialog-async. See https://react-dialog-async.a16n.dev/API/dialog-outlet for more details. This warning is only present in development',
+    );
+  }
+
+  return <>{dialogComponents}</>;
+};
diff --git a/packages/react-dialog-async/src/useDialog.test.tsx b/packages/react-dialog-async/src/useDialog.test.tsx
@@ -1,11 +1,19 @@
-import React, { act } from 'react';
+import React, { act, PropsWithChildren } from 'react';
 import { expect, test } from 'vitest';
 import { render, renderHook, screen } from '@testing-library/react';
 import DialogProvider from './DialogProvider/DialogProvider';
 
 import useDialog from './useDialog';
 import { AsyncDialogProps } from './types';
 import { useEffect } from 'react';
+import DialogOutlet from './DialogOutlet';
+
+const TestWrapper = ({ children }: PropsWithChildren) => (
+  <DialogProvider>
+    {children}
+    <DialogOutlet />
+  </DialogProvider>
+);
 
 const TestDialog = () => <div>Hello World!</div>;
 const TestDialogWithData = ({ data }: AsyncDialogProps<string>) => (
@@ -19,9 +27,9 @@ test('can be called without error', () => {
     return null;
   };
   const result = render(
-    <DialogProvider>
+    <TestWrapper>
       <TestComponent />
-    </DialogProvider>,
+    </TestWrapper>,
   );
 
   expect(result.asFragment()).toMatchSnapshot();
@@ -40,9 +48,9 @@ test('data is used when default data is not provided', () => {
   };
 
   render(
-    <DialogProvider>
+    <TestWrapper>
       <TestComponent />
-    </DialogProvider>,
+    </TestWrapper>,
   );
 
   expect(screen.getByText(message)).toBeDefined();
@@ -63,9 +71,9 @@ test('default data is used when data is not provided', () => {
   };
 
   render(
-    <DialogProvider>
+    <TestWrapper>
       <TestComponent />
-    </DialogProvider>,
+    </TestWrapper>,
   );
 
   expect(screen.getByText(message)).toBeDefined();
@@ -86,9 +94,9 @@ test('default data is overridden when data is provided', () => {
   };
 
   render(
-    <DialogProvider>
+    <TestWrapper>
       <TestComponent />
-    </DialogProvider>,
+    </TestWrapper>,
   );
 
   expect(screen.queryByText(message)).toBeNull();
@@ -101,7 +109,7 @@ test('unmount delay does not delay the promise being resolved', async () => {
         unmountDelayInMs: 100,
       }),
     {
-      wrapper: DialogProvider,
+      wrapper: TestWrapper,
     },
   );
 
@@ -120,3 +128,29 @@ test('unmount delay does not delay the promise being resolved', async () => {
 
   expect(value).toBe(true);
 });
+
+test('consumer does not rerender when dialog is opened', async () => {
+  // Track the number of renders
+  let renderCount = 0;
+
+  const TestComponent = () => {
+    renderCount++;
+    const testDialog = useDialog(TestDialog);
+
+    return <button onClick={() => testDialog.open()}>Open dialog</button>;
+  };
+
+  render(
+    <TestWrapper>
+      <TestComponent />
+    </TestWrapper>,
+  );
+
+  // Press the button to open the dialog
+  await act(async () => {
+    screen.getByText('Open dialog').click();
+  });
+
+  // Expect only the initial render
+  expect(renderCount).toBe(1);
+});
diff --git a/packages/react-dialog-async/src/useRenderDialogs.tsx b/packages/react-dialog-async/src/useRenderDialogs.tsx
@@ -2,6 +2,9 @@ import { dialogsStateData } from './DialogStateContext';
 import React, { useMemo } from 'react';
 import IndividualDialogContext from './IndividualDialogContext';
 
+/**
+ * Given the current dialog state, outputs an array of `Element`s to be rendered.
+ */
 const useRenderDialogs = (state: dialogsStateData) => {
   return useMemo(() => {
     const entries = Object.entries(state);
@@ -28,11 +31,11 @@ const useRenderDialogs = (state: dialogsStateData) => {
           isInsideDialogContext: true,
         };
 
+        // This key will be unique for each open of the dialog, ensuring that the dialog always resets its internal state.
+        const key = id + hash;
+
         return (
-          <IndividualDialogContext.Provider
-            key={id + hash}
-            value={contextValue}
-          >
+          <IndividualDialogContext.Provider key={key} value={contextValue}>
             <Component {...dialogProps} />
           </IndividualDialogContext.Provider>
         );
