fix(floating-ui): prevent setState-during-commit crash in React 19 - #23128
Conversation
✅ Deploy Preview for v12-carbon-web-components ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
✅ Deploy Preview for v12-carbon-react ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
|
All contributors have signed the DCO. |
✅ Deploy Preview for v11-carbon-web-components ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
✅ Deploy Preview for v11-carbon-react ready!Built without sensitive environment variables
To edit notification comments on pull requests, go to your Netlify project configuration. |
✅ Deploy Preview for carbon-elements ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
|
I have read the DCO document and I hereby sign the DCO. |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #23128 +/- ##
========================================
Coverage 89.67% 89.67%
========================================
Files 639 639
Lines 58879 59007 +128
Branches 8300 8286 -14
========================================
+ Hits 52797 52917 +120
- Misses 5916 5924 +8
Partials 166 166
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
| if (pendingFloatingNodeRef.current !== null) { | ||
| const node = pendingFloatingNodeRef.current; | ||
| pendingFloatingNodeRef.current = null; | ||
| refs.setFloating(node); | ||
| } |
There was a problem hiding this comment.
The null callback from React is the detach signal, but the !== null guard drops it. When the floating element unmounts or floating styles are disabled, refs.setFloating(null) never runs, so Floating UI keeps a reference to a removed element. Should the pending null value be preserved and forwarded?
This comment applies elsewhere too.
| // React 19: refs.setFloating / refs.setReference are useState setters. | ||
| // Passing them as ref callbacks causes setState during commit → crash. | ||
| // Capture the node in a ref and forward it in a passive effect (after commit). | ||
| const pendingFloatingNodeRef = useRef(null); |
There was a problem hiding this comment.
Are all eight copies of the React 19 explanation necessary? Could the ref forwarding logic and rationale live in a shared hook, making the component call sites less repetitive?
| useEffect(() => { | ||
| if (pendingFloatingNodeRef.current !== null) { | ||
| const node = pendingFloatingNodeRef.current; | ||
| pendingFloatingNodeRef.current = null; | ||
| refs.setFloating(node); | ||
| } | ||
| }); | ||
|
|
||
| const pendingReferenceNodeRef = useRef(null); | ||
| const setReferenceSafe = useCallback((node) => { | ||
| pendingReferenceNodeRef.current = node; | ||
| }, []); | ||
| useEffect(() => { | ||
| if (pendingReferenceNodeRef.current !== null) { | ||
| const node = pendingReferenceNodeRef.current; | ||
| pendingReferenceNodeRef.current = null; | ||
| refs.setReference(node); | ||
| } | ||
| }); |
There was a problem hiding this comment.
This workaround changes ref timing and detach behavior, but no tests were added. Could you add test coverage?
2 participants
Closes #
React 19 calls ref callbacks synchronously during the commit phase. The
refs.setFloatingandrefs.setReferencefunctions returned byuseFloating()from@floating-ui/reactare backed byuseStatesetters.Passing them directly as DOM ref props caused a setState-during-commit cycle,
exceeding React 19's nested update limit and crashing with
"Maximum update depth exceeded".
The fix captures the DOM node in a plain ref inside a stable
useCallbackref callback, then forwards it to the floating-ui setter inside a
useEffect(which runs after commit, safely outside the synchronous commit phase).
Changelog
Fixed
MenuButton,OverflowMenu(next),ComboButton,ComboBox,Dropdown,MultiSelect,FilterableMultiSelect, andMenuItemwhen
@floating-ui/reactref callbacks were used directly as DOM ref props.Testing / Reviewing
with "Maximum update depth exceeded"; they should now render correctly
yarn jest --testPathPatterns "MenuButton|OverflowMenu|ComboButton|ComboBox|Dropdown|MultiSelect|MenuItem"— all 352 tests pass
PR Checklist
As the author of this PR, before marking ready for review, confirm you:
Updated documentation and storybook examples— no public API or visual changeFollowed the required v12 migration documentation— this fix is not v12-specific behaviour, it is a React 19 compatibility patch with no consumer migration burden