From db44f628b1df4371179ec2f5d0a78df0306227ef Mon Sep 17 00:00:00 2001 From: Reid Barber Date: Wed, 4 Mar 2026 14:39:20 -0600 Subject: [PATCH 1/2] perf(dnd): add useDynamicDescription to prevent unnecessary re-renders --- packages/@react-aria/dnd/src/useDrag.ts | 12 +- .../@react-aria/dnd/src/useVirtualDrop.ts | 6 +- packages/@react-aria/dnd/test/dnd.test.js | 78 ++++++++++ packages/@react-aria/utils/src/index.ts | 2 +- .../@react-aria/utils/src/useDescription.ts | 70 ++++++++- .../utils/test/useDescription.test.tsx | 145 ++++++++++++++++++ 6 files changed, 300 insertions(+), 13 deletions(-) create mode 100644 packages/@react-aria/utils/test/useDescription.test.tsx diff --git a/packages/@react-aria/dnd/src/useDrag.ts b/packages/@react-aria/dnd/src/useDrag.ts index d91c5b2dc6c..c52b017a1a8 100644 --- a/packages/@react-aria/dnd/src/useDrag.ts +++ b/packages/@react-aria/dnd/src/useDrag.ts @@ -15,7 +15,7 @@ import {DragEndEvent, DragItem, DragMoveEvent, DragPreviewRenderer, DragStartEve import {DragEvent, HTMLAttributes, version as ReactVersion, useEffect, useRef, useState} from 'react'; import * as DragManager from './DragManager'; import {DROP_EFFECT_TO_DROP_OPERATION, DROP_OPERATION, EFFECT_ALLOWED} from './constants'; -import {getEventTarget, isVirtualClick, isVirtualPointerEvent, useDescription, useGlobalListeners} from '@react-aria/utils'; +import {getEventTarget, isVirtualClick, isVirtualPointerEvent, useDynamicDescription, useGlobalListeners} from '@react-aria/utils'; import {globalDropEffect, setGlobalAllowedDropOperations, setGlobalDropEffect, useDragModality, writeToDataTransfer} from './utils'; // @ts-ignore import intlMessages from '../intl/*.json'; @@ -69,6 +69,11 @@ const MESSAGES = { } }; +const DESCRIPTION_KEYS = { + start: 'react-aria-dnd-drag-start', + end: 'react-aria-dnd-drag-end' +}; + /** * Handles drag interactions for an element, with support for traditional mouse and touch * based drag and drop, in addition to full parity for keyboard and screen reader users. @@ -302,9 +307,10 @@ export function useDrag(options: DragOptions): DragResult { }; let modality = useDragModality(); - let message = !isDragging ? MESSAGES[modality].start : MESSAGES[modality].end; + let messageType: 'start' | 'end' = !isDragging ? 'start' : 'end'; + let message = MESSAGES[modality][messageType]; - let descriptionProps = useDescription(stringFormatter.format(message)); + let descriptionProps = useDynamicDescription(stringFormatter.format(message), DESCRIPTION_KEYS[messageType]); let interactions: HTMLAttributes = {}; if (!hasDragButton) { diff --git a/packages/@react-aria/dnd/src/useVirtualDrop.ts b/packages/@react-aria/dnd/src/useVirtualDrop.ts index b74897d3cba..2450184ba41 100644 --- a/packages/@react-aria/dnd/src/useVirtualDrop.ts +++ b/packages/@react-aria/dnd/src/useVirtualDrop.ts @@ -15,8 +15,8 @@ import {DOMAttributes} from 'react'; import * as DragManager from './DragManager'; // @ts-ignore import intlMessages from '../intl/*.json'; -import {useDescription} from '@react-aria/utils'; import {useDragModality} from './utils'; +import {useDynamicDescription} from '@react-aria/utils'; import {useLocalizedStringFormatter} from '@react-aria/i18n'; interface VirtualDropResult { @@ -29,11 +29,13 @@ const MESSAGES = { virtual: 'dropDescriptionVirtual' }; +const DESCRIPTION_KEY = 'react-aria-dnd-drop'; + export function useVirtualDrop(): VirtualDropResult { let stringFormatter = useLocalizedStringFormatter(intlMessages, '@react-aria/dnd'); let modality = useDragModality(); let dragSession = DragManager.useDragSession(); - let descriptionProps = useDescription(dragSession ? stringFormatter.format(MESSAGES[modality]) : ''); + let descriptionProps = useDynamicDescription(dragSession ? stringFormatter.format(MESSAGES[modality]) : '', DESCRIPTION_KEY); return { dropProps: { diff --git a/packages/@react-aria/dnd/test/dnd.test.js b/packages/@react-aria/dnd/test/dnd.test.js index 36f984c5179..d285e8b59e7 100644 --- a/packages/@react-aria/dnd/test/dnd.test.js +++ b/packages/@react-aria/dnd/test/dnd.test.js @@ -2511,6 +2511,84 @@ describe('useDrag and useDrop', function () { expect(announce).toHaveBeenCalledWith('Drop canceled.'); }); + it('should keep dynamic description ids stable while description text updates', async () => { + let tree = render(<> + + + ); + + let draggable = tree.getByText('Drag me'); + let droppable = tree.getByText('Drop here'); + + fireEvent.focus(draggable); + let draggableDescriptionId = draggable.getAttribute('aria-describedby'); + expect(document.getElementById(draggableDescriptionId)).toHaveTextContent('Click to start dragging'); + + fireEvent.click(draggable); + act(() => jest.runAllTimers()); + + expect(draggable).toHaveAttribute('data-dragging', 'true'); + let dragEndDescriptionId = draggable.getAttribute('aria-describedby'); + expect(dragEndDescriptionId).not.toBe(draggableDescriptionId); + expect(document.getElementById(dragEndDescriptionId)).toHaveTextContent('Dragging. Click to cancel drag.'); + + let dropDescriptionId = droppable.getAttribute('aria-describedby'); + let dropDescriptionNode = document.getElementById(dropDescriptionId); + expect(dropDescriptionNode).toHaveTextContent('Click to drop.'); + + fireEvent.click(draggable); + + expect(draggable).toHaveAttribute('data-dragging', 'false'); + let restoredStartDescriptionId = draggable.getAttribute('aria-describedby'); + expect(document.getElementById(restoredStartDescriptionId)).toHaveTextContent('Click to start dragging'); + expect(droppable).not.toHaveAttribute('aria-describedby'); + expect(document.getElementById(dropDescriptionId)).toBe(dropDescriptionNode); + expect(dropDescriptionNode).toHaveTextContent(''); + + fireEvent.click(draggable); + act(() => jest.runAllTimers()); + + let secondDragEndDescriptionId = draggable.getAttribute('aria-describedby'); + expect(document.getElementById(secondDragEndDescriptionId)).toHaveTextContent('Dragging. Click to cancel drag.'); + expect(droppable.getAttribute('aria-describedby')).toBe(dropDescriptionId); + expect(document.getElementById(dropDescriptionId)).toBe(dropDescriptionNode); + expect(dropDescriptionNode).toHaveTextContent('Click to drop.'); + + fireEvent.click(draggable); + }); + + it('should share drag start descriptions across many draggables', () => { + let tree = render(<> + Drag me 1 + Drag me 2 + Drag me 3 + Drag me 4 + Drag me 5 + Drag me 6 + Drag me 7 + Drag me 8 + Drag me 9 + Drag me 10 + ); + + let ids = [ + tree.getByText('Drag me 1').getAttribute('aria-describedby'), + tree.getByText('Drag me 2').getAttribute('aria-describedby'), + tree.getByText('Drag me 3').getAttribute('aria-describedby'), + tree.getByText('Drag me 4').getAttribute('aria-describedby'), + tree.getByText('Drag me 5').getAttribute('aria-describedby'), + tree.getByText('Drag me 6').getAttribute('aria-describedby'), + tree.getByText('Drag me 7').getAttribute('aria-describedby'), + tree.getByText('Drag me 8').getAttribute('aria-describedby'), + tree.getByText('Drag me 9').getAttribute('aria-describedby'), + tree.getByText('Drag me 10').getAttribute('aria-describedby') + ]; + + expect(new Set(ids).size).toBe(1); + expect(document.querySelectorAll('[id^="react-aria-description-"]')).toHaveLength(1); + expect(document.getElementById(ids[0])).toHaveTextContent('Click to start dragging'); + }); + it('should support clicking the original drag target to cancel drag (virtual pointer event)', async () => { let tree = render(<> diff --git a/packages/@react-aria/utils/src/index.ts b/packages/@react-aria/utils/src/index.ts index be51b95cd7f..f5b308e206d 100644 --- a/packages/@react-aria/utils/src/index.ts +++ b/packages/@react-aria/utils/src/index.ts @@ -34,7 +34,7 @@ export {getScrollParent} from './getScrollParent'; export {getScrollParents} from './getScrollParents'; export {isScrollable} from './isScrollable'; export {useViewportSize} from './useViewportSize'; -export {useDescription} from './useDescription'; +export {useDescription, useDynamicDescription} from './useDescription'; export {isMac, isIPhone, isIPad, isIOS, isAppleDevice, isWebKit, isChrome, isAndroid, isFirefox} from './platform'; export {useEvent} from './useEvent'; export {useValueEffect} from './useValueEffect'; diff --git a/packages/@react-aria/utils/src/useDescription.ts b/packages/@react-aria/utils/src/useDescription.ts index 01f953e9c32..692896eebf1 100644 --- a/packages/@react-aria/utils/src/useDescription.ts +++ b/packages/@react-aria/utils/src/useDescription.ts @@ -12,10 +12,32 @@ import {AriaLabelingProps} from '@react-types/shared'; import {useLayoutEffect} from './useLayoutEffect'; -import {useState} from 'react'; +import {useRef, useState} from 'react'; let descriptionId = 0; -const descriptionNodes = new Map(); +const descriptionNodes = new Map(); +const dynamicDescriptionNodes = new Map(); + +function createDescriptionNode(id: string, description: string): HTMLElement { + let node = document.createElement('div'); + node.id = id; + node.style.display = 'none'; + node.textContent = description; + document.body.appendChild(node); + return node; +} + +function getOrCreateDynamicDescriptionNode(descriptionKey: string) { + let desc = dynamicDescriptionNodes.get(descriptionKey); + if (!desc) { + let id = `react-aria-description-${descriptionId++}`; + let node = createDescriptionNode(id, ''); + desc = {refCount: 0, element: node}; + dynamicDescriptionNodes.set(descriptionKey, desc); + } + + return desc; +} export function useDescription(description?: string): AriaLabelingProps { let [id, setId] = useState(); @@ -30,11 +52,7 @@ export function useDescription(description?: string): AriaLabelingProps { let id = `react-aria-description-${descriptionId++}`; setId(id); - let node = document.createElement('div'); - node.id = id; - node.style.display = 'none'; - node.textContent = description; - document.body.appendChild(node); + let node = createDescriptionNode(id, description); desc = {refCount: 0, element: node}; descriptionNodes.set(description, desc); } else { @@ -54,3 +72,41 @@ export function useDescription(description?: string): AriaLabelingProps { 'aria-describedby': description ? id : undefined }; } + +/** + * Provides a stable `aria-describedby` id for descriptions that change over time. + * Unlike `useDescription`, this shares a hidden element by `descriptionKey` + * and updates its text content in place so many consumers can reuse one id. + */ +export function useDynamicDescription(description: string | undefined, descriptionKey: string): AriaLabelingProps { + let [id, setId] = useState(); + let nodeRef = useRef(null); + + useLayoutEffect(() => { + let desc = getOrCreateDynamicDescriptionNode(descriptionKey); + desc.refCount++; + nodeRef.current = desc.element; + setId(desc.element.id); + + return () => { + if (--desc.refCount === 0) { + desc.element.remove(); + dynamicDescriptionNodes.delete(descriptionKey); + } + + if (nodeRef.current === desc.element) { + nodeRef.current = null; + } + }; + }, [descriptionKey]); + + useLayoutEffect(() => { + if (nodeRef.current) { + nodeRef.current.textContent = description || ''; + } + }, [description]); + + return { + 'aria-describedby': description ? id : undefined + }; +} diff --git a/packages/@react-aria/utils/test/useDescription.test.tsx b/packages/@react-aria/utils/test/useDescription.test.tsx new file mode 100644 index 00000000000..890fd2c250e --- /dev/null +++ b/packages/@react-aria/utils/test/useDescription.test.tsx @@ -0,0 +1,145 @@ +/* + * Copyright 2025 Adobe. All rights reserved. + * This file is licensed to you under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. You may obtain a copy + * of the License at http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software distributed under + * the License is distributed on an "AS IS" BASIS, WITHOUT WARRANTIES OR REPRESENTATIONS + * OF ANY KIND, either express or implied. See the License for the specific language + * governing permissions and limitations under the License. + */ + +import {actHook as act, renderHook} from '@react-spectrum/test-utils-internal'; +import {useDescription, useDynamicDescription} from '../src/useDescription'; + +describe('useDescription', () => { + it('should return an id if description is provided', () => { + let {result} = renderHook(() => useDescription('Test description')); + expect(result.current['aria-describedby']).toMatch(/^react-aria-description-\d+$/); + }); + + it('should return undefined if no description is provided', () => { + let {result} = renderHook(() => useDescription()); + expect(result.current['aria-describedby']).toBeUndefined(); + }); + + it('should reuse the same id for the same description', () => { + let {result: result1} = renderHook(() => useDescription('Test description')); + let {result: result2} = renderHook(() => useDescription('Test description')); + expect(result1.current['aria-describedby']).toBe(result2.current['aria-describedby']); + }); + + it('should create a new id for a new description', () => { + let {result: result1} = renderHook(() => useDescription('Test description 1')); + let {result: result2} = renderHook(() => useDescription('Test description 2')); + expect(result1.current['aria-describedby']).not.toBe(result2.current['aria-describedby']); + }); + + it('should clean up description node on unmount', () => { + let {result, unmount} = renderHook(() => useDescription('Test description')); + let id = result.current['aria-describedby']; + expect(document.getElementById(id!)).not.toBeNull(); + unmount(); + expect(document.getElementById(id!)).toBeNull(); + }); + + it('should not clean up if other components are using the same description', () => { + let {result: result1, unmount: unmount1} = renderHook(() => useDescription('Test description')); + let {unmount: unmount2} = renderHook(() => useDescription('Test description')); + let id = result1.current['aria-describedby']; + expect(document.getElementById(id!)).not.toBeNull(); + unmount1(); + expect(document.getElementById(id!)).not.toBeNull(); + unmount2(); + expect(document.getElementById(id!)).toBeNull(); + }); +}); + +describe('useDynamicDescription', () => { + it('should return an id if description is provided', () => { + let {result} = renderHook(() => useDynamicDescription('Test description', 'dynamic-1')); + expect(result.current['aria-describedby']).toMatch(/^react-aria-description-\d+$/); + }); + + it('should return undefined if no description is provided', () => { + let {result} = renderHook(() => useDynamicDescription(undefined, 'dynamic-2')); + expect(result.current['aria-describedby']).toBeUndefined(); + }); + + it('should reuse the same id for the same description key', () => { + let {result: result1} = renderHook(() => useDynamicDescription('Test description', 'shared-key')); + let {result: result2} = renderHook(() => useDynamicDescription('Test description', 'shared-key')); + expect(result1.current['aria-describedby']).toBe(result2.current['aria-describedby']); + }); + + it('should create a new id for a different description key', () => { + let {result: result1} = renderHook(() => useDynamicDescription('Test description', 'dynamic-3')); + let {result: result2} = renderHook(() => useDynamicDescription('Test description', 'dynamic-4')); + expect(result1.current['aria-describedby']).not.toBe(result2.current['aria-describedby']); + }); + + it('should keep the same id and update text content when description changes for the same key', () => { + let {result, rerender} = renderHook( + ({description}: {description?: string}) => useDynamicDescription(description, 'dynamic-5'), + {initialProps: {description: 'Test description 1'}} + ); + + let id = result.current['aria-describedby']; + let node = document.getElementById(id!); + expect(node?.textContent).toBe('Test description 1'); + + act(() => { + rerender({description: 'Test description 2'}); + }); + + expect(result.current['aria-describedby']).toBe(id); + expect(document.getElementById(id!)).toBe(node); + expect(node?.textContent).toBe('Test description 2'); + }); + + it('should keep the same id for the lifetime of the component', () => { + let {result, rerender} = renderHook( + ({description}: {description?: string}) => useDynamicDescription(description, 'dynamic-6'), + {initialProps: {description: 'Test description'}} + ); + + let id = result.current['aria-describedby']; + let node = document.getElementById(id!); + + act(() => { + rerender({description: ''}); + }); + + expect(result.current['aria-describedby']).toBeUndefined(); + expect(document.getElementById(id!)).toBe(node); + expect(node?.textContent).toBe(''); + + act(() => { + rerender({description: 'Updated description'}); + }); + + expect(result.current['aria-describedby']).toBe(id); + expect(document.getElementById(id!)).toBe(node); + expect(node?.textContent).toBe('Updated description'); + }); + + it('should not clean up if other components are using the same description key', () => { + let {result: result1, unmount: unmount1} = renderHook(() => useDynamicDescription('Test description', 'shared-cleanup')); + let {unmount: unmount2} = renderHook(() => useDynamicDescription('Test description', 'shared-cleanup')); + let id = result1.current['aria-describedby']; + expect(document.getElementById(id!)).not.toBeNull(); + unmount1(); + expect(document.getElementById(id!)).not.toBeNull(); + unmount2(); + expect(document.getElementById(id!)).toBeNull(); + }); + + it('should clean up description node on unmount', () => { + let {result, unmount} = renderHook(() => useDynamicDescription('Test description', 'dynamic-7')); + let id = result.current['aria-describedby']; + expect(document.getElementById(id!)).not.toBeNull(); + unmount(); + expect(document.getElementById(id!)).toBeNull(); + }); +}); From f0f8bce6613b8adb0ff6fa5a676131ec3f009855 Mon Sep 17 00:00:00 2001 From: Reid Barber Date: Wed, 4 Mar 2026 14:53:44 -0600 Subject: [PATCH 2/2] don't create nodes for initial empty descriptions --- packages/@react-aria/dnd/test/dnd.test.js | 2 +- .../@react-aria/utils/src/useDescription.ts | 53 ++++++++++++++----- .../utils/test/useDescription.test.tsx | 2 +- 3 files changed, 41 insertions(+), 16 deletions(-) diff --git a/packages/@react-aria/dnd/test/dnd.test.js b/packages/@react-aria/dnd/test/dnd.test.js index d285e8b59e7..f070673ec82 100644 --- a/packages/@react-aria/dnd/test/dnd.test.js +++ b/packages/@react-aria/dnd/test/dnd.test.js @@ -2543,7 +2543,7 @@ describe('useDrag and useDrop', function () { expect(document.getElementById(restoredStartDescriptionId)).toHaveTextContent('Click to start dragging'); expect(droppable).not.toHaveAttribute('aria-describedby'); expect(document.getElementById(dropDescriptionId)).toBe(dropDescriptionNode); - expect(dropDescriptionNode).toHaveTextContent(''); + expect(dropDescriptionNode).toHaveTextContent('Click to drop.'); fireEvent.click(draggable); act(() => jest.runAllTimers()); diff --git a/packages/@react-aria/utils/src/useDescription.ts b/packages/@react-aria/utils/src/useDescription.ts index 692896eebf1..a7df9082207 100644 --- a/packages/@react-aria/utils/src/useDescription.ts +++ b/packages/@react-aria/utils/src/useDescription.ts @@ -80,32 +80,57 @@ export function useDescription(description?: string): AriaLabelingProps { */ export function useDynamicDescription(description: string | undefined, descriptionKey: string): AriaLabelingProps { let [id, setId] = useState(); + let subscriptionRef = useRef<{key: string, desc: {refCount: number, element: HTMLElement}} | null>(null); let nodeRef = useRef(null); useLayoutEffect(() => { - let desc = getOrCreateDynamicDescriptionNode(descriptionKey); - desc.refCount++; - nodeRef.current = desc.element; - setId(desc.element.id); - - return () => { - if (--desc.refCount === 0) { - desc.element.remove(); - dynamicDescriptionNodes.delete(descriptionKey); + let subscription = subscriptionRef.current; + if (subscription && subscription.key !== descriptionKey) { + if (--subscription.desc.refCount === 0) { + subscription.desc.element.remove(); + dynamicDescriptionNodes.delete(subscription.key); } - if (nodeRef.current === desc.element) { + subscriptionRef.current = null; + if (nodeRef.current === subscription.desc.element) { nodeRef.current = null; } - }; - }, [descriptionKey]); + subscription = null; + } + + if (!subscription && description) { + let desc = getOrCreateDynamicDescriptionNode(descriptionKey); + desc.refCount++; + subscriptionRef.current = {key: descriptionKey, desc}; + nodeRef.current = desc.element; + setId(desc.element.id); + desc.element.textContent = description; + } + }, [descriptionKey, description]); useLayoutEffect(() => { - if (nodeRef.current) { - nodeRef.current.textContent = description || ''; + if (description && nodeRef.current) { + nodeRef.current.textContent = description; } }, [description]); + useLayoutEffect(() => { + return () => { + let subscription = subscriptionRef.current; + if (subscription) { + if (--subscription.desc.refCount === 0) { + subscription.desc.element.remove(); + dynamicDescriptionNodes.delete(subscription.key); + } + + subscriptionRef.current = null; + if (nodeRef.current === subscription.desc.element) { + nodeRef.current = null; + } + } + }; + }, []); + return { 'aria-describedby': description ? id : undefined }; diff --git a/packages/@react-aria/utils/test/useDescription.test.tsx b/packages/@react-aria/utils/test/useDescription.test.tsx index 890fd2c250e..1477200a7fc 100644 --- a/packages/@react-aria/utils/test/useDescription.test.tsx +++ b/packages/@react-aria/utils/test/useDescription.test.tsx @@ -113,7 +113,7 @@ describe('useDynamicDescription', () => { expect(result.current['aria-describedby']).toBeUndefined(); expect(document.getElementById(id!)).toBe(node); - expect(node?.textContent).toBe(''); + expect(node?.textContent).toBe('Test description'); act(() => { rerender({description: 'Updated description'});