Skip to content

Commit

Permalink
Add no-find-dom-node rule (fixes #678)
Browse files Browse the repository at this point in the history
  • Loading branch information
yannickcr committed Jul 19, 2016
1 parent 30b267b commit 55b9cbe
Show file tree
Hide file tree
Showing 5 changed files with 185 additions and 1 deletion.
2 changes: 2 additions & 0 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -87,6 +87,7 @@ Finally, enable all of the rules that you would like to use. Use [our preset](#
* [react/no-did-mount-set-state](docs/rules/no-did-mount-set-state.md): Prevent usage of `setState` in `componentDidMount`
* [react/no-did-update-set-state](docs/rules/no-did-update-set-state.md): Prevent usage of `setState` in `componentDidUpdate`
* [react/no-direct-mutation-state](docs/rules/no-direct-mutation-state.md): Prevent direct mutation of `this.state`
* [react/no-find-dom-node](docs/rules/no-find-dom-node.md): Prevent usage of `findDOMNode`
* [react/no-is-mounted](docs/rules/no-is-mounted.md): Prevent usage of `isMounted`
* [react/no-multi-comp](docs/rules/no-multi-comp.md): Prevent multiple component definition per file
* [react/no-render-return-value](docs/rules/no-render-return-value.md): Prevent usage of the return value of `React.render`
Expand Down Expand Up @@ -162,6 +163,7 @@ The rules enabled in this configuration are:
* [react/no-did-mount-set-state](docs/rules/no-did-mount-set-state.md) with `allow-in-func` option
* [react/no-did-update-set-state](docs/rules/no-did-update-set-state.md) with `allow-in-func` option
* [react/no-direct-mutation-state](docs/rules/no-direct-mutation-state.md)
* [react/no-find-dom-node](docs/rules/no-find-dom-node.md)
* [react/no-is-mounted](docs/rules/no-is-mounted.md)
* [react/no-unknown-property](docs/rules/no-unknown-property.md)
* [react/prop-types](docs/rules/prop-types.md)
Expand Down
33 changes: 33 additions & 0 deletions docs/rules/no-find-dom-node.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,33 @@
# Prevent usage of findDOMNode (no-find-dom-node)

Facebook will eventually deprecate `findDOMNode` as it blocks certain improvements in React in the future.

It is recommended to use callback refs instead. See [Dan Abramov comments and examples](https://github.com/yannickcr/eslint-plugin-react/issues/678#issue-165177220).

## Rule Details

The following patterns are considered warnings:

```js
class MyComponent extends Component {
componentDidMount() {
findDOMNode(this).scrollIntoView();
}
render() {
return <div />
}
}
```

The following patterns are not considered warnings:

```js
class MyComponent extends Component {
componentDidMount() {
this.node.scrollIntoView();
}
render() {
return <div ref={node => this.node = node} />
}
}
```
4 changes: 3 additions & 1 deletion index.js
Original file line number Diff line number Diff line change
Expand Up @@ -47,7 +47,8 @@ module.exports = {
'jsx-first-prop-new-line': require('./lib/rules/jsx-first-prop-new-line'),
'jsx-no-target-blank': require('./lib/rules/jsx-no-target-blank'),
'jsx-filename-extension': require('./lib/rules/jsx-filename-extension'),
'require-optimization': require('./lib/rules/require-optimization')
'require-optimization': require('./lib/rules/require-optimization'),
'no-find-dom-node': require('./lib/rules/no-find-dom-node')
},
configs: {
recommended: {
Expand All @@ -67,6 +68,7 @@ module.exports = {
'react/no-did-mount-set-state': [2, 'allow-in-func'],
'react/no-did-update-set-state': [2, 'allow-in-func'],
'react/no-direct-mutation-state': 2,
'react/no-find-dom-node': 2,
'react/no-is-mounted': 2,
'react/no-unknown-property': 2,
'react/no-render-return-value': 2,
Expand Down
44 changes: 44 additions & 0 deletions lib/rules/no-find-dom-node.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,44 @@
/**
* @fileoverview Prevent usage of findDOMNode
* @author Yannick Croissant
*/
'use strict';

// ------------------------------------------------------------------------------
// Rule Definition
// ------------------------------------------------------------------------------

module.exports = function(context) {

// --------------------------------------------------------------------------
// Public
// --------------------------------------------------------------------------

return {

CallExpression: function(node) {
var callee = node.callee;
if (
callee.type !== 'MemberExpression' || (

This comment has been minimized.

Copy link
@pfhayes

pfhayes Jul 19, 2016

Contributor

It looks like this rule can get triggered when callee.type == 'MemberExpression' but text != findDOMNode. is that correct?

when I run the latest eslint on my project I get thousands of incorrect findDOMNode errors (in files where findDOMNode isn't being used at all). this appears to be the cause

This comment has been minimized.

Copy link
@yannickcr

yannickcr Jul 21, 2016

Author Member

Right, my condition is wrong. I'll patch it.

Thanks for catching this up before the release 👍

callee.object.callee && callee.object.callee.name !== 'findDOMNode' &&
callee.property.name !== 'findDOMNode'
)
) {
return;
}
var ancestors = context.getAncestors(callee);
for (var i = 0, j = ancestors.length; i < j; i++) {
if (ancestors[i].type === 'Property' || ancestors[i].type === 'MethodDefinition') {
context.report({
node: callee,
message: 'Do not use findDOMNode'
});
break;
}
}
}
};

};

module.exports.schema = [];
103 changes: 103 additions & 0 deletions tests/lib/rules/no-find-dom-node.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,103 @@
/**
* @fileoverview Prevent usage of findDOMNode
* @author Yannick Croissant
*/
'use strict';

// ------------------------------------------------------------------------------
// Requirements
// ------------------------------------------------------------------------------

var rule = require('../../../lib/rules/no-find-dom-node');
var RuleTester = require('eslint').RuleTester;

var parserOptions = {
ecmaVersion: 6,
ecmaFeatures: {
jsx: true
}
};

// ------------------------------------------------------------------------------
// Tests
// ------------------------------------------------------------------------------

var ruleTester = new RuleTester();
ruleTester.run('no-find-dom-node', rule, {

valid: [{
code: [
'var Hello = function() {};'
].join('\n'),
parserOptions: parserOptions
}, {
code: [
'var Hello = React.createClass({',
' render: function() {',
' return <div>Hello</div>;',
' }',
'});'
].join('\n'),
parserOptions: parserOptions
}, {
code: [
'var Hello = React.createClass({',
' componentDidMount: function() {',
' someNonMemberFunction(arg);',
' this.someFunc = React.findDOMNode;',
' },',
' render: function() {',
' return <div>Hello</div>;',
' }',
'});'
].join('\n'),
parserOptions: parserOptions
}],

invalid: [{
code: [
'var Hello = React.createClass({',
' componentDidMount: function() {',
' React.findDOMNode(this).scrollIntoView();',
' },',
' render: function() {',
' return <div>Hello</div>;',
' }',
'});'
].join('\n'),
parserOptions: parserOptions,
errors: [{
message: 'Do not use findDOMNode'
}]
}, {
code: [
'var Hello = React.createClass({',
' componentDidMount: function() {',
' ReactDOM.findDOMNode(this).scrollIntoView();',
' },',
' render: function() {',
' return <div>Hello</div>;',
' }',
'});'
].join('\n'),
parserOptions: parserOptions,
errors: [{
message: 'Do not use findDOMNode'
}]
}, {
code: [
'class Hello extends Component {',
' componentDidMount() {',
' findDOMNode(this).scrollIntoView();',
' }',
' render() {',
' return <div>Hello</div>;',
' }',
'};'
].join('\n'),
parserOptions: parserOptions,
errors: [{
message: 'Do not use findDOMNode'
}]
}]
});

0 comments on commit 55b9cbe

Please sign in to comment.