react-spectrum

fix: Memory leak suspense safe useId

#7757

Merged

Closes #7738

This PR makes useId safe to use with Suspense. As stated in the issue, using useId inside a Suspense boundary in concurrent mode can result in a memory leak. This is because we write to a Map during render so that we can track values during the render phase.
Because we do it during render though, and mount/unmount are never called on the suspended components, we never have the opportunity to clean up.

React does eventually clean up Suspended components depending on waves hands criteria.

We can take advantage of that by adding an object we know will be stable for the entire lifetime of that component to a FinalizationRegistry. When that component is cleaned up and the gc runs, we will get a callback such that we can remove the key from the Map.

One thing to note in the docs for FinalizationRegistry, is there is a caveat, this function may never be called. However, I believe this applies to the life time of the entire application. I think it doesn't mean that the memory can be reclaimed without calling it. Since we don't care when the values are cleaned up, I think this is an ok caveat.

I've checked our other components. Nowhere else appears to have this behaviour. We do have a couple minor memory leaks with our Formatter caches, however, those should be negligible and we'd need to write our own GC to handle those. I've left them alone for now.

In the future, we should use WeakMap whenever possible for these situations.

For now, to avoid a breaking change, we have to use the FinalizationRegistry.

Tests are omitted because getting node to GC on demand is very fiddly. I've left a story in storybook under useId.

✅ Pull Request Checklist:

• Included link to corresponding React Spectrum GitHub Issue.
• Added/updated unit tests and storybook for this change (for new code or code which already has tests).
• Filled out test instructions.
• Updated documentation (if it already exists for this component).
• Looked at the Accessibility Practices for this feature - Aria Practices

📝 Test Instructions:

🧢 Your Project:

Build successful! 🎉

• View the storybook
• View the storybook-19
• View the storybook-17
• View the storybook-16
• View the S2 storybook
• View the documentation

Build successful! 🎉

• View the storybook
• View the storybook-19
• View the storybook-17
• View the storybook-16
• View the S2 storybook
• View the documentation

Test Findings:
In my testing, memory still isn't being properly released. The cached useCallback appears to retain fiber references, preventing garbage collection and consequently keeping cleanupRef in memory indefinitely.

Working Solution:
Changing to this structure successfully triggers GC:

let idRefsMap: Map<string, { current: string | null }[]> = new Map();

cache useCallback

cache ref

Thanks for double checking that, I'll update that soon.

Build successful! 🎉

• View the storybook
• View the storybook-19
• View the storybook-17
• View the storybook-16
• View the S2 storybook
• View the documentation

Build successful! 🎉

• View the storybook
• View the storybook-19
• View the storybook-17
• View the storybook-16
• View the S2 storybook
• View the documentation

Compared to your videos, looks like this is behaving correctly now. Thanks again for checking me and tracking down the issue, apologies for not picking up on your comment earlier.

Build successful! 🎉

• View the storybook
• View the storybook-19
• View the storybook-17
• View the storybook-16
• View the S2 storybook
• View the documentation

Unfortunately breaks SSR on certain serverless environments, see #7882.