From a3c23908b11a8cab1769e4e7c0b423cc8ffd2ee2 Mon Sep 17 00:00:00 2001
From: Adi Dahiya <adi.dahiya14@gmail.com>
Date: Fri, 26 Jan 2024 12:07:35 -0500
Subject: [PATCH 1/4] WIP fix useOverlayStack

---
 .../core/src/components/overlay2/overlay2.tsx | 21 ++++++++++++-------
 .../core/src/components/popover/popover.md    |  4 ++--
 .../core/src/components/popover/popover.tsx   |  2 +-
 packages/core/src/components/portal/portal.md | 14 ++++++-------
 .../src/components/toast/overlayToaster.tsx   |  2 +-
 .../components/toast/overlayToasterProps.ts   |  2 +-
 .../src/context/overlays/overlaysProvider.tsx | 12 +++++++----
 packages/core/src/docs/variables.md           |  2 +-
 .../hooks/overlays/useLegacyOverlayStack.ts   |  3 +++
 .../src/hooks/overlays/useOverlayStack.ts     | 10 +++++++++
 packages/core/test/dialog/dialogTests.tsx     |  2 +-
 .../docs-app/src/components/blueprintDocs.tsx | 12 +++++------
 .../docs-theme/src/components/scrollbar.ts    |  2 +-
 packages/table-dev-app/src/index.tsx          |  8 +++----
 14 files changed, 59 insertions(+), 37 deletions(-)

diff --git a/packages/core/src/components/overlay2/overlay2.tsx b/packages/core/src/components/overlay2/overlay2.tsx
index 4c45bb6a00..870a97da79 100644
--- a/packages/core/src/components/overlay2/overlay2.tsx
+++ b/packages/core/src/components/overlay2/overlay2.tsx
@@ -36,8 +36,7 @@ import {
     setRef,
 } from "../../common/utils";
 import { hasDOMEnvironment } from "../../common/utils/domUtils";
-import { useOverlayStack } from "../../hooks/overlays/useOverlayStack";
-import { usePrevious } from "../../hooks/usePrevious";
+import { useOverlayStack, usePrevious } from "../../hooks";
 import type { OverlayProps } from "../overlay/overlayProps";
 import { getKeyboardFocusableElements } from "../overlay/overlayUtils";
 import { Portal } from "../portal/portal";
@@ -116,7 +115,8 @@ export const Overlay2 = React.forwardRef<OverlayInstance, Overlay2Props>((props,
     } = props;
 
     useOverlay2Validation(props);
-    const { closeOverlay, getLastOpened, getThisOverlayAndDescendants, openOverlay } = useOverlayStack();
+    const { closeOverlay, getLastOpened, getThisOverlayAndDescendants, openOverlay, getStack } = useOverlayStack();
+    const stack = getStack();
 
     const [isAutoFocusing, setIsAutoFocusing] = React.useState(false);
     const [hasEverOpened, setHasEverOpened] = React.useState(false);
@@ -210,7 +210,7 @@ export const Overlay2 = React.forwardRef<OverlayInstance, Overlay2Props>((props,
         [autoFocus, bringFocusInsideOverlay, enforceFocus, handleDocumentFocus, hasBackdrop, id, usePortal],
     );
 
-    const handleDocumentClick = React.useCallback(
+    const handleDocumentMousedown = React.useCallback(
         (e: MouseEvent) => {
             if (instance.current == null) {
                 return;
@@ -274,7 +274,8 @@ export const Overlay2 = React.forwardRef<OverlayInstance, Overlay2Props>((props,
         }
 
         if (canOutsideClickClose && !hasBackdrop) {
-            document.addEventListener("mousedown", handleDocumentClick);
+            console.log("adding document mousedown listener, stack is: ", stack);
+            document.addEventListener("mousedown", handleDocumentMousedown);
         }
 
         setRef(lastActiveElementBeforeOpened, getActiveElement(getRef(containerElement)));
@@ -284,10 +285,11 @@ export const Overlay2 = React.forwardRef<OverlayInstance, Overlay2Props>((props,
         canOutsideClickClose,
         enforceFocus,
         getLastOpened,
-        handleDocumentClick,
+        handleDocumentMousedown,
         handleDocumentFocus,
         hasBackdrop,
         openOverlay,
+        stack,
     ]);
 
     const overlayWillClose = React.useCallback(() => {
@@ -296,8 +298,9 @@ export const Overlay2 = React.forwardRef<OverlayInstance, Overlay2Props>((props,
         }
 
         document.removeEventListener("focus", handleDocumentFocus, /* useCapture */ true);
-        document.removeEventListener("mousedown", handleDocumentClick);
+        document.removeEventListener("mousedown", handleDocumentMousedown);
 
+        console.log("closing overlay", instance.current);
         closeOverlay(instance.current);
         const lastOpenedOverlay = getLastOpened();
         if (lastOpenedOverlay !== undefined) {
@@ -309,7 +312,7 @@ export const Overlay2 = React.forwardRef<OverlayInstance, Overlay2Props>((props,
                 document.addEventListener("focus", lastOpenedOverlay.handleDocumentFocus, /* useCapture */ true);
             }
         }
-    }, [closeOverlay, getLastOpened, handleDocumentClick, handleDocumentFocus]);
+    }, [closeOverlay, getLastOpened, handleDocumentMousedown, handleDocumentFocus]);
 
     const prevIsOpen = usePrevious(isOpen) ?? false;
     React.useEffect(() => {
@@ -332,6 +335,8 @@ export const Overlay2 = React.forwardRef<OverlayInstance, Overlay2Props>((props,
     React.useEffect(() => {
         return () => {
             if (isOpen) {
+                // if the overlay is still open, we need to run cleanup code to remove some event handlers
+                console.log("----- unmounting, running overlayWillClose");
                 overlayWillClose();
             }
         };
diff --git a/packages/core/src/components/popover/popover.md b/packages/core/src/components/popover/popover.md
index 277a7d76d1..c983204b06 100644
--- a/packages/core/src/components/popover/popover.md
+++ b/packages/core/src/components/popover/popover.md
@@ -426,7 +426,7 @@ performance. If your components are not updating in a synchronous fashion as exp
 `setTimeout` to wait for asynchronous Popover rendering to catch up:
 
 ```tsx
-import { Classes, Overlay, Popover } from "@blueprintjs/core";
+import { Classes, Overlay2, Popover } from "@blueprintjs/core";
 import { assert } from "chai";
 import { mount } from "enzyme";
 import { Target } from "react-popper";
@@ -445,7 +445,7 @@ wrapper.find(`.${Classes.POPOVER}`).hostNodes().simulate("mouseleave");
 
 setTimeout(() => {
     // Popover delays closing using setTimeout, so need to defer this check too.
-    const isOpen = wrapper.find(Overlay).prop("isOpen");
+    const isOpen = wrapper.find(Overlay2).prop("isOpen");
     assert.equal(isOpen, false);
 });
 ```
diff --git a/packages/core/src/components/popover/popover.tsx b/packages/core/src/components/popover/popover.tsx
index 7d5831b295..38782d736f 100644
--- a/packages/core/src/components/popover/popover.tsx
+++ b/packages/core/src/components/popover/popover.tsx
@@ -206,7 +206,7 @@ export class Popover<
     public targetRef = React.createRef<HTMLElement>();
 
     /**
-     * Overlay transition container element ref.
+     * Overlay2 transition container element ref.
      */
     private transitionContainerElement = React.createRef<HTMLDivElement>();
 
diff --git a/packages/core/src/components/portal/portal.md b/packages/core/src/components/portal/portal.md
index 67052a6487..52363404be 100644
--- a/packages/core/src/components/portal/portal.md
+++ b/packages/core/src/components/portal/portal.md
@@ -1,19 +1,19 @@
 @# Portal
 
-The __Portal__ component renders its children into a new DOM "subtree" outside of the current component
-hierarchy. It is an essential piece of the [Overlay](#core/components/overlay) component, responsible for
+The **Portal** component renders its children into a new DOM "subtree" outside of the current component
+hierarchy. It is an essential piece of the [Overlay2](#core/components/overlay2) component, responsible for
 ensuring that the overlay contents appear above the rest of the application. In most cases, you do not
 need to use a Portal directly; this documentation is provided only for reference.
 
 @## DOM Behavior
 
-__Portal__ component functions like a declarative `appendChild()`. The children of a __Portal__ are inserted into a *new child* of the target element. This target element is determined in the following order:
+**Portal** component functions like a declarative `appendChild()`. The children of a **Portal** are inserted into a _new child_ of the target element. This target element is determined in the following order:
+
 1. The `container` prop, if specified
 2. The `portalContainer` from the closest [PortalProvider](#core/context/portal-provider), if specified
 3. Otherwise `document.body`
 
-
-__Portal__ is used inside [Overlay](#core/components/overlay) to actually overlay the content on the
+**Portal** is used inside [Overlay2](#core/components/overlay2) to actually overlay the content on the
 application.
 
 <div class="@ns-callout @ns-intent-warning @ns-icon-move @ns-callout-has-body-content">
@@ -31,7 +31,7 @@ apply `position: absolute` to the `<body>` tag.
 
 @## React context options
 
-__Portal__ supports some customization through [React context](https://react.dev/learn/passing-data-deeply-with-context).
+**Portal** supports some customization through [React context](https://react.dev/learn/passing-data-deeply-with-context).
 Using this API can be helpful if you need to apply some custom styling or logic to _all_ Blueprint
 components which use portals (popovers, tooltips, dialogs, etc.). You can do so by rendering a
 [PortalProvider](#core/context/portal-provider) in your React tree
@@ -63,7 +63,7 @@ This feature uses React's legacy context API. Support for this API will be remov
 
 </div>
 
-__Portal__ supports the following options via the [React legacy context API](https://reactjs.org/docs/legacy-context.html).
+**Portal** supports the following options via the [React legacy context API](https://reactjs.org/docs/legacy-context.html).
 To use them, supply a child context to a subtree that contains the Portals you want to customize.
 
 @interface PortalLegacyContext
diff --git a/packages/core/src/components/toast/overlayToaster.tsx b/packages/core/src/components/toast/overlayToaster.tsx
index d2681f719b..42e046db25 100644
--- a/packages/core/src/components/toast/overlayToaster.tsx
+++ b/packages/core/src/components/toast/overlayToaster.tsx
@@ -235,7 +235,7 @@ export class OverlayToaster extends AbstractPureComponent<OverlayToasterProps, O
     }
 
     /**
-     * If provided `Toast` children, automaticaly upgrade them to `Toast2` elements so that `Overlay` can inject
+     * If provided `Toast` children, automaticaly upgrade them to `Toast2` elements so that `Overlay2` can inject
      * refs into them for use by `CSSTransition`. This is a bit hacky but ensures backwards compatibility for
      * `OverlayToaster`. It should be an uncommon code path in most applications, since we expect most usage to
      * occur via the imperative toaster APIs.
diff --git a/packages/core/src/components/toast/overlayToasterProps.ts b/packages/core/src/components/toast/overlayToasterProps.ts
index 4ad22fef68..68808d07ad 100644
--- a/packages/core/src/components/toast/overlayToasterProps.ts
+++ b/packages/core/src/components/toast/overlayToasterProps.ts
@@ -45,7 +45,7 @@ export interface OverlayToasterProps extends Props {
      */
     canEscapeKeyClear?: boolean;
 
-    /** Toasts to display inside the Overlay. */
+    /** Toasts to display inside the Overlay2. */
     children?: React.ReactNode;
 
     /**
diff --git a/packages/core/src/context/overlays/overlaysProvider.tsx b/packages/core/src/context/overlays/overlaysProvider.tsx
index 0f78a8382d..2a917e7121 100644
--- a/packages/core/src/context/overlays/overlaysProvider.tsx
+++ b/packages/core/src/context/overlays/overlaysProvider.tsx
@@ -41,7 +41,8 @@ export type OverlayStackAction =
 
 export type OverlaysContextInstance = readonly [OverlaysContextState, React.Dispatch<OverlayStackAction>];
 
-const initialOverlaysState: OverlaysContextState = { hasProvider: false, stack: [] };
+const initialStateWithoutProvider: OverlaysContextState = { hasProvider: false, stack: [] };
+const initialStateWithProvider: OverlaysContextState = { hasProvider: true, stack: [] };
 const noOpDispatch: React.Dispatch<OverlayStackAction> = () => null;
 
 /**
@@ -54,7 +55,10 @@ const noOpDispatch: React.Dispatch<OverlayStackAction> = () => null;
  *
  * For more information, see the [OverlaysProvider documentation](https://blueprintjs.com/docs/#core/context/overlays-provider).
  */
-export const OverlaysContext = React.createContext<OverlaysContextInstance>([initialOverlaysState, noOpDispatch]);
+export const OverlaysContext = React.createContext<OverlaysContextInstance>([
+    initialStateWithoutProvider,
+    noOpDispatch,
+]);
 
 /**
  * N.B. this is exported in order for `useOverlayStack()` to implement backwards-compatibility for overlays
@@ -73,7 +77,7 @@ export const overlaysReducer = (state: OverlaysContextState, action: OverlayStac
             newStack.splice(index, 1);
             return { ...state, stack: newStack };
         case "RESET_STACK":
-            return initialOverlaysState;
+            return { ...state, stack: [] };
         default:
             return state;
     }
@@ -90,6 +94,6 @@ export interface OverlaysProviderProps {
  * @see https://blueprintjs.com/docs/#core/context/overlays-provider
  */
 export const OverlaysProvider = ({ children }: OverlaysProviderProps) => {
-    const contextValue = React.useReducer(overlaysReducer, { ...initialOverlaysState, hasProvider: true });
+    const contextValue = React.useReducer(overlaysReducer, initialStateWithProvider);
     return <OverlaysContext.Provider value={contextValue}>{children}</OverlaysContext.Provider>;
 };
diff --git a/packages/core/src/docs/variables.md b/packages/core/src/docs/variables.md
index 537aaaa6dd..0b1228623d 100644
--- a/packages/core/src/docs/variables.md
+++ b/packages/core/src/docs/variables.md
@@ -89,7 +89,7 @@ without writing much CSS. Here are some resources for learning flexbox:
 @## Layering
 
 Blueprint provides variables for three z-index layers. This should be enough for most use cases,
-especially if you make correct use of [stacking context][MDN]. [Overlay](#core/components/overlay)
+especially if you make correct use of [stacking context][MDN]. [Overlay2](#core/components/overlay2)
 components such as dialogs and popovers use these z-index values to configure their stacking
 contexts.
 
diff --git a/packages/core/src/hooks/overlays/useLegacyOverlayStack.ts b/packages/core/src/hooks/overlays/useLegacyOverlayStack.ts
index b9a6a0b9a4..0491acf890 100644
--- a/packages/core/src/hooks/overlays/useLegacyOverlayStack.ts
+++ b/packages/core/src/hooks/overlays/useLegacyOverlayStack.ts
@@ -99,9 +99,12 @@ export function useLegacyOverlayStack(): UseOverlayStackReturnValue {
         [stack],
     );
 
+    const getStack = React.useCallback(() => stack, [stack]);
+
     return {
         closeOverlay,
         getLastOpened,
+        getStack,
         getThisOverlayAndDescendants,
         openOverlay,
         resetStack,
diff --git a/packages/core/src/hooks/overlays/useOverlayStack.ts b/packages/core/src/hooks/overlays/useOverlayStack.ts
index 384926aedd..199e776dd8 100644
--- a/packages/core/src/hooks/overlays/useOverlayStack.ts
+++ b/packages/core/src/hooks/overlays/useOverlayStack.ts
@@ -35,6 +35,8 @@ export interface UseOverlayStackReturnValue {
      */
     getLastOpened: () => OverlayInstance | undefined;
 
+    getStack: () => OverlayInstance[];
+
     /**
      * @param overlay current overlay
      * @returns a list of the current overlay and all overlays which are descendants of it.
@@ -68,6 +70,7 @@ export function useOverlayStack(): UseOverlayStackReturnValue {
 
     const getThisOverlayAndDescendants = React.useCallback(
         (overlay: OverlayInstance) => {
+            console.log("looking for overlay and descendants in stack:", stack, overlay.containerElement.current);
             const stackIndex = stack.findIndex(o => o.id === overlay.id);
             return stack.slice(stackIndex);
         },
@@ -103,6 +106,12 @@ export function useOverlayStack(): UseOverlayStackReturnValue {
         [dispatch, stack],
     );
 
+    const getStack = React.useCallback(() => stack, [stack]);
+
+    React.useEffect(() => {
+        console.info("overlay stack", stack);
+    }, [stack]);
+
     if (!state.hasProvider) {
         if (isNodeEnv("development")) {
             console.error(OVERLAY2_REQUIRES_OVERLAY_PROVDER);
@@ -113,6 +122,7 @@ export function useOverlayStack(): UseOverlayStackReturnValue {
     return {
         closeOverlay,
         getLastOpened,
+        getStack,
         getThisOverlayAndDescendants,
         openOverlay,
         resetStack,
diff --git a/packages/core/test/dialog/dialogTests.tsx b/packages/core/test/dialog/dialogTests.tsx
index b18de8168b..42504170a9 100644
--- a/packages/core/test/dialog/dialogTests.tsx
+++ b/packages/core/test/dialog/dialogTests.tsx
@@ -197,7 +197,7 @@ describe("<Dialog>", () => {
         });
     });
 
-    // N.B. everything else about Dialog is tested by Overlay
+    // N.B. everything else about Dialog is tested by Overlay2
 
     function renderDialogBodyAndFooter(): React.JSX.Element[] {
         return [
diff --git a/packages/docs-app/src/components/blueprintDocs.tsx b/packages/docs-app/src/components/blueprintDocs.tsx
index bb82e5a580..901880557c 100644
--- a/packages/docs-app/src/components/blueprintDocs.tsx
+++ b/packages/docs-app/src/components/blueprintDocs.tsx
@@ -107,9 +107,9 @@ export class BlueprintDocs extends React.Component<BlueprintDocsProps, { themeNa
             />
         );
         return (
-            <HotkeysProvider>
-                <PortalProvider>
-                    <OverlaysProvider>
+            <PortalProvider>
+                <OverlaysProvider>
+                    <HotkeysProvider>
                         <Documentation
                             {...this.props}
                             className={this.state.themeName}
@@ -122,9 +122,9 @@ export class BlueprintDocs extends React.Component<BlueprintDocsProps, { themeNa
                             renderPageActions={this.renderPageActions}
                             renderViewSourceLinkText={this.renderViewSourceLinkText}
                         />
-                    </OverlaysProvider>
-                </PortalProvider>
-            </HotkeysProvider>
+                    </HotkeysProvider>
+                </OverlaysProvider>
+            </PortalProvider>
         );
     }
 
diff --git a/packages/docs-theme/src/components/scrollbar.ts b/packages/docs-theme/src/components/scrollbar.ts
index b506411b15..48675da8b0 100644
--- a/packages/docs-theme/src/components/scrollbar.ts
+++ b/packages/docs-theme/src/components/scrollbar.ts
@@ -18,7 +18,7 @@ import { Classes } from "@blueprintjs/core";
 
 /**
  * Inject some CSS style rules into a new `<style>` element to add padding equal to the
- * width of the scrollbar when an `Overlay` is open, such that page content will not
+ * width of the scrollbar when an overlay is open, such that page content will not
  * shift due to the disappearing vertical scrollbar.
  */
 export function addScrollbarStyle() {
diff --git a/packages/table-dev-app/src/index.tsx b/packages/table-dev-app/src/index.tsx
index 87e7440f21..6f65ade8dc 100644
--- a/packages/table-dev-app/src/index.tsx
+++ b/packages/table-dev-app/src/index.tsx
@@ -24,10 +24,10 @@ import { Nav } from "./nav";
 
 ReactDOM.render(<Nav selected="index" />, document.getElementById("nav"));
 ReactDOM.render(
-    <HotkeysProvider>
-        <OverlaysProvider>
+    <OverlaysProvider>
+        <HotkeysProvider>
             <MutableTable />
-        </OverlaysProvider>
-    </HotkeysProvider>,
+        </HotkeysProvider>
+    </OverlaysProvider>,
     document.querySelector("#page-content"),
 );

From 414982fd40432475242f7152092806702f5c0c67 Mon Sep 17 00:00:00 2001
From: Adi Dahiya <adi.dahiya14@gmail.com>
Date: Mon, 29 Jan 2024 17:00:31 -0500
Subject: [PATCH 2/4] use mutable stack

---
 .../core/src/components/overlay2/overlay2.tsx | 105 ++++++++++--------
 packages/core/src/context/index.ts            |   2 +-
 .../src/context/overlays/overlaysProvider.tsx |  53 +++------
 .../hooks/overlays/useLegacyOverlayStack.ts   |  35 +++---
 .../src/hooks/overlays/useOverlayStack.ts     |  38 +++----
 5 files changed, 104 insertions(+), 129 deletions(-)

diff --git a/packages/core/src/components/overlay2/overlay2.tsx b/packages/core/src/components/overlay2/overlay2.tsx
index 870a97da79..ce9588f4bd 100644
--- a/packages/core/src/components/overlay2/overlay2.tsx
+++ b/packages/core/src/components/overlay2/overlay2.tsx
@@ -54,6 +54,9 @@ export interface OverlayInstance {
     /** Document "focus" event handler which needs to be attached & detached appropriately. */
     handleDocumentFocus: (e: FocusEvent) => void;
 
+    /** Document "mousedown" event handler which needs to be attached & detached appropriately. */
+    handleDocumentMousedown: (e: MouseEvent) => void;
+
     /** Unique ID for this overlay which helps to identify it across prop changes. */
     id: string;
 
@@ -115,8 +118,7 @@ export const Overlay2 = React.forwardRef<OverlayInstance, Overlay2Props>((props,
     } = props;
 
     useOverlay2Validation(props);
-    const { closeOverlay, getLastOpened, getThisOverlayAndDescendants, openOverlay, getStack } = useOverlayStack();
-    const stack = getStack();
+    const { closeOverlay, getLastOpened, getThisOverlayAndDescendants, openOverlay } = useOverlayStack();
 
     const [isAutoFocusing, setIsAutoFocusing] = React.useState(false);
     const [hasEverOpened, setHasEverOpened] = React.useState(false);
@@ -148,7 +150,7 @@ export const Overlay2 = React.forwardRef<OverlayInstance, Overlay2Props>((props,
             const container = getRef(containerElement);
             const activeElement = getActiveElement(container);
 
-            if (container == null || activeElement == null || !isOpen) {
+            if (container == null || activeElement == null) {
                 return;
             }
 
@@ -159,11 +161,18 @@ export const Overlay2 = React.forwardRef<OverlayInstance, Overlay2Props>((props,
                 setIsAutoFocusing(false);
             }
         });
-    }, [isOpen]);
+    }, []);
+
+    /** Unique ID for this overlay in the global stack */
+    const id = useOverlay2ID();
+
+    // N.B. use `null` here and not simply `undefined` because `useImperativeHandle` will set `null` on unmount,
+    // and we need the following code to be resilient to that value.
+    const instance = React.useRef<OverlayInstance>(null);
 
     /**
-     * When multiple Overlays are open, this event handler is only active for the most recently
-     * opened one to avoid Overlays competing with each other for focus.
+     * When multiple `enforceFocus` Overlays are open, this event handler is only active for the most
+     * recently opened one to avoid Overlays competing with each other for focus.
      */
     const handleDocumentFocus = React.useCallback(
         (e: FocusEvent) => {
@@ -171,45 +180,17 @@ export const Overlay2 = React.forwardRef<OverlayInstance, Overlay2Props>((props,
             // see https://github.com/palantir/blueprint/issues/4220
             const eventTarget = e.composed ? e.composedPath()[0] : e.target;
             const container = getRef(containerElement);
-            if (
-                enforceFocus &&
-                container != null &&
-                eventTarget instanceof Node &&
-                !container.contains(eventTarget as HTMLElement)
-            ) {
+            if (container != null && eventTarget instanceof Node && !container.contains(eventTarget as HTMLElement)) {
                 // prevent default focus behavior (sometimes auto-scrolls the page)
                 e.preventDefault();
                 e.stopImmediatePropagation();
                 bringFocusInsideOverlay();
             }
         },
-        [bringFocusInsideOverlay, enforceFocus],
-    );
-
-    const id = useOverlay2ID();
-
-    // N.B. use `null` here and not simply `undefined` because `useImperativeHandle` will set `null` on unmount,
-    // and we need the following code to be resilient to that value.
-    const instance = React.useRef<OverlayInstance>(null);
-    // send this instance's imperative handle to the the forwarded ref as well as our local ref
-    const ref = React.useMemo(() => mergeRefs(forwardedRef, instance), [forwardedRef]);
-    React.useImperativeHandle(
-        ref,
-        () => ({
-            bringFocusInsideOverlay,
-            containerElement,
-            handleDocumentFocus,
-            id,
-            props: {
-                autoFocus,
-                enforceFocus,
-                hasBackdrop,
-                usePortal,
-            },
-        }),
-        [autoFocus, bringFocusInsideOverlay, enforceFocus, handleDocumentFocus, hasBackdrop, id, usePortal],
+        [bringFocusInsideOverlay],
     );
 
+    // N.B. this listener is only kept attached when `isOpen={true}` and `canOutsideClickClose={true}`
     const handleDocumentMousedown = React.useCallback(
         (e: MouseEvent) => {
             if (instance.current == null) {
@@ -230,12 +211,52 @@ export const Overlay2 = React.forwardRef<OverlayInstance, Overlay2Props>((props,
                 },
             );
 
-            if (isOpen && !isClickInThisOverlayOrDescendant && canOutsideClickClose) {
+            if (!isClickInThisOverlayOrDescendant) {
                 // casting to any because this is a native event
                 onClose?.(e as any);
             }
         },
-        [canOutsideClickClose, getThisOverlayAndDescendants, isOpen, onClose],
+        [getThisOverlayAndDescendants, onClose],
+    );
+
+    // Important: clean up old document-level event listeners if their memoized values change (this is rare, but
+    // may happen, for example, if a user forgets to use `React.useCallback` in the `props.onClose` value).
+    // Otherwise, we will lose the reference to those values and create a memory leak since we won't be able
+    // to successfully detach them inside overlayWillClose.
+    React.useEffect(() => {
+        document.removeEventListener("mousedown", handleDocumentMousedown);
+    }, [handleDocumentMousedown]);
+    React.useEffect(() => {
+        document.removeEventListener("focus", handleDocumentFocus, /* useCapture */ true);
+    }, [handleDocumentFocus]);
+
+    // send this instance's imperative handle to the the forwarded ref as well as our local ref
+    const ref = React.useMemo(() => mergeRefs(forwardedRef, instance), [forwardedRef]);
+    React.useImperativeHandle(
+        ref,
+        () => ({
+            bringFocusInsideOverlay,
+            containerElement,
+            handleDocumentFocus,
+            handleDocumentMousedown,
+            id,
+            props: {
+                autoFocus,
+                enforceFocus,
+                hasBackdrop,
+                usePortal,
+            },
+        }),
+        [
+            autoFocus,
+            bringFocusInsideOverlay,
+            enforceFocus,
+            handleDocumentFocus,
+            handleDocumentMousedown,
+            hasBackdrop,
+            id,
+            usePortal,
+        ],
     );
 
     const handleContainerKeyDown = React.useCallback(
@@ -274,7 +295,6 @@ export const Overlay2 = React.forwardRef<OverlayInstance, Overlay2Props>((props,
         }
 
         if (canOutsideClickClose && !hasBackdrop) {
-            console.log("adding document mousedown listener, stack is: ", stack);
             document.addEventListener("mousedown", handleDocumentMousedown);
         }
 
@@ -289,7 +309,6 @@ export const Overlay2 = React.forwardRef<OverlayInstance, Overlay2Props>((props,
         handleDocumentFocus,
         hasBackdrop,
         openOverlay,
-        stack,
     ]);
 
     const overlayWillClose = React.useCallback(() => {
@@ -300,7 +319,6 @@ export const Overlay2 = React.forwardRef<OverlayInstance, Overlay2Props>((props,
         document.removeEventListener("focus", handleDocumentFocus, /* useCapture */ true);
         document.removeEventListener("mousedown", handleDocumentMousedown);
 
-        console.log("closing overlay", instance.current);
         closeOverlay(instance.current);
         const lastOpenedOverlay = getLastOpened();
         if (lastOpenedOverlay !== undefined) {
@@ -312,7 +330,7 @@ export const Overlay2 = React.forwardRef<OverlayInstance, Overlay2Props>((props,
                 document.addEventListener("focus", lastOpenedOverlay.handleDocumentFocus, /* useCapture */ true);
             }
         }
-    }, [closeOverlay, getLastOpened, handleDocumentMousedown, handleDocumentFocus]);
+    }, [closeOverlay, getLastOpened, handleDocumentFocus, handleDocumentMousedown]);
 
     const prevIsOpen = usePrevious(isOpen) ?? false;
     React.useEffect(() => {
@@ -336,7 +354,6 @@ export const Overlay2 = React.forwardRef<OverlayInstance, Overlay2Props>((props,
         return () => {
             if (isOpen) {
                 // if the overlay is still open, we need to run cleanup code to remove some event handlers
-                console.log("----- unmounting, running overlayWillClose");
                 overlayWillClose();
             }
         };
diff --git a/packages/core/src/context/index.ts b/packages/core/src/context/index.ts
index 86a648b8dc..f76abc73bf 100644
--- a/packages/core/src/context/index.ts
+++ b/packages/core/src/context/index.ts
@@ -23,7 +23,7 @@ export {
 export {
     OverlaysContext,
     OverlaysProvider,
-    type OverlaysContextInstance,
+    type OverlaysContextState,
     type OverlaysProviderProps,
 } from "./overlays/overlaysProvider";
 export { PortalContext, type PortalContextOptions, PortalProvider } from "./portal/portalProvider";
diff --git a/packages/core/src/context/overlays/overlaysProvider.tsx b/packages/core/src/context/overlays/overlaysProvider.tsx
index 2a917e7121..b9613b083f 100644
--- a/packages/core/src/context/overlays/overlaysProvider.tsx
+++ b/packages/core/src/context/overlays/overlaysProvider.tsx
@@ -18,7 +18,12 @@ import * as React from "react";
 
 import type { OverlayInstance } from "../../components/overlay2/overlay2";
 
-interface OverlaysContextState {
+// N.B. using a mutable ref for the stack is much easier to work with in the world of hooks and FCs.
+// This matches the mutable global behavior of the old Overlay implementation in Blueprint v5. An alternative
+// approach was considered with an immutable array data structure and a reducer, but that implementation
+// caused lots of unnecessary invalidation of `React.useCallback()` for document-level event handlers, which
+// led to memory leaks and bugs.
+export interface OverlaysContextState {
     /**
      * Whether the context instance is being used within a tree which has an `<OverlaysProvider>`.
      * `useOverlayStack()` will work if this is `false` in Blueprint v5, but this will be unsupported
@@ -32,19 +37,9 @@ interface OverlaysContextState {
     /**
      * The application-wide global overlay stack.
      */
-    stack: OverlayInstance[];
+    stack: React.MutableRefObject<OverlayInstance[]>;
 }
 
-export type OverlayStackAction =
-    | { type: "OPEN_OVERLAY" | "CLOSE_OVERLAY"; payload: OverlayInstance }
-    | { type: "RESET_STACK" };
-
-export type OverlaysContextInstance = readonly [OverlaysContextState, React.Dispatch<OverlayStackAction>];
-
-const initialStateWithoutProvider: OverlaysContextState = { hasProvider: false, stack: [] };
-const initialStateWithProvider: OverlaysContextState = { hasProvider: true, stack: [] };
-const noOpDispatch: React.Dispatch<OverlayStackAction> = () => null;
-
 /**
  * A React context used to interact with the overlay stack in an application.
  * Users should take care to make sure that only _one_ of these is instantiated and used within an
@@ -55,33 +50,10 @@ const noOpDispatch: React.Dispatch<OverlayStackAction> = () => null;
  *
  * For more information, see the [OverlaysProvider documentation](https://blueprintjs.com/docs/#core/context/overlays-provider).
  */
-export const OverlaysContext = React.createContext<OverlaysContextInstance>([
-    initialStateWithoutProvider,
-    noOpDispatch,
-]);
-
-/**
- * N.B. this is exported in order for `useOverlayStack()` to implement backwards-compatibility for overlays
- * outside of a `<OverlaysProvider>. It should stop being exported from this module in Blueprint v6.
- */
-export const overlaysReducer = (state: OverlaysContextState, action: OverlayStackAction): OverlaysContextState => {
-    switch (action.type) {
-        case "OPEN_OVERLAY":
-            return { ...state, stack: [...state.stack, action.payload] };
-        case "CLOSE_OVERLAY":
-            const index = state.stack.findIndex(o => o.id === action.payload.id);
-            if (index === -1) {
-                return state;
-            }
-            const newStack = state.stack.slice();
-            newStack.splice(index, 1);
-            return { ...state, stack: newStack };
-        case "RESET_STACK":
-            return { ...state, stack: [] };
-        default:
-            return state;
-    }
-};
+export const OverlaysContext = React.createContext<OverlaysContextState>({
+    hasProvider: false,
+    stack: { current: [] },
+});
 
 export interface OverlaysProviderProps {
     /** The component subtree which will have access to this overlay stack context. */
@@ -94,6 +66,7 @@ export interface OverlaysProviderProps {
  * @see https://blueprintjs.com/docs/#core/context/overlays-provider
  */
 export const OverlaysProvider = ({ children }: OverlaysProviderProps) => {
-    const contextValue = React.useReducer(overlaysReducer, initialStateWithProvider);
+    const stack = React.useRef<OverlayInstance[]>([]);
+    const contextValue = React.useMemo(() => ({ hasProvider: true, stack }), [stack]);
     return <OverlaysContext.Provider value={contextValue}>{children}</OverlaysContext.Provider>;
 };
diff --git a/packages/core/src/hooks/overlays/useLegacyOverlayStack.ts b/packages/core/src/hooks/overlays/useLegacyOverlayStack.ts
index 0491acf890..7a263bf813 100644
--- a/packages/core/src/hooks/overlays/useLegacyOverlayStack.ts
+++ b/packages/core/src/hooks/overlays/useLegacyOverlayStack.ts
@@ -20,34 +20,23 @@ import { useSyncExternalStore } from "use-sync-external-store/shim";
 
 import { Classes } from "../../common";
 import type { OverlayInstance } from "../../components";
-import { overlaysReducer, type OverlayStackAction } from "../../context/overlays/overlaysProvider";
 
 import type { UseOverlayStackReturnValue } from "./useOverlayStack";
 
-let legacyGlobalOverlayStack: OverlayInstance[] = [];
-let legacyGlobalOverlayStoreListeners: Array<() => void> = [];
+const legacyGlobalOverlayStack: OverlayInstance[] = [];
+const legacyGlobalOverlayStoreListeners: Array<() => void> = [];
 
 const legacyGlobalOverlayStackStore = {
     getSnapshot: () => legacyGlobalOverlayStack,
     subscribe: (listener: () => void) => {
-        legacyGlobalOverlayStoreListeners = [...legacyGlobalOverlayStoreListeners, listener];
+        legacyGlobalOverlayStoreListeners.push(listener);
         return () => {
-            legacyGlobalOverlayStoreListeners = legacyGlobalOverlayStoreListeners.filter(l => l !== listener);
+            const index = legacyGlobalOverlayStoreListeners.indexOf(listener);
+            legacyGlobalOverlayStoreListeners.splice(index, 1);
         };
     },
 };
 
-/**
- * Public only for testing.
- */
-export const dispatch: React.Dispatch<OverlayStackAction> = action => {
-    const state = { stack: legacyGlobalOverlayStack, hasProvider: false };
-    const newStack = overlaysReducer(state, action).stack;
-    legacyGlobalOverlayStack = newStack;
-    // IMPORTANT: notify all listeners that the global stack has changed
-    legacyGlobalOverlayStoreListeners.forEach(listener => listener());
-};
-
 /**
  * Legacy implementation of a global overlay stack which maintains state in a global variable.
  * This is used for backwards-compatibility with overlay-based components in Blueprint v5.
@@ -73,10 +62,12 @@ export function useLegacyOverlayStack(): UseOverlayStackReturnValue {
         [stack],
     );
 
-    const resetStack = React.useCallback(() => dispatch({ type: "RESET_STACK" }), []);
+    const resetStack = React.useCallback(() => {
+        legacyGlobalOverlayStack.splice(0, legacyGlobalOverlayStack.length);
+    }, []);
 
     const openOverlay = React.useCallback((overlay: OverlayInstance) => {
-        dispatch({ type: "OPEN_OVERLAY", payload: overlay });
+        legacyGlobalOverlayStack.push(overlay);
         if (overlay.props.usePortal && overlay.props.hasBackdrop) {
             // add a class to the body to prevent scrolling of content below the overlay
             document.body.classList.add(Classes.OVERLAY_OPEN);
@@ -89,7 +80,10 @@ export function useLegacyOverlayStack(): UseOverlayStackReturnValue {
                 o => o.props.usePortal && o.props.hasBackdrop && o.id !== overlay.id,
             );
 
-            dispatch({ type: "CLOSE_OVERLAY", payload: overlay });
+            const index = legacyGlobalOverlayStack.findIndex(o => o.id === overlay.id);
+            if (index > -1) {
+                legacyGlobalOverlayStack.splice(index, 1);
+            }
 
             if (otherOverlaysWithBackdrop.length === 0) {
                 // remove body class which prevents scrolling of content below overlay
@@ -99,12 +93,9 @@ export function useLegacyOverlayStack(): UseOverlayStackReturnValue {
         [stack],
     );
 
-    const getStack = React.useCallback(() => stack, [stack]);
-
     return {
         closeOverlay,
         getLastOpened,
-        getStack,
         getThisOverlayAndDescendants,
         openOverlay,
         resetStack,
diff --git a/packages/core/src/hooks/overlays/useOverlayStack.ts b/packages/core/src/hooks/overlays/useOverlayStack.ts
index 199e776dd8..f62a556258 100644
--- a/packages/core/src/hooks/overlays/useOverlayStack.ts
+++ b/packages/core/src/hooks/overlays/useOverlayStack.ts
@@ -35,8 +35,6 @@ export interface UseOverlayStackReturnValue {
      */
     getLastOpened: () => OverlayInstance | undefined;
 
-    getStack: () => OverlayInstance[];
-
     /**
      * @param overlay current overlay
      * @returns a list of the current overlay and all overlays which are descendants of it.
@@ -62,57 +60,54 @@ export interface UseOverlayStackReturnValue {
  */
 export function useOverlayStack(): UseOverlayStackReturnValue {
     // get the overlay stack from application-wide React context
-    const [state, dispatch] = React.useContext(OverlaysContext);
+    const { stack, hasProvider } = React.useContext(OverlaysContext);
     const legacyOverlayStack = useLegacyOverlayStack();
-    const { stack } = state;
 
-    const getLastOpened = React.useCallback(() => stack.at(-1), [stack]);
+    const getLastOpened = React.useCallback(() => stack.current.at(-1), [stack]);
 
     const getThisOverlayAndDescendants = React.useCallback(
         (overlay: OverlayInstance) => {
-            console.log("looking for overlay and descendants in stack:", stack, overlay.containerElement.current);
-            const stackIndex = stack.findIndex(o => o.id === overlay.id);
-            return stack.slice(stackIndex);
+            const stackIndex = stack.current.findIndex(o => o.id === overlay.id);
+            return stack.current.slice(stackIndex);
         },
         [stack],
     );
 
-    const resetStack = React.useCallback(() => dispatch({ type: "RESET_STACK" }), [dispatch]);
+    const resetStack = React.useCallback(() => {
+        stack.current = [];
+    }, [stack]);
 
     const openOverlay = React.useCallback(
         (overlay: OverlayInstance) => {
-            dispatch({ type: "OPEN_OVERLAY", payload: overlay });
+            stack.current.push(overlay);
             if (overlay.props.usePortal && overlay.props.hasBackdrop) {
                 // add a class to the body to prevent scrolling of content below the overlay
                 document.body.classList.add(Classes.OVERLAY_OPEN);
             }
         },
-        [dispatch],
+        [stack],
     );
 
     const closeOverlay = React.useCallback(
         (overlay: OverlayInstance) => {
-            const otherOverlaysWithBackdrop = stack.filter(
+            const otherOverlaysWithBackdrop = stack.current.filter(
                 o => o.props.usePortal && o.props.hasBackdrop && o.id !== overlay.id,
             );
 
-            dispatch({ type: "CLOSE_OVERLAY", payload: overlay });
+            const index = stack.current.findIndex(o => o.id === overlay.id);
+            if (index > -1) {
+                stack.current.splice(index, 1);
+            }
 
             if (otherOverlaysWithBackdrop.length === 0) {
                 // remove body class which prevents scrolling of content below overlay
                 document.body.classList.remove(Classes.OVERLAY_OPEN);
             }
         },
-        [dispatch, stack],
+        [stack],
     );
 
-    const getStack = React.useCallback(() => stack, [stack]);
-
-    React.useEffect(() => {
-        console.info("overlay stack", stack);
-    }, [stack]);
-
-    if (!state.hasProvider) {
+    if (!hasProvider) {
         if (isNodeEnv("development")) {
             console.error(OVERLAY2_REQUIRES_OVERLAY_PROVDER);
         }
@@ -122,7 +117,6 @@ export function useOverlayStack(): UseOverlayStackReturnValue {
     return {
         closeOverlay,
         getLastOpened,
-        getStack,
         getThisOverlayAndDescendants,
         openOverlay,
         resetStack,

From 47ce70382dd269c6c7d921cfd544c926408d584e Mon Sep 17 00:00:00 2001
From: Adi Dahiya <adi.dahiya14@gmail.com>
Date: Mon, 29 Jan 2024 18:34:02 -0500
Subject: [PATCH 3/4] fix tests

---
 .../core/src/components/overlay2/overlay2.tsx |  2 +-
 .../hooks/overlays/useLegacyOverlayStack.ts   | 30 ++++++++++++-------
 .../src/hooks/overlays/useOverlayStack.ts     | 11 +++++--
 .../core/test/hooks/useOverlayStackTests.tsx  | 28 +++++++++++------
 4 files changed, 48 insertions(+), 23 deletions(-)

diff --git a/packages/core/src/components/overlay2/overlay2.tsx b/packages/core/src/components/overlay2/overlay2.tsx
index ce9588f4bd..5b6ca9aa65 100644
--- a/packages/core/src/components/overlay2/overlay2.tsx
+++ b/packages/core/src/components/overlay2/overlay2.tsx
@@ -55,7 +55,7 @@ export interface OverlayInstance {
     handleDocumentFocus: (e: FocusEvent) => void;
 
     /** Document "mousedown" event handler which needs to be attached & detached appropriately. */
-    handleDocumentMousedown: (e: MouseEvent) => void;
+    handleDocumentMousedown?: (e: MouseEvent) => void;
 
     /** Unique ID for this overlay which helps to identify it across prop changes. */
     id: string;
diff --git a/packages/core/src/hooks/overlays/useLegacyOverlayStack.ts b/packages/core/src/hooks/overlays/useLegacyOverlayStack.ts
index 7a263bf813..2e410fe7e8 100644
--- a/packages/core/src/hooks/overlays/useLegacyOverlayStack.ts
+++ b/packages/core/src/hooks/overlays/useLegacyOverlayStack.ts
@@ -23,16 +23,26 @@ import type { OverlayInstance } from "../../components";
 
 import type { UseOverlayStackReturnValue } from "./useOverlayStack";
 
-const legacyGlobalOverlayStack: OverlayInstance[] = [];
-const legacyGlobalOverlayStoreListeners: Array<() => void> = [];
+const globalStack: OverlayInstance[] = [];
+const globalStackListeners: Array<() => void> = [];
+
+/**
+ * Modify the global stack in-place and notify all listeners of the updated value.
+ *
+ * @public for testing
+ */
+export const modifyGlobalStack = (fn: (stack: OverlayInstance[]) => void) => {
+    fn(globalStack);
+    globalStackListeners.forEach(listener => listener());
+};
 
 const legacyGlobalOverlayStackStore = {
-    getSnapshot: () => legacyGlobalOverlayStack,
+    getSnapshot: () => globalStack,
     subscribe: (listener: () => void) => {
-        legacyGlobalOverlayStoreListeners.push(listener);
+        globalStackListeners.push(listener);
         return () => {
-            const index = legacyGlobalOverlayStoreListeners.indexOf(listener);
-            legacyGlobalOverlayStoreListeners.splice(index, 1);
+            const index = globalStackListeners.indexOf(listener);
+            globalStackListeners.splice(index, 1);
         };
     },
 };
@@ -63,11 +73,11 @@ export function useLegacyOverlayStack(): UseOverlayStackReturnValue {
     );
 
     const resetStack = React.useCallback(() => {
-        legacyGlobalOverlayStack.splice(0, legacyGlobalOverlayStack.length);
+        modifyGlobalStack(s => s.splice(0, s.length));
     }, []);
 
     const openOverlay = React.useCallback((overlay: OverlayInstance) => {
-        legacyGlobalOverlayStack.push(overlay);
+        globalStack.push(overlay);
         if (overlay.props.usePortal && overlay.props.hasBackdrop) {
             // add a class to the body to prevent scrolling of content below the overlay
             document.body.classList.add(Classes.OVERLAY_OPEN);
@@ -80,9 +90,9 @@ export function useLegacyOverlayStack(): UseOverlayStackReturnValue {
                 o => o.props.usePortal && o.props.hasBackdrop && o.id !== overlay.id,
             );
 
-            const index = legacyGlobalOverlayStack.findIndex(o => o.id === overlay.id);
+            const index = globalStack.findIndex(o => o.id === overlay.id);
             if (index > -1) {
-                legacyGlobalOverlayStack.splice(index, 1);
+                globalStack.splice(index, 1);
             }
 
             if (otherOverlaysWithBackdrop.length === 0) {
diff --git a/packages/core/src/hooks/overlays/useOverlayStack.ts b/packages/core/src/hooks/overlays/useOverlayStack.ts
index f62a556258..378cbb1699 100644
--- a/packages/core/src/hooks/overlays/useOverlayStack.ts
+++ b/packages/core/src/hooks/overlays/useOverlayStack.ts
@@ -63,12 +63,17 @@ export function useOverlayStack(): UseOverlayStackReturnValue {
     const { stack, hasProvider } = React.useContext(OverlaysContext);
     const legacyOverlayStack = useLegacyOverlayStack();
 
-    const getLastOpened = React.useCallback(() => stack.current.at(-1), [stack]);
+    const getLastOpened = React.useCallback(() => {
+        return stack.current.at(-1);
+    }, [stack]);
 
     const getThisOverlayAndDescendants = React.useCallback(
         (overlay: OverlayInstance) => {
-            const stackIndex = stack.current.findIndex(o => o.id === overlay.id);
-            return stack.current.slice(stackIndex);
+            const index = stack.current.findIndex(o => o.id === overlay.id);
+            if (index === -1) {
+                return [];
+            }
+            return stack.current.slice(index);
         },
         [stack],
     );
diff --git a/packages/core/test/hooks/useOverlayStackTests.tsx b/packages/core/test/hooks/useOverlayStackTests.tsx
index c10ba7a9ed..bcca0ed57b 100644
--- a/packages/core/test/hooks/useOverlayStackTests.tsx
+++ b/packages/core/test/hooks/useOverlayStackTests.tsx
@@ -24,7 +24,7 @@ import type { OverlayProps } from "../../src/components/overlay/overlayProps";
 import type { OverlayInstance } from "../../src/components/overlay2/overlay2";
 import { OverlaysProvider } from "../../src/context";
 import { useOverlayStack, usePrevious } from "../../src/hooks";
-import { dispatch as legacyOverlayStackDispatch } from "../../src/hooks/overlays/useLegacyOverlayStack";
+import { modifyGlobalStack } from "../../src/hooks/overlays/useLegacyOverlayStack";
 
 interface TestComponentProps extends OverlayProps {
     handleLastOpenedChange?: (lastOpened: OverlayInstance | undefined) => void;
@@ -122,7 +122,7 @@ const TestComponentWithProvider: React.FC<TestComponentProps> = props => {
 describe("useOverlayStack()", () => {
     const handleLastOpenedChange = spy();
     const containerRef = React.createRef<HTMLDivElement>();
-    const DEFAULT_PROPS: TestComponentProps = {
+    const TEST_PROPS_CLOSED: TestComponentProps = {
         autoFocus: true,
         containerRef,
         enforceFocus: true,
@@ -131,6 +131,10 @@ describe("useOverlayStack()", () => {
         isOpen: false,
         usePortal: true,
     };
+    const TEST_PROPS_OPEN: TestComponentProps = {
+        ...TEST_PROPS_CLOSED,
+        isOpen: true,
+    };
 
     afterEach(() => {
         handleLastOpenedChange.resetHistory();
@@ -138,12 +142,15 @@ describe("useOverlayStack()", () => {
 
     describe("with <OverlaysProvider>", () => {
         it("should render without crashing", () => {
-            render(<TestComponentWithProvider {...DEFAULT_PROPS} />);
+            render(<TestComponentWithProvider {...TEST_PROPS_CLOSED} />);
         });
 
         it("opening an overlay should change the result of getLastOpened()", () => {
-            const { rerender } = render(<TestComponentWithProvider {...DEFAULT_PROPS} />);
-            rerender(<TestComponentWithProvider {...DEFAULT_PROPS} isOpen={true} />);
+            const { rerender } = render(<TestComponentWithProvider {...TEST_PROPS_CLOSED} />);
+            // we need to re-render twice: the overlay is added to the stack _after_ the first re-render completes,
+            // so it wont' trigger a change in getLastOpened() until the second re-render.
+            rerender(<TestComponentWithProvider {...TEST_PROPS_OPEN} />);
+            rerender(<TestComponentWithProvider {...TEST_PROPS_OPEN} />);
             expect(
                 handleLastOpenedChange.calledOnce,
                 `expected getLastOpened() result to change after re-rendering with isOpen={true}`,
@@ -158,16 +165,19 @@ describe("useOverlayStack()", () => {
     describe("without <OverlaysProvider>", () => {
         before(() => {
             // ensure there is a clean global state that might be polluted by other test suites
-            legacyOverlayStackDispatch({ type: "RESET_STACK" });
+            modifyGlobalStack(s => s.splice(0, s.length));
         });
 
         it("should render without crashing", () => {
-            render(<TestComponentWithoutProvider {...DEFAULT_PROPS} />);
+            render(<TestComponentWithoutProvider {...TEST_PROPS_CLOSED} />);
         });
 
         it("opening an overlay should change the result of getLastOpened()", () => {
-            const { rerender } = render(<TestComponentWithoutProvider {...DEFAULT_PROPS} />);
-            rerender(<TestComponentWithoutProvider {...DEFAULT_PROPS} isOpen={true} />);
+            const { rerender } = render(<TestComponentWithoutProvider {...TEST_PROPS_CLOSED} />);
+            // we need to re-render twice: the overlay is added to the stack _after_ the first re-render completes,
+            // so it wont' trigger a change in getLastOpened() until the second re-render.
+            rerender(<TestComponentWithoutProvider {...TEST_PROPS_OPEN} />);
+            rerender(<TestComponentWithoutProvider {...TEST_PROPS_OPEN} />);
             expect(
                 handleLastOpenedChange.callCount > 0,
                 `expected getLastOpened() result to change after re-rendering with isOpen={true}`,

From e03f00f74c264bd52aa822bcc04d4373abc5206d Mon Sep 17 00:00:00 2001
From: Adi Dahiya <adi.dahiya14@gmail.com>
Date: Mon, 29 Jan 2024 18:37:16 -0500
Subject: [PATCH 4/4] fix lint: break import cycle

---
 packages/core/src/components/overlay2/overlay2.tsx | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/packages/core/src/components/overlay2/overlay2.tsx b/packages/core/src/components/overlay2/overlay2.tsx
index 5b6ca9aa65..46efc7c159 100644
--- a/packages/core/src/components/overlay2/overlay2.tsx
+++ b/packages/core/src/components/overlay2/overlay2.tsx
@@ -36,7 +36,8 @@ import {
     setRef,
 } from "../../common/utils";
 import { hasDOMEnvironment } from "../../common/utils/domUtils";
-import { useOverlayStack, usePrevious } from "../../hooks";
+import { useOverlayStack } from "../../hooks/overlays/useOverlayStack";
+import { usePrevious } from "../../hooks/usePrevious";
 import type { OverlayProps } from "../overlay/overlayProps";
 import { getKeyboardFocusableElements } from "../overlay/overlayUtils";
 import { Portal } from "../portal/portal";