-
Notifications
You must be signed in to change notification settings - Fork 81
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: Update TextInput ref forwarding #3011
fix: Update TextInput ref forwarding #3011
Conversation
a20a2fd
to
8a6b3e5
Compare
|
||
TextInput.displayName = 'TextInput' |
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.
A bit weird, but the existing pattern for making sure the components have a display name is done like so:
const DayForwardRef: React.ForwardRefRenderFunction< export const Day = forwardRef(DayForwardRef)
I don't remember if there was a reason we did it that way, or if your approach is better, so I won't block on it
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.
I am happy to change but I find it a bit strange - this approach makes it so that the display name of the component (shows up in dev tools, error messages) will be DayForwardRef
not Day
which I find somewhat unideal.
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.
I agree with you. Let's leave it as is. Can always update easily if we find any problems or want to unify one way or the other
Could you add some context in the Summary for what bug this addresses? |
Yea this might have been a relic from much older versions of react, since I think this was likely one of the first components built for ReactUSWDS |
@all-contributors please add ocoutts-usds for bug, code |
Could not find the user |
@all-contributors please add @ocoutts-usds for bug,code |
I've put up a pull request to add @ocoutts-usds! 🎉 |
Summary
Currently this component forwards refs in a way that does not work in new versions of react and I'm not sure if it ever worked in any version of react. Leaving the old way in case somebody is using it.