[dialog] Fix closing behavior under strict mode

CLAUDE.md

@@ -0,0 +1,36 @@
+# Claude Instructions for Toolpad Project
+
+## Workspace Commands
+
+**IMPORTANT**: This is a monorepo managed with pnpm workspaces. Always use workspace commands instead of changing directories:
+
+### Correct Usage
+
+```bash
+# Run commands in specific workspace
+pnpm -F @toolpad/core <command>
+
+# Examples:
+pnpm -F @toolpad/core test
+pnpm -F @toolpad/core build
+pnpm -F @toolpad/core lint
+```
+
+### Avoid
+
+```bash
+# DON'T do this
+cd packages/toolpad-core && pnpm test
+```
+
+## Available Workspaces
+
+- `@toolpad/core` - Core React components and hooks
+- `@toolpad/utils` - Shared utilities
+- `create-toolpad-app` - Project scaffolding tool
+
+## Testing
+
+- Always run tests using `pnpm -F <workspace> test`
+- For linting: `pnpm eslint`
+- For type checking: `pnpm -F <workspace> typescript`

packages/toolpad-core/src/DashboardLayout/DashboardLayout.browser.test.tsx

@@ -6,18 +6,22 @@ import { page } from '@vitest/browser/context';
 import { AppProvider } from '../AppProvider';
 import { DashboardLayout } from './DashboardLayout';
 
+const IMG_URL =
+  'data:image/svg+xml;utf8,<svg width="80" height="80" xmlns="http://www.w3.org/2000/svg"><text x="10" y="60" font-family="Arial, sans-serif" font-size="60" fill="black">M</text></svg>';
+
 describe('DashboardLayout', () => {
   test('renders branding correctly in header', async () => {
     const BRANDING = {
       title: 'My Company',
-      logo: <img src="https://placehold.co/600x400" alt="Placeholder Logo" />,
+      logo: <img src={IMG_URL} alt="Placeholder Logo" />,
     };
 
     render(
       <AppProvider branding={BRANDING}>
         <DashboardLayout>Hello world</DashboardLayout>
       </AppProvider>,
     );
+
     await page.screenshot();
   });
 });

packages/toolpad-core/src/useDialogs/DialogsProvider.tsx

@@ -37,6 +37,7 @@ function DialogsProvider(props: DialogProviderProps) {
   const [stack, setStack] = React.useState<DialogStackEntry<any, any>[]>([]);
   const keyPrefix = React.useId();
   const nextId = React.useRef(0);
+  const dialogMetadata = React.useRef(new WeakMap<Promise<any>, DialogStackEntry<any, any>>());
 
   const requestDialog = useEventCallback<OpenDialog>(function open<P, R>(
     Component: DialogComponent<P, R>,
@@ -63,6 +64,9 @@ function DialogsProvider(props: DialogProviderProps) {
       resolve,
     };
 
+    // Store metadata for reliable access during close
+    dialogMetadata.current.set(promise, newEntry);
+
     setStack((prevStack) => [...prevStack, newEntry]);
     return promise;
   });
@@ -74,18 +78,23 @@ function DialogsProvider(props: DialogProviderProps) {
     setTimeout(() => {
       // wait for closing animation
       setStack((prevStack) => prevStack.filter((entry) => entry.promise !== dialog));
+      // WeakMap automatically cleans up when promise is garbage collected
     }, unmountAfter);
   });
 
   const closeDialog = useEventCallback(async function closeDialog<R>(
     dialog: Promise<R>,
     result: R,
   ) {
-    const entryToClose = stack.find((entry) => entry.promise === dialog);
+    const entryToClose = dialogMetadata.current.get(dialog);
     invariant(entryToClose, 'dialog not found');
-    await entryToClose.onClose(result);
-    entryToClose.resolve(result);
-    closeDialogUi(dialog);
+
+    try {
+      await entryToClose.onClose(result);
+    } finally {
+      entryToClose.resolve(result);
+      closeDialogUi(dialog);
+    }
     return dialog;
   });
 

packages/toolpad-core/src/useDialogs/useDialogs.test.tsx

@@ -3,8 +3,8 @@
  */
 
 import * as React from 'react';
-import { describe, test, expect } from 'vitest';
-import { renderHook, within, screen, waitFor } from '@testing-library/react';
+import { describe, test, expect, vi } from 'vitest';
+import { renderHook, within, screen, waitFor, render } from '@testing-library/react';
 import { userEvent } from '@testing-library/user-event';
 import { DialogProps, useDialogs } from './useDialogs';
 import { DialogsProvider } from './DialogsProvider';
@@ -259,4 +259,99 @@ describe('useDialogs', () => {
       await waitFor(() => expect(screen.queryByRole('dialog')).toBeFalsy());
     });
   });
+
+  describe('React Strict Mode behavior', () => {
+    test('should not leave dialogs open when effect runs twice', async () => {
+      function CustomDialog({ open }: DialogProps) {
+        return open ? <div role="dialog">Custom Dialog Content</div> : null;
+      }
+
+      function TestComponent() {
+        const dialogs = useDialogs();
+        const dialogRef = React.useRef<Promise<void> | null>(null);
+
+        React.useEffect(() => {
+          const dialog = dialogs.open(CustomDialog);
+          dialogRef.current = dialog;
+
+          return () => {
+            dialogs.close(dialog, undefined);
+            dialogRef.current = null;
+          };
+        }, [dialogs]);
+
+        React.useEffect(() => {
+          const timeout = setTimeout(() => {
+            if (dialogRef.current) {
+              dialogs.close(dialogRef.current, undefined);
+              dialogRef.current = null;
+            }
+          }, 50);
+          return () => clearTimeout(timeout);
+        });
+
+        return <div>Test Component</div>;
+      }
+
+      render(
+        <DialogsProvider>
+          <TestComponent />
+        </DialogsProvider>,
+        { reactStrictMode: true },
+      );
+
+      await waitFor(
+        async () => {
+          const dialogs = screen.queryAllByRole('dialog');
+          expect(dialogs.length).toBeLessThanOrEqual(0);
+        },
+        { timeout: 100 },
+      );
+    });
+  });
+
+  describe('stale closure', () => {
+    test('can close dialog after multiple re-renders', async () => {
+      const { result, rerender } = renderHook(() => useDialogs(), { wrapper: TestWrapper });
+
+      const theDialog = result.current.alert('Hello');
+
+      // Force multiple re-renders to create stale closures
+      rerender();
+      rerender();
+      rerender();
+
+      await screen.findByRole('dialog');
+
+      // This should work even with stale closures
+      await result.current.close(theDialog, undefined);
+
+      await waitFor(() => expect(screen.queryByRole('dialog')).toBeFalsy());
+    });
+
+    test('throws when closing unknown dialog', async () => {
+      const { result } = renderHook(() => useDialogs(), { wrapper: TestWrapper });
+
+      const fakeDialog = Promise.resolve(undefined);
+
+      await expect(result.current.close(fakeDialog, undefined)).rejects.toThrow('dialog not found');
+    });
+
+    test('dialog still closes when onClose callback throws', async () => {
+      const onCloseError = new Error('onClose failed');
+      const onCloseMock = vi.fn().mockRejectedValue(onCloseError);
+      const { result } = renderHook(() => useDialogs(), { wrapper: TestWrapper });
+
+      const theDialog = result.current.alert('Hello', { onClose: onCloseMock });
+
+      await screen.findByRole('dialog');
+
+      // Close should throw the onClose error
+      await expect(result.current.close(theDialog, undefined)).rejects.toThrow('onClose failed');
+
+      // But dialog should still be closed in UI and promise should be resolved
+      await waitFor(() => expect(screen.queryByRole('dialog')).toBeFalsy());
+      await expect(theDialog).resolves.toBeUndefined();
+    });
+  });
 });

packages/toolpad-core/src/useDialogs/useDialogs.tsx

@@ -9,6 +9,7 @@ import DialogContentText from '@mui/material/DialogContentText';
 import { useNonNullableContext } from '@toolpad/utils/react';
 import invariant from 'invariant';
 import * as React from 'react';
+import useEventCallback from '@mui/utils/useEventCallback';
 import { DialogsContext } from './DialogsContext';
 import { WindowContext } from '../shared/context';
 import { useLocaleText, type LocaleText } from '../AppProvider/LocalizationProvider';
@@ -337,19 +338,16 @@ export function PromptDialog({ open, payload, onClose }: PromptDialogProps) {
 export function useDialogs(): DialogHook {
   const { open, close } = useNonNullableContext(DialogsContext);
 
-  const alert = React.useCallback<OpenAlertDialog>(
-    (msg, { onClose, ...options } = {}) => open(AlertDialog, { ...options, msg }, { onClose }),
-    [open],
+  const alert = useEventCallback<OpenAlertDialog>((msg, { onClose, ...options } = {}) =>
+    open(AlertDialog, { ...options, msg }, { onClose }),
   );
 
-  const confirm = React.useCallback<OpenConfirmDialog>(
-    (msg, { onClose, ...options } = {}) => open(ConfirmDialog, { ...options, msg }, { onClose }),
-    [open],
+  const confirm = useEventCallback<OpenConfirmDialog>((msg, { onClose, ...options } = {}) =>
+    open(ConfirmDialog, { ...options, msg }, { onClose }),
   );
 
-  const prompt = React.useCallback<OpenPromptDialog>(
-    (msg, { onClose, ...options } = {}) => open(PromptDialog, { ...options, msg }, { onClose }),
-    [open],
+  const prompt = useEventCallback<OpenPromptDialog>((msg, { onClose, ...options } = {}) =>
+    open(PromptDialog, { ...options, msg }, { onClose }),
   );
 
   return React.useMemo(