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

Receiving/passing ref #65

Open
klimashkin opened this issue May 17, 2017 · 7 comments
Open

Receiving/passing ref #65

klimashkin opened this issue May 17, 2017 · 7 comments

Comments

@klimashkin
Copy link

Hi! To continue our conversation at #46

In 2.1.0 you added mapThemrProps with example:

function mapThemrProps(props, theme) {
  const { composeTheme, innerRef, mapThemrProps, themeNamespace, ...rest } = props;
  return { ...rest, withRef: innerRef, theme };
}

But withRef is usually boolean, for example in react-redux, so it doesn't work.

It was case, when @themr hoc wraps another hoc. But what if vice-versa, I have places where it's wrapped by @connect.

It used to be simple convention: In hoc's ref handler function it checked if underline instance also has getWrappedInstance and used it to take ref from it, so it's a chain when you can reach real component no matter how many hocs do you have on top of it

    saveComponentRef(instance) {
      this.instance = instance && instance.getWrappedInstance ? instance.getWrappedInstance() : instance;
    }

But now having somewhere withRef, somewhere innerRef or mapThemrProps I'm confused.
Feels like it has added complexity and problems instead of solving anything

@klimashkin
Copy link
Author

Moreover, if I have hundred components with themr and redux, I need to write everywhere the same thing

function mapThemrProps(props, theme) {
  const { composeTheme, innerRef, mapThemrProps, themeNamespace, ...rest } = props;
  return { ...rest, withRef: innerRef, theme };
}

That adds absolutely redundant data transformation on each render just to get underlying ref.

@raveclassic
Copy link
Contributor

@klimashkin Unfortunately I don't have enough practice with innerRef/withRef because, well, I just don't use it. But anyway, why can't we just pass actual ref prop through all the hocs? Does React somehow violate accessing it?
I mean what was the point of entrodusing withRef/innerRef prop? Maybe @javivelasco can shed some light on it?

On the other hand, when you assume that underline instance has a getWrappedInstance method you rely on inner implementation, don't you?

Still after looking into the source there's a prop innerRef which accepts a function that is passed as an actual ref prop to decorated component. Can't you use it to pass the ref?

@klimashkin
Copy link
Author

klimashkin commented May 17, 2017

@raveclassic

why can't we just pass actual ref prop through all the hocs?

Because of React design - child component never receive it, react uses it to pass component instance of child element up to parent.

On the other hand, when you assume that underline instance has a getWrappedInstance method you rely on inner implementation, don't you?

And with innerRef the same - parent should know that child component wrapped in hoc with innerRef, but statically - on react element creation time. In case of getWrappedInstance we can check if child component is a hoc or not dynamically, inside save ref handler. That's why it's very handy for another hocs, they simply can't know about it statically without introducing new option like passInnerRef that I described here #46 (comment)

Still after looking into the source there's a prop innerRef which accepts a function that is passed as an actual ref prop to decorated component. Can't you use it to pass the ref?

Read about ref and this thread one more time : )

@raveclassic
Copy link
Contributor

@klimashkin I see. Do you suggest reverting replacement of withRef with innerRef or do you have some more elegant solution?

@klimashkin
Copy link
Author

klimashkin commented May 17, 2017

That's why I asked @javivelasco at #46 (comment) about introducing to react community new convention with two properties: innerRef and passInnerRef.

I'm pretty happy with current withRef and getWrappedInstance convention - it does job well.
And because of new innerRef and mapThemrProps I can't update react-css-themr to 2.x version in my project , because it breaks hoc structure in many components. And because I can't update it I'm getting react-props warnings now. So I'm stuck in the middle : )

@raveclassic
Copy link
Contributor

I don't believe that introduction of a new convention looks like a solution but still I agree that innerRef could be reverted back to withRef at least to be similar to other existing hocs.

@klimashkin
Copy link
Author

klimashkin commented Mar 14, 2018

The new ref forwarding api is coming to react

https://github.com/bvaughn/rfcs/blob/ref-forwarding/text/0000-ref-forwarding.md
image

https://reactjs.org/docs/forwarding-refs.html

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

2 participants