-
-
Notifications
You must be signed in to change notification settings - Fork 10.3k
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 withRouter HOC stateful to allow refs #5382
Conversation
using ref as string is not the best way. better to pass a callback,
and lift this ref to the parent component in componentDidMount
```
class ChildComponent extends Component {
static propTypes = {
liftNode: PropTypes.func.isRequired
}
componentDidMount = () => {
this.props.liftNode(this.node)
}
getNode = node => { this.node = node }
render () {
return (
<div
ref={this.getNode}
/>
)
}
}
class HOC extends Component {
liftNode = node => { this.div = node }
render () {
< ChildComponent
liftNode={this.liftNode}
/>
}
}
```
you can even use React context API to lift node up the component tree.
2017-07-28 18:28 GMT+08:00 Nikita Lobachev <notifications@github.com>:
… Right now when you use withRouter on a component with ref it suddenly
breaks because withRouter is a stateless component which doesn't allow refs.
There is wrappedComponentRef property but it forces container to know
which child components use withRouter and which don't.
Components refs are mostly used to get a DOM node via ReactDOM.findDOMNode
so usually it doesn't matter if ref points to withRouter HOC instead of
wrapped component.
------------------------------
You can view, comment on, or merge this pull request online at:
#5382
Commit Summary
- Make withRouter HOC stateful to allow refs
File Changes
- *M* packages/react-router/modules/__tests__/withRouter-test.js
<https://github.com/ReactTraining/react-router/pull/5382/files#diff-0>
(20)
- *M* packages/react-router/modules/withRouter.js
<https://github.com/ReactTraining/react-router/pull/5382/files#diff-1>
(28)
Patch Links:
- https://github.com/ReactTraining/react-router/pull/5382.patch
- https://github.com/ReactTraining/react-router/pull/5382.diff
—
You are receiving this because you are subscribed to this thread.
Reply to this email directly, view it on GitHub
<#5382>, or mute the
thread
<https://github.com/notifications/unsubscribe-auth/ACJsecSOKZxfKLoWF09KT0WrpTSsHzAkks5sSbfggaJpZM4OmcA9>
.
|
The main point, is that ReactDOM.findDOMNode is very old API, which
definitely adds up the complexity to the bundle.
2017-07-28 18:46 GMT+08:00 Alexey Lyahov <justfly1984@gmail.com>:
… using ref as string is not the best way. better to pass a callback,
and lift this ref to the parent component in componentDidMount
```
class ChildComponent extends Component {
static propTypes = {
liftNode: PropTypes.func.isRequired
}
componentDidMount = () => {
this.props.liftNode(this.node)
}
getNode = node => { this.node = node }
render () {
return (
<div
ref={this.getNode}
/>
)
}
}
class HOC extends Component {
liftNode = node => { this.div = node }
render () {
< ChildComponent
liftNode={this.liftNode}
/>
}
}
```
you can even use React context API to lift node up the component tree.
2017-07-28 18:28 GMT+08:00 Nikita Lobachev ***@***.***>:
> Right now when you use withRouter on a component with ref it suddenly
> breaks because withRouter is a stateless component which doesn't allow refs.
>
> There is wrappedComponentRef property but it forces container to know
> which child components use withRouter and which don't.
>
> Components refs are mostly used to get a DOM node via
> ReactDOM.findDOMNode so usually it doesn't matter if ref points to
> withRouter HOC instead of wrapped component.
> ------------------------------
> You can view, comment on, or merge this pull request online at:
>
> #5382
> Commit Summary
>
> - Make withRouter HOC stateful to allow refs
>
> File Changes
>
> - *M* packages/react-router/modules/__tests__/withRouter-test.js
> <https://github.com/ReactTraining/react-router/pull/5382/files#diff-0>
> (20)
> - *M* packages/react-router/modules/withRouter.js
> <https://github.com/ReactTraining/react-router/pull/5382/files#diff-1>
> (28)
>
> Patch Links:
>
> - https://github.com/ReactTraining/react-router/pull/5382.patch
> - https://github.com/ReactTraining/react-router/pull/5382.diff
>
> —
> You are receiving this because you are subscribed to this thread.
> Reply to this email directly, view it on GitHub
> <#5382>, or mute the
> thread
> <https://github.com/notifications/unsubscribe-auth/ACJsecSOKZxfKLoWF09KT0WrpTSsHzAkks5sSbfggaJpZM4OmcA9>
> .
>
|
@JustFly1984 It affects both string and functional refs. Also findDOMNode isn't deprecated and useful for some edge cases or quick prototyping. |
It was explained in #4838 that we do this because it follows the recommendation from the React docs. The ref you would get back from this component wouldn't be the correct one, as it points at the wrapper instead of the wrapped component. |
I ended up writing this utility function:
Use like you would use the original I understand the future is |
Right now when you use withRouter on a component with ref it suddenly breaks because withRouter is a stateless component which doesn't allow refs.
There is
wrappedComponentRef
property but it forces container to know which child components use withRouter and which don't.Components refs are mostly used to get a DOM node via ReactDOM.findDOMNode so usually it doesn't matter if ref points to withRouter HOC instead of wrapped component.