From e9526b6251bb1b7e957e37436ad079416bdf77ea Mon Sep 17 00:00:00 2001 From: Reid Barber Date: Fri, 27 Mar 2026 10:37:49 -0500 Subject: [PATCH 1/3] fix(useId): avoid FinalizationRegistry entry leak in useId --- packages/react-aria/src/utils/useId.ts | 11 ++- packages/react-aria/test/utils/useId.test.jsx | 74 +++++++++++++++++++ 2 files changed, 83 insertions(+), 2 deletions(-) create mode 100644 packages/react-aria/test/utils/useId.test.jsx diff --git a/packages/react-aria/src/utils/useId.ts b/packages/react-aria/src/utils/useId.ts index 693f3ba901e..a2fc5555b7d 100644 --- a/packages/react-aria/src/utils/useId.ts +++ b/packages/react-aria/src/utils/useId.ts @@ -32,6 +32,7 @@ if (typeof FinalizationRegistry !== 'undefined') { idsUpdaterMap.delete(heldValue); }); } +let registeredIds = new WeakMap(); /** * If a default is not provided, generate an id. @@ -44,8 +45,13 @@ export function useId(defaultId?: string): string { let res = useSSRSafeId(value); let cleanupRef = useRef(null); - if (registry) { - registry.register(cleanupRef, res); + let registeredId = registeredIds.get(cleanupRef); + if (registry && registeredId !== res) { + if (registeredId != null) { + registry.unregister(cleanupRef); + } + registry.register(cleanupRef, res, cleanupRef); + registeredIds.set(cleanupRef, res); } if (canUseDOM) { @@ -64,6 +70,7 @@ export function useId(defaultId?: string): string { // when it is though, also remove it from the finalization registry. if (registry) { registry.unregister(cleanupRef); + registeredIds.delete(cleanupRef); } idsUpdaterMap.delete(r); }; diff --git a/packages/react-aria/test/utils/useId.test.jsx b/packages/react-aria/test/utils/useId.test.jsx new file mode 100644 index 00000000000..09d77677081 --- /dev/null +++ b/packages/react-aria/test/utils/useId.test.jsx @@ -0,0 +1,74 @@ +/* + * Copyright 2026 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. + */ + +describe('useId', function () { + let OriginalFinalizationRegistry = global.FinalizationRegistry; + + afterEach(() => { + global.FinalizationRegistry = OriginalFinalizationRegistry; + jest.resetModules(); + }); + + it('registers once per mounted id and uses an unregister token', function () { + let register = jest.fn(); + let unregister = jest.fn(); + + global.FinalizationRegistry = jest.fn(function () { + this.register = register; + this.unregister = unregister; + }); + + jest.isolateModules(() => { + let React = require('react'); + let {createRoot} = require('react-dom/client'); + let {act} = require('react-dom/test-utils'); + let {useId} = require('../../src/utils/useId'); + let previousActEnvironment = global.IS_REACT_ACT_ENVIRONMENT; + global.IS_REACT_ACT_ENVIRONMENT = true; + + function Test({tick}) { + let id = useId(); + return React.createElement('div', {'data-id': id}, tick); + } + + let container = document.createElement('div'); + document.body.appendChild(container); + let root = createRoot(container); + + act(() => { + root.render(React.createElement(Test, {tick: 0})); + }); + + act(() => { + root.render(React.createElement(Test, {tick: 1})); + }); + + act(() => { + root.render(React.createElement(Test, {tick: 2})); + }); + + expect(register).toHaveBeenCalledTimes(1); + expect(register.mock.calls[0][1]).toEqual(expect.any(String)); + expect(register.mock.calls[0][2]).toBe(register.mock.calls[0][0]); + + act(() => { + root.unmount(); + }); + + document.body.removeChild(container); + global.IS_REACT_ACT_ENVIRONMENT = previousActEnvironment; + + expect(unregister).toHaveBeenCalledTimes(1); + expect(unregister).toHaveBeenCalledWith(register.mock.calls[0][2]); + }); + }); +}); From 400aff22c6ed31d02d96f1f63cccb39d0216372b Mon Sep 17 00:00:00 2001 From: Reid Barber Date: Fri, 27 Mar 2026 10:56:13 -0500 Subject: [PATCH 2/3] fix test in React 16/17 --- packages/react-aria/test/utils/useId.test.jsx | 28 +++++++++++++------ 1 file changed, 19 insertions(+), 9 deletions(-) diff --git a/packages/react-aria/test/utils/useId.test.jsx b/packages/react-aria/test/utils/useId.test.jsx index 09d77677081..f0811363f14 100644 --- a/packages/react-aria/test/utils/useId.test.jsx +++ b/packages/react-aria/test/utils/useId.test.jsx @@ -29,11 +29,10 @@ describe('useId', function () { jest.isolateModules(() => { let React = require('react'); - let {createRoot} = require('react-dom/client'); + let ReactDOM = require('react-dom'); let {act} = require('react-dom/test-utils'); let {useId} = require('../../src/utils/useId'); - let previousActEnvironment = global.IS_REACT_ACT_ENVIRONMENT; - global.IS_REACT_ACT_ENVIRONMENT = true; + let isReact18OrHigher = parseInt(React.version, 10) >= 18; function Test({tick}) { let id = useId(); @@ -42,18 +41,30 @@ describe('useId', function () { let container = document.createElement('div'); document.body.appendChild(container); - let root = createRoot(container); + + let renderElement; + let unmount; + if (isReact18OrHigher) { + let {createRoot} = require('react-dom/client'); + global.IS_REACT_ACT_ENVIRONMENT = true; + let root = createRoot(container); + renderElement = (el) => root.render(el); + unmount = () => root.unmount(); + } else { + renderElement = (el) => ReactDOM.render(el, container); + unmount = () => ReactDOM.unmountComponentAtNode(container); + } act(() => { - root.render(React.createElement(Test, {tick: 0})); + renderElement(React.createElement(Test, {tick: 0})); }); act(() => { - root.render(React.createElement(Test, {tick: 1})); + renderElement(React.createElement(Test, {tick: 1})); }); act(() => { - root.render(React.createElement(Test, {tick: 2})); + renderElement(React.createElement(Test, {tick: 2})); }); expect(register).toHaveBeenCalledTimes(1); @@ -61,11 +72,10 @@ describe('useId', function () { expect(register.mock.calls[0][2]).toBe(register.mock.calls[0][0]); act(() => { - root.unmount(); + unmount(); }); document.body.removeChild(container); - global.IS_REACT_ACT_ENVIRONMENT = previousActEnvironment; expect(unregister).toHaveBeenCalledTimes(1); expect(unregister).toHaveBeenCalledWith(register.mock.calls[0][2]); From 59706b7f2166cf933a1730066c318b42b05dacf9 Mon Sep 17 00:00:00 2001 From: Reid Barber Date: Fri, 27 Mar 2026 17:41:13 -0500 Subject: [PATCH 3/3] add comments to test --- packages/react-aria/test/utils/useId.test.jsx | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/packages/react-aria/test/utils/useId.test.jsx b/packages/react-aria/test/utils/useId.test.jsx index f0811363f14..c6f0942091c 100644 --- a/packages/react-aria/test/utils/useId.test.jsx +++ b/packages/react-aria/test/utils/useId.test.jsx @@ -42,6 +42,7 @@ describe('useId', function () { let container = document.createElement('div'); document.body.appendChild(container); + // Use createRoot for React 18+ and ReactDOM.render for older let renderElement; let unmount; if (isReact18OrHigher) { @@ -67,7 +68,10 @@ describe('useId', function () { renderElement(React.createElement(Test, {tick: 2})); }); + // Re-rendering the same mounted hook should not add more registry entries. expect(register).toHaveBeenCalledTimes(1); + // The held value should be the generated id string, and the unregister token should match + // the target object so useId can remove the same registration during cleanup. expect(register.mock.calls[0][1]).toEqual(expect.any(String)); expect(register.mock.calls[0][2]).toBe(register.mock.calls[0][0]); @@ -77,6 +81,7 @@ describe('useId', function () { document.body.removeChild(container); + // Unmount should remove the specific registration created for this hook instance. expect(unregister).toHaveBeenCalledTimes(1); expect(unregister).toHaveBeenCalledWith(register.mock.calls[0][2]); });