Skip to content
This repository has been archived by the owner on Dec 13, 2018. It is now read-only.

Suggestion: allow innerRef-prop to be passed to underlying component via option-flag #349

Closed
danielberndt opened this issue Oct 25, 2017 · 4 comments

Comments

@danielberndt
Copy link
Collaborator

danielberndt commented Oct 25, 2017

I'm working on a project with a big Ui File which exports a lot of components like Buttons, Boxes, Text-Components, etc.

I'd like those to behave as similarily as possible. I.e. I'd like them all to use innerRef to be able to access the underlying DOM-Node.

For wrapping plain html-tags (glamorous('div')({...})) this works as expected.
Now if I want to wrap a Component like a React-Router Link I can do const WrappedLink = glamorous(Link)({})

If I write <WrappedLink innerRef={...}/> innerRef gives me the reference to the Link Component. However I would like to do something like this:

const WrappedLink = glamorous(Link, {forwardProps: ["innerRef"]})({})

such that innerRef now is passed to the Link component yielding its underlying DOM-Node.

I believe that fixing this line could resolve this issue.

Do you believe it's worthwhile pursuing? If so I'm happy to do a PR :)

@marzelin
Copy link
Collaborator

To achieve your goal you can use findDOMNode() from react-dom package:

<WrappedLink innerRef={
    (ref) => el = ReactDOM.findDOMNode(ref)
}/>

Implementing your suggestion would require changes in this file as well (adding innerRef to forwarded props on specific condition).

@danielberndt
Copy link
Collaborator Author

Thanks for your suggestion!

However I don't really like relying on ReactDOM.findDOMNode. First it's not cross-plattform, and second, it's possible that innerRef and ReactDOM.findDOMNode might return different nodes* in which case I want to rely on the result of innerRef.

*Think of something like <Container><input ref={this.props.innerRef}/></Container>

@kentcdodds
Copy link
Contributor

Hi @danielberndt,
I don't see any reason to disallow opting-into forwarding innerRef as you have here. I'd be open to a pull request with the implementation and tests 👍

Thanks!

@Ailrun
Copy link
Contributor

Ailrun commented Oct 25, 2017

Nice! I really appreciate for your suggestion!

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

No branches or pull requests

4 participants