Replace findDOMNode by ref callback#98
Conversation
|
|
||
| saveSidebarWidth() { | ||
| const width = ReactDOM.findDOMNode(this.refs.sidebar).offsetWidth; | ||
| const width = this.sidebar.offsetWidth; |
There was a problem hiding this comment.
'const' is available in ES6 (use 'esversion: 6') or Mozilla JS extensions (use moz).
| <div {...rootProps}> | ||
| <div className={this.props.sidebarClassName} style={sidebarStyle} ref="sidebar"> | ||
| <div className={this.props.sidebarClassName} style={sidebarStyle} | ||
| ref={node => (this.sidebar = node)}> |
There was a problem hiding this comment.
We should cache this function on the instance to avoid recreating this method on each invocation of render
|
|
||
| saveSidebarWidth() { | ||
| const width = ReactDOM.findDOMNode(this.refs.sidebar).offsetWidth; | ||
| const width = this.refs.sidebar.offsetWidth; |
There was a problem hiding this comment.
'const' is available in ES6 (use 'esversion: 6') or Mozilla JS extensions (use moz).
|
@balloob I made it even simpler. |
|
String refs are considered a legacy API and support will be removed to the future. The function was the right choice https://facebook.github.io/react/docs/refs-and-the-dom.html#legacy-api-string-refs |
| this.onScroll = this.onScroll.bind(this); | ||
| } | ||
|
|
||
| saveSidebarRef = (node) => this.sidebar = node; |
There was a problem hiding this comment.
Class properties must be methods. Expected '(' but instead saw '='.
'function closure expressions' is only available in Mozilla JavaScript extensions (use moz option).
Expected an identifier and instead saw '=>'.
Class properties must be methods. Expected '(' but instead saw '.'.
|
Nice! Ok to merge when ESLint fixed (see build failure) |
|
Sorry for the linting, I edited strait on github :) |
|
Thanks! |
3 participants
Hi @balloob,
This PR is to remove the call to
findDOMNode, because the use of this function is discouraged.Instead I used a ref callback (available since react@0.13: https://twitter.com/dan_abramov/status/584125325212307456?lang=en).
(My initial need is for snapshot testing with jest: facebook/react#8989)
Thanks!