diff --git a/.changeset/tangy-items-give.md b/.changeset/tangy-items-give.md new file mode 100644 index 000000000..18170391b --- /dev/null +++ b/.changeset/tangy-items-give.md @@ -0,0 +1,5 @@ +--- +'@vapor-ui/core': patch +--- + +Change useMutationObserver hook to use only ref diff --git a/packages/core/src/components/navigation-menu/navigation-menu.tsx b/packages/core/src/components/navigation-menu/navigation-menu.tsx index 8d7f2ad56..4d8017426 100644 --- a/packages/core/src/components/navigation-menu/navigation-menu.tsx +++ b/packages/core/src/components/navigation-menu/navigation-menu.tsx @@ -7,7 +7,7 @@ import { NavigationMenu as BaseNavigationMenu } from '@base-ui-components/react' import { ChevronDownOutlineIcon } from '@vapor-ui/icons'; import clsx from 'clsx'; -import { useMutationObserver } from '~/hooks/use-mutation-observer'; +import { useMutationObserverRef } from '~/hooks/use-mutation-observer-ref'; import { createContext } from '~/libs/create-context'; import { createSlot } from '~/libs/create-slot'; import { vars } from '~/styles/themes.css'; @@ -296,7 +296,7 @@ export const NavigationMenuPopupPrimitive = forwardRef< if (initialAlign) setAlign(initialAlign); }, []); - const arrowRef = useMutationObserver({ + const arrowRef = useMutationObserverRef({ callback: (mutations) => { mutations.forEach((mutation) => { const { attributeName, target: mutationTarget } = mutation; diff --git a/packages/core/src/components/popover/popover.tsx b/packages/core/src/components/popover/popover.tsx index 1ec00fc25..408964a8f 100644 --- a/packages/core/src/components/popover/popover.tsx +++ b/packages/core/src/components/popover/popover.tsx @@ -6,7 +6,7 @@ import { forwardRef, useEffect, useMemo, useRef, useState } from 'react'; import { Popover as BasePopover } from '@base-ui-components/react/popover'; import clsx from 'clsx'; -import { useMutationObserver } from '~/hooks/use-mutation-observer'; +import { useMutationObserverRef } from '~/hooks/use-mutation-observer-ref'; import { createSlot } from '~/libs/create-slot'; import { vars } from '~/styles/themes.css'; import { composeRefs } from '~/utils/compose-refs'; @@ -113,7 +113,7 @@ export const PopoverPopupPrimitive = forwardRef({ + const arrowRef = useMutationObserverRef({ callback: (mutations) => { mutations.forEach((mutation) => { const { attributeName, target: mutationTarget } = mutation; diff --git a/packages/core/src/components/tooltip/tooltip.tsx b/packages/core/src/components/tooltip/tooltip.tsx index 37435027a..518ade8a4 100644 --- a/packages/core/src/components/tooltip/tooltip.tsx +++ b/packages/core/src/components/tooltip/tooltip.tsx @@ -6,7 +6,7 @@ import { forwardRef, useEffect, useMemo, useRef, useState } from 'react'; import { Tooltip as BaseTooltip } from '@base-ui-components/react/tooltip'; import clsx from 'clsx'; -import { useMutationObserver } from '~/hooks/use-mutation-observer'; +import { useMutationObserverRef } from '~/hooks/use-mutation-observer-ref'; import { createSlot } from '~/libs/create-slot'; import { vars } from '~/styles/themes.css'; import { composeRefs } from '~/utils/compose-refs'; @@ -111,7 +111,7 @@ export const TooltipPopupPrimitive = forwardRef({ + const arrowRef = useMutationObserverRef({ callback: (mutations) => { mutations.forEach((mutation) => { const { attributeName, target: mutationTarget } = mutation; diff --git a/packages/core/src/hooks/use-mutation-observer-ref.test.tsx b/packages/core/src/hooks/use-mutation-observer-ref.test.tsx new file mode 100644 index 000000000..5a72b8c86 --- /dev/null +++ b/packages/core/src/hooks/use-mutation-observer-ref.test.tsx @@ -0,0 +1,201 @@ +import { render, waitFor } from '@testing-library/react'; +import { describe, expect, it, vi } from 'vitest'; + +import { useMutationObserverRef } from './use-mutation-observer-ref'; + +describe('useMutationObserverRef', () => { + it('should observe mutations on the attached element', async () => { + const callback = vi.fn(); + let testRef = null as HTMLDivElement | null; + + function TestComponent() { + const ref = useMutationObserverRef({ + callback, + options: { attributes: true, attributeFilter: ['data-test'] }, + }); + + return ( +
{ + testRef = node; + ref(node); + }} + data-test="initial" + > + Test Content +
+ ); + } + + render(); + + // Wait for the component to mount and observer to be set up + await waitFor(() => expect(testRef).not.toBeNull()); + + // Trigger a mutation + testRef!.setAttribute('data-test', 'changed'); + + // Wait for the callback to be called + await waitFor(() => { + expect(callback).toHaveBeenCalled(); + }); + + expect(callback.mock.calls[0][0]).toHaveLength(1); + expect(callback.mock.calls[0][0][0].type).toBe('attributes'); + expect(callback.mock.calls[0][0][0].attributeName).toBe('data-test'); + }); + + it('should disconnect observer when ref is set to null', async () => { + const callback = vi.fn(); + + function TestComponent({ show }: { show: boolean }) { + const ref = useMutationObserverRef({ + callback, + options: { attributes: true }, + }); + + if (!show) return null; + + return ( +
+ Test Content +
+ ); + } + + const { rerender } = render(); + + // Unmount the component + rerender(); + + // The observer should be disconnected, so no further mutations should be observed + await waitFor(() => { + expect(callback).not.toHaveBeenCalled(); + }); + }); + + it('should update to the latest callback on each render', async () => { + const firstCallback = vi.fn(); + const secondCallback = vi.fn(); + let testRef = null as HTMLDivElement | null; + + function TestComponent({ callback }: { callback: (mutations: MutationRecord[]) => void }) { + const ref = useMutationObserverRef({ + callback, + options: { attributes: true, attributeFilter: ['data-test'] }, + }); + + return ( +
{ + testRef = node; + ref(node); + }} + data-test="initial" + > + Test Content +
+ ); + } + + const { rerender } = render(); + + await waitFor(() => expect(testRef).not.toBeNull()); + + // Trigger first mutation + testRef!.setAttribute('data-test', 'changed1'); + + await waitFor(() => { + expect(firstCallback).toHaveBeenCalled(); + }); + + // Clear the first callback and switch to second + firstCallback.mockClear(); + rerender(); + + // Trigger second mutation + testRef!.setAttribute('data-test', 'changed2'); + + await waitFor(() => { + expect(secondCallback).toHaveBeenCalled(); + }); + + // First callback should not have been called again + expect(firstCallback).not.toHaveBeenCalled(); + }); + + it('should observe child list mutations when configured', async () => { + const callback = vi.fn(); + let testRef = null as HTMLDivElement | null; + + function TestComponent() { + const ref = useMutationObserverRef({ + callback, + options: { childList: true }, + }); + + return ( +
{ + testRef = node; + ref(node); + }} + > + Initial Child +
+ ); + } + + render(); + + await waitFor(() => expect(testRef).not.toBeNull()); + + // Add a new child + const newChild = document.createElement('span'); + newChild.textContent = 'New Child'; + testRef!.appendChild(newChild); + + await waitFor(() => { + expect(callback).toHaveBeenCalled(); + }); + + expect(callback.mock.calls[0][0][0].type).toBe('childList'); + }); + + it('should return cleanup function that disconnects observer (React 19+ support)', async () => { + const callback = vi.fn(); + let cleanupFn: (() => void) | undefined; + + function TestComponent() { + const ref = useMutationObserverRef({ + callback, + options: { attributes: true }, + }); + + return ( +
{ + const cleanup = ref(node); + // Store cleanup function if returned (React 19+) + if (typeof cleanup === 'function') { + cleanupFn = cleanup; + } + }} + data-test="initial" + > + Test Content +
+ ); + } + + const { unmount } = render(); + + // If cleanup function was returned, it should work + if (cleanupFn) { + cleanupFn(); + expect(callback).not.toHaveBeenCalled(); + } + + unmount(); + }); +}); diff --git a/packages/core/src/hooks/use-mutation-observer-ref.ts b/packages/core/src/hooks/use-mutation-observer-ref.ts new file mode 100644 index 000000000..7c85d6e98 --- /dev/null +++ b/packages/core/src/hooks/use-mutation-observer-ref.ts @@ -0,0 +1,67 @@ +import { useCallback, useRef } from 'react'; + +interface Params { + callback: (mutations: MutationRecord[]) => void; + options?: MutationObserverInit; +} + +/** + * A hook that returns a callback ref which sets up a MutationObserver + * when attached to a DOM element. + * + * This implementation uses callback refs instead of useEffect to avoid + * issues with stale dependencies and unnecessary observer recreation. + * It stores the callback and options in refs to maintain stable references + * across renders, ensuring compatibility with React 17+. + * + * For React 19+, the callback ref returns a cleanup function that will + * automatically disconnect the observer when the ref changes or unmounts. + * + * @see https://tkdodo.eu/blog/ref-callbacks-react-19-and-the-compiler + * + * @example + * const ref = useMutationObserverRef({ + * callback: (mutations) => console.log(mutations), + * options: { attributes: true } + * }); + * + * return
Content
; + */ +export const useMutationObserverRef = ({ callback, options }: Params) => { + const observerRef = useRef(null); + const callbackRef = useRef(callback); + const optionsRef = useRef(options); + + // Keep refs up to date with latest values + callbackRef.current = callback; + optionsRef.current = options; + + const refCallback = useCallback((node: T | null) => { + // Cleanup previous observer if it exists + if (observerRef.current) { + observerRef.current.disconnect(); + observerRef.current = null; + } + + // Set up new observer if node is present + if (node) { + // Use the refs to get the latest callback and options + const observer = new MutationObserver((mutations) => { + callbackRef.current(mutations); + }); + observer.observe(node, optionsRef.current); + observerRef.current = observer; + + // Return cleanup function for React 19+ + // For React 17-18, this return value is ignored + return () => { + observer.disconnect(); + if (observerRef.current === observer) { + observerRef.current = null; + } + }; + } + }, []); // Empty dependency array - stable across all renders + + return refCallback; +}; diff --git a/packages/core/src/hooks/use-mutation-observer.ts b/packages/core/src/hooks/use-mutation-observer.ts deleted file mode 100644 index 85eb01021..000000000 --- a/packages/core/src/hooks/use-mutation-observer.ts +++ /dev/null @@ -1,22 +0,0 @@ -import { useEffect, useRef } from 'react'; - -interface Params { - callback: (mutations: MutationRecord[]) => void; - options?: MutationObserverInit; -} - -export const useMutationObserver = ({ callback, options }: Params) => { - const elementRef = useRef(null); - - useEffect(() => { - const element = elementRef.current; - if (!element) return; - - const observer = new MutationObserver(callback); - observer.observe(element, options); - - return () => observer.disconnect(); - }, [callback, options]); - - return elementRef; -};