Skip to content

Commit

Permalink
[New] Add ignoreDOMComponents option to jsx-no-bind
Browse files Browse the repository at this point in the history
Resolves #1238
  • Loading branch information
alexzherdev authored and ljharb committed Jun 30, 2018
1 parent c3e57c7 commit 5709271
Show file tree
Hide file tree
Showing 9 changed files with 96 additions and 56 deletions.
27 changes: 19 additions & 8 deletions docs/rules/jsx-no-bind.md
Expand Up @@ -7,59 +7,70 @@ 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,
"allowBind": <boolean> || false
}]
```

### `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`
When `true` the following is **not** considered a warning:
```jsx
<div onClick={() => alert("1337")} />
<Foo onClick={() => alert("1337")} />
```
### `allowFunctions`
When `true` the following is not considered a warning:
```jsx
<div onClick={function () { alert("1337") }} />
<Foo onClick={function () { alert("1337") }} />
```
### `allowBind`
When `true` the following is **not** considered a warning:
```jsx
<div onClick={this._handleClick.bind(this)} />
<Foo onClick={this._handleClick.bind(this)} />
```
## Protips
Expand Down
9 changes: 9 additions & 0 deletions lib/rules/jsx-no-bind.js
Expand Up @@ -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
Expand Down Expand Up @@ -48,6 +49,10 @@ module.exports = {
ignoreRefs: {
default: false,
type: 'boolean'
},
ignoreDOMComponents: {
default: false,
type: 'boolean'
}
},
additionalProperties: false
Expand Down Expand Up @@ -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);
Expand Down
15 changes: 3 additions & 12 deletions lib/rules/jsx-no-undef.js
Expand Up @@ -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
Expand Down Expand Up @@ -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;
Expand Down
4 changes: 2 additions & 2 deletions lib/rules/jsx-pascal-case.js
Expand Up @@ -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]+$/;

// ------------------------------------------------------------------------------
Expand Down Expand Up @@ -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;

Expand Down
18 changes: 2 additions & 16 deletions lib/rules/jsx-sort-props.js
Expand Up @@ -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
Expand All @@ -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',
Expand Down Expand Up @@ -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');
}

Expand Down
13 changes: 2 additions & 11 deletions lib/rules/no-danger.js
Expand Up @@ -5,6 +5,7 @@
'use strict';

const docsUrl = require('../util/docsUrl');
const jsxUtil = require('../util/jsx');

// ------------------------------------------------------------------------------
// Constants
Expand All @@ -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.
Expand Down Expand Up @@ -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,
Expand Down
10 changes: 3 additions & 7 deletions lib/rules/self-closing-comp.js
Expand Up @@ -5,6 +5,7 @@
'use strict';

const docsUrl = require('../util/docsUrl');
const jsxUtil = require('../util/jsx');

// ------------------------------------------------------------------------------
// Rule Definition
Expand Down Expand Up @@ -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) {
Expand All @@ -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);
}

Expand Down
30 changes: 30 additions & 0 deletions 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
};
26 changes: 26 additions & 0 deletions tests/lib/rules/jsx-no-bind.js
Expand Up @@ -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'
}
],

Expand Down Expand Up @@ -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()'}]
}
]
});

0 comments on commit 5709271

Please sign in to comment.