-
Notifications
You must be signed in to change notification settings - Fork 22
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
Make ref
s point to "correct" elements
#169
Comments
ref
to point to "correct" elementsref
s point to "correct" elements
The draft PR ⬆️ solves the issue for In my opinion we could just forward refs for the components where they are being used |
I'm not sure I agree with this. Some doubts that maybe would also help others have a more informed opinion:
Maybe some benchmarking would also help us decide? How other component libraries deal with the same topic? |
I understand your concerns and agree that we lack some research to make an informed decision, I'll do the research and give an answer before the end of the week |
Hey, I haven't had a lot of time to take a deep look into this, but I wanted to give an answer before the end of the week to at least give some initial context for everyone new to the discussion, so here it goes. First I want to make a small clarification, in this discussion we are not talking about general usage of refs in Wave, we are talking specifically about forwarding refs, meaning exposing an underlying DOM element so that a project using a Wave component can easily interact with said element (e.g. we can forward a ref in the The imperative (
but even though we should use them sparingly, sometimes they are simply necessary, here are some still valid examples from the legacy React docs:
And to add a specific example from Wave take a look at this PR in the As for downsides of refs I haven't really found any other than the shift in paradigm, there is a Github issue in React with some benchmarks on With all this being said, I think we need two agreements related to forwarding refs: Which underlying DOM elements should we attach a ref to when we forward it?This one is actually easier to answer since we already had an "agreement" to always attach to the most obvious element (e.g. Which components should we forward refs on?It's entirely up to us, I don't have a strong opinion about this but I think ideally we should allow to pass refs only to elements that have behaviour that can't be invoked declaratively (e.g.
Of course this is simply my personal opinion and I'd really like to hear some others 🤗 |
I'll close this issue after merging #389, in that PR we are following our agreement on refs for the components that were already forwarding the ref ( |
The outcome of the dialog #144
Example code: https://codesandbox.io/s/wave-input-refs-c5ure
Currently, the
ref
is attached to the wrapper which contradicts our agreement to attach to the most obvious element. We need to attach the ref to theinput
and research for other use cases needed to be fixed.Components using
forwardRef
that need correction:Input
should have anHTMLInputElement
InnerInput
should have anHTMLInputElement
Password
should have anHTMLInputElement
Components using other components internally that have changed refs:
DatepickerRangeInput
usingInput
PhoneInput
usingInput
The text was updated successfully, but these errors were encountered: