Repository: yannickcr/eslint-plugin-react (js)

Issue:
Allow jsx-no-bind to ignore dom components
In my point of view the `jsx-no-bind` rule is mostly for `PureComponent`, although there is consideration on GC performance, it should not be significant

In many cases, we need to pass some arguments to an event handler, for example: 

```js
<li key={name} className={className} onClick={() => this.onClickTag(name, selected)}>{text}</li>
```

This will trigger the `jsx-no-bind` rule, here we have 2 concerns:

1. If we are using a custom component in above code, we can pass `name` and `selected` to component's `props` and call `this.props.onClick(this.props.name, this.props.selected)` to prevent such function bindings, however we cannot do this with DOM components
2. DOM components are never `PureComponent`, so a large reason for `jsx-no-bind` rule will not be a benefit for DOM components

For these reasons, I expect the `jsx-no-bind` can have a `ignoreDOMComponents` config to ignore function bindings on DOM components

Pull request:
Add ignoreDOMComponents option to jsx-no-bind
Resolves #1238

Patch:
diff --git a/docs/rules/jsx-no-bind.md b/docs/rules/jsx-no-bind.md
index 686d1366..fdeef54b 100644
--- a/docs/rules/jsx-no-bind.md
+++ b/docs/rules/jsx-no-bind.md
@@ -7,21 +7,22 @@ A `bind` call or [arrow function](https://developer.mozilla.org/en-US/docs/Web/J
 The following patterns are considered warnings:
 
 ```jsx
-<div onClick={this._handleClick.bind(this)}></div>
+<Foo onClick={this._handleClick.bind(this)}></Foo>
 ```
 ```jsx
-<div onClick={() => console.log('Hello!')}></div>
+<Foo onClick={() => console.log('Hello!')}></Foo>
 ```
 
 The following patterns are **not** considered warnings:
 ```jsx
-<div onClick={this._handleClick}></div>
+<Foo onClick={this._handleClick}></Foo>
 ```
 
 ## Rule Options
 
 ```js
 "react/jsx-no-bind": [<enabled>, {
+  "ignoreDOMComponents": <boolean> || false,
   "ignoreRefs": <boolean> || false,
   "allowArrowFunctions": <boolean> || false,
   "allowFunctions": <boolean> || false,
@@ -29,13 +30,23 @@ The following patterns are **not** considered warnings:
 }]
 ```
 
+### `ignoreDOMComponents`
+
+When `true` the following are **not** considered warnings:
+
+```jsx
+<div onClick={this._handleClick.bind(this) />
+<span onClick={() => console.log("Hello!")} />
+<button onClick={function() { alert("1337") }} />
+```
+
 ### `ignoreRefs`
 
 When `true` the following are **not** considered warnings:
 
 ```jsx
-<div ref={c => this._div = c} />
-<div ref={this._refCallback.bind(this)} />
+<Foo ref={c => this._div = c} />
+<Foo ref={this._refCallback.bind(this)} />
 ```
 
 ### `allowArrowFunctions`
@@ -43,7 +54,7 @@ When `true` the following are **not** considered warnings:
 When `true` the following is **not** considered a warning:
 
 ```jsx
-<div onClick={() => alert("1337")} />
+<Foo onClick={() => alert("1337")} />
 ```
 
 ### `allowFunctions`
@@ -51,7 +62,7 @@ When `true` the following is **not** considered a warning:
 When `true` the following is not considered a warning:
 
 ```jsx
-<div onClick={function () { alert("1337") }} />
+<Foo onClick={function () { alert("1337") }} />
 ```
 
 ### `allowBind`
@@ -59,7 +70,7 @@ When `true` the following is not considered a warning:
 When `true` the following is **not** considered a warning:
 
 ```jsx
-<div onClick={this._handleClick.bind(this)} />
+<Foo onClick={this._handleClick.bind(this)} />
 ```
 
 ## Protips
diff --git a/lib/rules/jsx-no-bind.js b/lib/rules/jsx-no-bind.js
index def107f5..f6e87b22 100644
--- a/lib/rules/jsx-no-bind.js
+++ b/lib/rules/jsx-no-bind.js
@@ -9,6 +9,7 @@
 const propName = require('jsx-ast-utils/propName');
 const Components = require('../util/Components');
 const docsUrl = require('../util/docsUrl');
+const jsxUtil = require('../util/jsx');
 
 // -----------------------------------------------------------------------------
 // Rule Definition
@@ -48,6 +49,10 @@ module.exports = {
         ignoreRefs: {
           default: false,
           type: 'boolean'
+        },
+        ignoreDOMComponents: {
+          default: false,
+          type: 'boolean'
         }
       },
       additionalProperties: false
@@ -165,6 +170,10 @@ module.exports = {
         if (isRef || !node.value || !node.value.expression) {
           return;
         }
+        const isDOMComponent = jsxUtil.isDOMComponent(node.parent);
+        if (configuration.ignoreDOMComponents && isDOMComponent) {
+          return;
+        }
         const valueNode = node.value.expression;
         const valueNodeType = valueNode.type;
         const nodeViolationType = getNodeViolationType(valueNode);
diff --git a/lib/rules/jsx-no-undef.js b/lib/rules/jsx-no-undef.js
index c177bf34..fc8a3bd2 100644
--- a/lib/rules/jsx-no-undef.js
+++ b/lib/rules/jsx-no-undef.js
@@ -6,16 +6,7 @@
 'use strict';
 
 const docsUrl = require('../util/docsUrl');
-
-/**
- * Checks if a node name match the JSX tag convention.
- * @param {String} name - Name of the node to check.
- * @returns {boolean} Whether or not the node name match the JSX tag convention.
- */
-const tagConvention = /^[a-z]|\-/;
-function isTagName(name) {
-  return tagConvention.test(name);
-}
+const jsxUtil = require('../util/jsx');
 
 // ------------------------------------------------------------------------------
 // Rule Definition
@@ -95,10 +86,10 @@ module.exports = {
       JSXOpeningElement: function(node) {
         switch (node.name.type) {
           case 'JSXIdentifier':
-            node = node.name;
-            if (isTagName(node.name)) {
+            if (jsxUtil.isDOMComponent(node)) {
               return;
             }
+            node = node.name;
             break;
           case 'JSXMemberExpression':
             node = node.name;
diff --git a/lib/rules/jsx-pascal-case.js b/lib/rules/jsx-pascal-case.js
index 12d5fecf..e7629c57 100644
--- a/lib/rules/jsx-pascal-case.js
+++ b/lib/rules/jsx-pascal-case.js
@@ -7,13 +7,13 @@
 
 const elementType = require('jsx-ast-utils/elementType');
 const docsUrl = require('../util/docsUrl');
+const jsxUtil = require('../util/jsx');
 
 // ------------------------------------------------------------------------------
 // Constants
 // ------------------------------------------------------------------------------
 
 const PASCAL_CASE_REGEX = /^([A-Z0-9]|[A-Z0-9]+[a-z0-9]+(?:[A-Z0-9]+[a-z0-9]*)*)$/;
-const COMPAT_TAG_REGEX = /^[a-z]|\-/;
 const ALL_CAPS_TAG_REGEX = /^[A-Z0-9]+$/;
 
 // ------------------------------------------------------------------------------
@@ -60,7 +60,7 @@ module.exports = {
         }
 
         const isPascalCase = PASCAL_CASE_REGEX.test(name);
-        const isCompatTag = COMPAT_TAG_REGEX.test(name);
+        const isCompatTag = jsxUtil.isDOMComponent(node);
         const isAllowedAllCaps = allowAllCaps && ALL_CAPS_TAG_REGEX.test(name);
         const isIgnored = ignore.indexOf(name) !== -1;
 
diff --git a/lib/rules/jsx-sort-props.js b/lib/rules/jsx-sort-props.js
index dcabaa45..99aba6b0 100644
--- a/lib/rules/jsx-sort-props.js
+++ b/lib/rules/jsx-sort-props.js
@@ -4,9 +4,9 @@
  */
 'use strict';
 
-const elementType = require('jsx-ast-utils/elementType');
 const propName = require('jsx-ast-utils/propName');
 const docsUrl = require('../util/docsUrl');
+const jsxUtil = require('../util/jsx');
 
 // ------------------------------------------------------------------------------
 // Rule Definition
@@ -16,20 +16,6 @@ function isCallbackPropName(name) {
   return /^on[A-Z]/.test(name);
 }
 
-const COMPAT_TAG_REGEX = /^[a-z]|\-/;
-function isDOMComponent(node) {
-  let name = elementType(node);
-
-  // Get namespace if the type is JSXNamespacedName or JSXMemberExpression
-  if (name.indexOf(':') > -1) {
-    name = name.substring(0, name.indexOf(':'));
-  } else if (name.indexOf('.') > -1) {
-    name = name.substring(0, name.indexOf('.'));
-  }
-
-  return COMPAT_TAG_REGEX.test(name);
-}
-
 const RESERVED_PROPS_LIST = [
   'children',
   'dangerouslySetInnerHTML',
@@ -229,7 +215,7 @@ module.exports = {
     return {
       JSXOpeningElement: function(node) {
         // `dangerouslySetInnerHTML` is only "reserved" on DOM components
-        if (reservedFirst && !isDOMComponent(node)) {
+        if (reservedFirst && !jsxUtil.isDOMComponent(node)) {
           reservedList = reservedList.filter(prop => prop !== 'dangerouslySetInnerHTML');
         }
 
diff --git a/lib/rules/no-danger.js b/lib/rules/no-danger.js
index 5d40d923..8fca2904 100644
--- a/lib/rules/no-danger.js
+++ b/lib/rules/no-danger.js
@@ -5,6 +5,7 @@
 'use strict';
 
 const docsUrl = require('../util/docsUrl');
+const jsxUtil = require('../util/jsx');
 
 // ------------------------------------------------------------------------------
 // Constants
@@ -25,16 +26,6 @@ const DANGEROUS_PROPERTIES = DANGEROUS_PROPERTY_NAMES.reduce((props, prop) => {
 // Helpers
 // ------------------------------------------------------------------------------
 
-/**
- * Checks if a node name match the JSX tag convention.
- * @param {String} name - Name of the node to check.
- * @returns {boolean} Whether or not the node name match the JSX tag convention.
- */
-const tagConvention = /^[a-z]|\-/;
-function isTagName(name) {
-  return tagConvention.test(name);
-}
-
 /**
  * Checks if a JSX attribute is dangerous.
  * @param {String} name - Name of the attribute to check.
@@ -63,7 +54,7 @@ module.exports = {
     return {
 
       JSXAttribute: function(node) {
-        if (isTagName(node.parent.name.name) && isDangerous(node.name.name)) {
+        if (jsxUtil.isDOMComponent(node.parent) && isDangerous(node.name.name)) {
           context.report({
             node: node,
             message: DANGEROUS_MESSAGE,
diff --git a/lib/rules/self-closing-comp.js b/lib/rules/self-closing-comp.js
index e7ea52f5..31c81867 100644
--- a/lib/rules/self-closing-comp.js
+++ b/lib/rules/self-closing-comp.js
@@ -5,6 +5,7 @@
 'use strict';
 
 const docsUrl = require('../util/docsUrl');
+const jsxUtil = require('../util/jsx');
 
 // ------------------------------------------------------------------------------
 // Rule Definition
@@ -39,13 +40,8 @@ module.exports = {
   },
 
   create: function(context) {
-    const tagConvention = /^[a-z]|\-/;
-    function isTagName(name) {
-      return tagConvention.test(name);
-    }
-
     function isComponent(node) {
-      return node.name && node.name.type === 'JSXIdentifier' && !isTagName(node.name.name);
+      return node.name && node.name.type === 'JSXIdentifier' && !jsxUtil.isDOMComponent(node);
     }
 
     function hasChildren(node) {
@@ -63,7 +59,7 @@ module.exports = {
       const configuration = Object.assign({}, optionDefaults, context.options[0]);
       return (
         configuration.component && isComponent(node) ||
-        configuration.html && isTagName(node.name.name)
+        configuration.html && jsxUtil.isDOMComponent(node)
       ) && !node.selfClosing && !hasChildren(node);
     }
 
diff --git a/lib/util/jsx.js b/lib/util/jsx.js
new file mode 100644
index 00000000..e6c3e007
--- /dev/null
+++ b/lib/util/jsx.js
@@ -0,0 +1,30 @@
+/**
+ * @fileoverview Utility functions for JSX
+ */
+'use strict';
+
+const elementType = require('jsx-ast-utils/elementType');
+
+const COMPAT_TAG_REGEX = /^[a-z]|\-/;
+
+/**
+ * Checks if a node represents a DOM element.
+ * @param {String} node - JSXOpeningElement to check.
+ * @returns {boolean} Whether or not the node corresponds to a DOM element.
+ */
+function isDOMComponent(node) {
+  let name = elementType(node);
+
+  // Get namespace if the type is JSXNamespacedName or JSXMemberExpression
+  if (name.indexOf(':') > -1) {
+    name = name.slice(0, name.indexOf(':'));
+  } else if (name.indexOf('.') > -1) {
+    name = name.slice(0, name.indexOf('.'));
+  }
+
+  return COMPAT_TAG_REGEX.test(name);
+}
+
+module.exports = {
+  isDOMComponent: isDOMComponent
+};


Test patch:
diff --git a/tests/lib/rules/jsx-no-bind.js b/tests/lib/rules/jsx-no-bind.js
index 89312b06..73c3d77e 100644
--- a/tests/lib/rules/jsx-no-bind.js
+++ b/tests/lib/rules/jsx-no-bind.js
@@ -268,6 +268,25 @@ ruleTester.run('jsx-no-bind', rule, {
         '  }',
         '}'
       ].join('\n')
+    },
+
+    // ignore DOM components
+    {
+      code: '<div onClick={this._handleClick.bind(this)}></div>',
+      options: [{ignoreDOMComponents: true}]
+    },
+    {
+      code: '<div onClick={() => alert("1337")}></div>',
+      options: [{ignoreDOMComponents: true}]
+    },
+    {
+      code: '<div onClick={function () { alert("1337") }}></div>',
+      options: [{ignoreDOMComponents: true}]
+    },
+    {
+      code: '<div foo={::this.onChange} />',
+      options: [{ignoreDOMComponents: true}],
+      parser: 'babel-eslint'
     }
   ],
 
@@ -772,6 +791,13 @@ ruleTester.run('jsx-no-bind', rule, {
       ].join('\n'),
       errors: [{message: 'JSX props should not use ::'}],
       parser: 'babel-eslint'
+    },
+
+    // ignore DOM components
+    {
+      code: '<Foo onClick={this._handleClick.bind(this)} />',
+      options: [{ignoreDOMComponents: true}],
+      errors: [{message: 'JSX props should not use .bind()'}]
     }
   ]
 });