Skip to content
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

Add boilerplate to compose refs #13782

Closed
pronebird opened this issue Oct 5, 2018 · 3 comments
Closed

Add boilerplate to compose refs #13782

pronebird opened this issue Oct 5, 2018 · 3 comments

Comments

@pronebird
Copy link

pronebird commented Oct 5, 2018

Do you want to request a feature or report a bug?
feature

What is the current behavior?

Currently there is no way to compose refs. I.e I have a situation when I hook up a ref from multiple components via context, because one of the components needs to calculate the layout and the other ref is passed by the consumer.

Here is a little snapshot:

export const MegaComponent = React.forwardRef(function MegaComponent (props, ref) {
  return (<Context.Consumer>{
      (context) => (
        <PrivateMegaComponent {...context} ref={(value) => {
          // Runtime implementation detail
          // Seriously, I just happen to know it's RefObject.
          context.ref.current = value;

          if (ref) {
            // Always good to dive into how runtime works
            if (typeof ref === 'function') {
              ref(value)
            } else if (typeof ref === 'object') {
              ref.current = value;
            }
          }
        }}>
          {props.children}
        </PrivateMegaComponent >
      )}
    </Context.Consumer>
  );
});

What is the expected behavior?

I expect that React would provide a way to compose refs so that consumer wouldn't have to dive into runtime implementation details and reimplement what React already does.

@aweary
Copy link
Contributor

aweary commented Oct 9, 2018

@pronebird checkout #13029, specifically @gaearon's comment. This is the recommend approach in this case, though this pattern shouldn't be used frequently.

@azizhk
Copy link

azizhk commented Jan 25, 2019

Hi @aweary,
I don't get the reason why this pattern shouldn't be used frequently.
If it breaks encapsulation then there should be a boilerplate function exposed to compose refs.

@pronebird
Copy link
Author

pronebird commented Jan 29, 2019

@aweary while the comment you've mentioned is a solution, there is still a problem with it.

When using TypeScript the RefObject.current is considered readonly for the consumer, which makes sense, however it makes it impossible to mutate it.

Folks make their own homegrown mergeRefs and basically break the type-safety in order to make it work. If the library had a sanctioned way of doing this it would be easier for everyone.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants