-
Notifications
You must be signed in to change notification settings - Fork 1.8k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
fix(react): use useId
instead of getInstanceId
#16988
fix(react): use useId
instead of getInstanceId
#16988
Conversation
✅ Deploy Preview for carbon-elements ready!
To edit notification comments on pull requests, go to your Netlify site configuration. |
✅ Deploy Preview for v11-carbon-react ready!
To edit notification comments on pull requests, go to your Netlify site configuration. |
useId
instead of getInstanceId
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Interesting catch! LGTM! 🚀
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Clean !!
dff81f8
Brought up in slack, there's quite a few components that use
setupGetInstanceId()
where they could/should be usinguseId()
because:useId
in React 18+ is unified between server and clientuseId
has internal state to guard the id from changing based on rerenders of the parent componentThe specific issue that highlighted this was that Modal's unique id changes every time it re-renders (example). To avoid this you currently have to move state downwards (example), which isn't possible in all cases. Elsewhere in the codebase setupGetInstanceId was already guarded from changing on re-renders by using
useMemo
oruseRef
.There are still two class-based components that cannot take advantage of this, OverflowMenu (the old implementation, not the feature flagged one) and DataTable.
Changelog
Changed
useId
instead ofsetupGetInstanceId
Testing / Reviewing
For each component:
Example of a failure (in prod right now)
2024-07-17.at.13.31.34-Components.Modal.-.Playground.Storybook-Google.Chrome-converted.mp4
Example with this PR
2024-07-17.at.14.52.58-Components.Modal.-.Playground.Storybook-Google.Chrome-converted.mp4