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

How to access public interfaces for components (replacement for ref) #1356

Closed
dzearing opened this issue Mar 27, 2017 · 5 comments
Closed

How to access public interfaces for components (replacement for ref) #1356

dzearing opened this issue Mar 27, 2017 · 5 comments

Comments

@dzearing
Copy link
Member

dzearing commented Mar 27, 2017

Problem

Components like Buttons may have public methods exposed within an interface like IButton. These methods might include things like focus(). Users are expected to put a ref on their button, get the component reference, and call focus() on that. Unfortunately, this is easily broken when we use decorators or higher-order components, which wrap a component. When this occurs, using ref callbacks return the HOC wrapper rather than the actual component, which then causes bugs for customers. For example, recently someone added a @withResponsiveMode decorator to a DropDown, which prohibits accessing the focus method.

This has been a known limitation for a long time in React, and it seems there isn't a standard solution yet (such as a user defined getPublicInstance):
facebook/react#4213

Other libraries experience similar bugs:
airbnb/react-with-styles#53

Redux supports this with the connect HOC in a roundabout way. The connect method takes in options, including a withRef boolean. If true, it will track the wrapped component ref and expose it on the HOC via a public getWrappedInstance() method. This seems a bit redundant because as a consumer you still need to use refs to get a reference to the HOC, then call getWrappedInstance() to get the internal ref.

https://github.com/reactjs/react-redux/blob/master/docs/api.md

Proposed solution

I propose we standardize on a componentRef prop and handle it for all subclasses of BaseComponent. In BaseComponent, the default implementation would be in _updateComponentRef, which checks if componentRef is provided, and calls it with "this", similar how ref callbacks are executed. It would work as a replacement for ref.

This would allow subclasses of BaseComponent to override _updateComponentRef when necessary, to either no-op let the componentRef pass through to an inner component which could manage it. or to provide an alternative componentRef result.

So to the customer, they'd just use componentRef instead of ref, and it would reliably return the reference to the interface, even when wrapping them in decorators.

It would also let wrappers resolve the componentRef separately and ignore passing it into the inner component. This would let a variant provide an alternative interface.

Alternative solutions

We talked about hoisting non-react method proxies automatically in decorators, and in fact that happens currently in BaseDecorator. However it's not reliable and it's easy to break and gives very little control about what to do with componentRef.

@micahgodbolt
Copy link
Member

How does a consumer know when to us ref vs innerRef? They'd have to refer to the props, right?

Or else maybe always to use innerRef and we resolve that to ref on components that don't need inner?

@dzearing
Copy link
Member Author

We would always use innerRef as you suggested. I would just make BaseComponent resolve it for you so you don't need to implement it; you just need to expose it in your props for documentation and typing purposes.

@cschleiden
Copy link
Contributor

So instead of

<TextField value="42" ref={this._resolveTextField} />

I'd use

<TextField value="42" innerRef={this._resolveTextField} />

?

inner doesn't really speak to me, it feels like something is elevated to the consumer that shouldn't be (do I really care that TextField uses a HOC under the hood?). Could we call it something else, componentRef maybe?

<TextField value="42" componentRef={this._resolveTextField} />

Apart from that I agree, don't see a better solution.

@dzearing
Copy link
Member Author

dzearing commented Mar 28, 2017

Yeah that works for me. componentRef sounds better. Editing proposal.

@micahgodbolt
Copy link
Member

This appears to be resolved in #1389 so I'll close this issue up.

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

No branches or pull requests

3 participants