fix: fix useAriaHidden deps (#5422)

* fix: fix useAriaHidden deps

* docs: add changeset

* test: add tests for useAriaHidden

Co-authored-by: Segun Adebayo <joseshegs@gmail.com>

.changeset/nervous-plums-own.md

@@ -0,0 +1,5 @@
+---
+"@chakra-ui/modal": patch
+---
+
+Fix `useAriaHidden` hook dependency to make it work as expected

packages/modal/src/use-modal.ts

@@ -1,7 +1,7 @@
 import { useIds } from "@chakra-ui/hooks"
 import { callAllHandlers } from "@chakra-ui/utils"
 import { mergeRefs, PropGetter } from "@chakra-ui/react-utils"
-import { hideOthers, Undo } from "aria-hidden"
+import { hideOthers } from "aria-hidden"
 import {
   KeyboardEvent,
   MouseEvent,
@@ -203,19 +203,14 @@ export function useAriaHidden(
   ref: RefObject<HTMLElement>,
   shouldHide: boolean,
 ) {
-  useEffect(() => {
-    if (!ref.current) return undefined
-
-    let undo: Undo | null = null
+  // save current ref in a local var to trigger the effect on identity change
+  const currentElement = ref.current
 
-    if (shouldHide && ref.current) {
-      undo = hideOthers(ref.current)
-    }
+  useEffect(() => {
+    // keep using `ref.current` inside the effect
+    // it may have changed during render and the execution of the effect
+    if (!ref.current || !shouldHide) return undefined
 
-    return () => {
-      if (shouldHide) {
-        undo?.()
-      }
-    }
-  }, [shouldHide, ref])
+    return hideOthers(ref.current)
+  }, [shouldHide, ref, currentElement])
 }

packages/modal/tests/use-modal.test.tsx

@@ -0,0 +1,36 @@
+import { renderHook } from "@testing-library/react-hooks"
+import { hideOthers } from "aria-hidden"
+import { MutableRefObject } from "react"
+import { useAriaHidden } from "../src"
+
+jest.mock("aria-hidden")
+
+beforeEach(() => {
+  jest.clearAllMocks()
+})
+
+describe("useAriaHidden", () => {
+  it("should be triggered if ref.current is changed", () => {
+    const ref: MutableRefObject<null | HTMLElement> = { current: null }
+
+    renderHook(() => useAriaHidden(ref, true))
+    expect(hideOthers).not.toBeCalled()
+
+    ref.current = document.createElement("div")
+
+    renderHook(() => useAriaHidden(ref, true))
+    expect(hideOthers).toBeCalledWith(ref.current)
+  })
+
+  it("shouldn't be triggered if `shouldHide` is `false`", () => {
+    const ref: MutableRefObject<null | HTMLElement> = { current: null }
+
+    renderHook(() => useAriaHidden(ref, true))
+    expect(hideOthers).not.toBeCalled()
+
+    ref.current = document.createElement("div")
+
+    renderHook(() => useAriaHidden(ref, false))
+    expect(hideOthers).not.toBeCalled()
+  })
+})