Skip to content

[jsx-no-bind] Only warn for props and account for variable declaration #1459

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

Merged
Merged
9 changes: 9 additions & 0 deletions docs/rules/jsx-no-bind.md
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@ The following patterns are not considered warnings:
"react/jsx-no-bind": [<enabled>, {
"ignoreRefs": <boolean> || false,
"allowArrowFunctions": <boolean> || false,
"allowFunctions": <boolean> || false,
"allowBind": <boolean> || false
}]
```
Expand All @@ -45,6 +46,14 @@ When `true` the following is not considered a warning:
<div onClick={() => alert("1337")} />
```

### `allowFunctions`

When `true` the following is not considered a warning:

```jsx
<div onClick={function () { alert("1337") }} />
```

### `allowBind`

When `true` the following is not considered a warning:
Expand Down
158 changes: 110 additions & 48 deletions lib/rules/jsx-no-bind.js
Original file line number Diff line number Diff line change
@@ -1,7 +1,8 @@
/**
* @fileoverview Prevents usage of Function.prototype.bind and arrow functions
* in React component definition.
* in React component props.
* @author Daniel Lo Nigro <dan.cx>
* @author Jacky Ho
*/
'use strict';

Expand All @@ -12,10 +13,17 @@ const propName = require('jsx-ast-utils/propName');
// Rule Definition
// -----------------------------------------------------------------------------

const violationMessageStore = {
bindCall: 'JSX props should not use .bind()',
arrowFunc: 'JSX props should not use arrow functions',
bindExpression: 'JSX props should not use ::',
func: 'JSX props should not use functions'
};

module.exports = {
meta: {
docs: {
description: 'Prevents usage of Function.prototype.bind and arrow functions in React component definition',
description: 'Prevents usage of Function.prototype.bind and arrow functions in React component props',
category: 'Best Practices',
recommended: false
},
Expand All @@ -31,6 +39,10 @@ module.exports = {
default: false,
type: 'boolean'
},
allowFunctions: {
default: false,
type: 'boolean'
},
ignoreRefs: {
default: false,
type: 'boolean'
Expand All @@ -40,33 +52,100 @@ module.exports = {
}]
},

create: Components.detect((context, components, utils) => {
create: Components.detect(context => {
const configuration = context.options[0] || {};

// Keep track of all the variable names pointing to a bind call,
// bind expression or an arrow function in different block statements
const blockVariableNameSets = {};

function setBlockVariableNameSet(blockStart) {
blockVariableNameSets[blockStart] = {
arrowFunc: new Set(),
bindCall: new Set(),
bindExpression: new Set(),
func: new Set()
};
}

function getNodeViolationType(node) {
const nodeType = node.type;

if (
!configuration.allowBind &&
nodeType === 'CallExpression' &&
node.callee.type === 'MemberExpression' &&
node.callee.property.type === 'Identifier' &&
node.callee.property.name === 'bind'
) {
return 'bindCall';
} else if (
!configuration.allowArrowFunctions &&
nodeType === 'ArrowFunctionExpression'
) {
return 'arrowFunc';
} else if (
!configuration.allowFunctions &&
nodeType === 'FunctionExpression'
) {
return 'func';
} else if (
!configuration.allowBind &&
nodeType === 'BindExpression'
) {
return 'bindExpression';
}

return null;
}

function addVariableNameToSet(violationType, variableName, blockStart) {
blockVariableNameSets[blockStart][violationType].add(variableName);
}

function getBlockStatementAncestors(node) {
return context.getAncestors(node).reverse().filter(
ancestor => ancestor.type === 'BlockStatement'
);
}

function reportVariableViolation(node, name, blockStart) {
const blockSets = blockVariableNameSets[blockStart];
const violationTypes = Object.keys(blockSets);

return violationTypes.find(type => {
if (blockSets[type].has(name)) {
context.report({node: node, message: violationMessageStore[type]});
return true;
}

return false;
});
}

function findVariableViolation(node, name) {
getBlockStatementAncestors(node).find(
block => reportVariableViolation(node, name, block.start)
);
}

return {
CallExpression: function(node) {
const callee = node.callee;
BlockStatement(node) {
setBlockVariableNameSet(node.start);
},

VariableDeclarator(node) {
const blockAncestors = getBlockStatementAncestors(node);
const variableViolationType = getNodeViolationType(node.init);

if (
!configuration.allowBind &&
(callee.type !== 'MemberExpression' || callee.property.name !== 'bind')
blockAncestors.length > 0 &&
variableViolationType &&
node.parent.kind === 'const' // only support const right now
) {
return;
}
const ancestors = context.getAncestors(callee).reverse();
Copy link
Contributor Author

@jackyho112 jackyho112 Oct 15, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this block of code reports any violation in a component's render method. This rule is supposed to only report violations in JSX props, so this is not needed.

for (let i = 0, j = ancestors.length; i < j; i++) {
if (
!configuration.allowBind &&
(ancestors[i].type === 'MethodDefinition' && ancestors[i].key.name === 'render') ||
(ancestors[i].type === 'Property' && ancestors[i].key.name === 'render')
) {
if (utils.isReturningJSX(ancestors[i])) {
context.report({
node: callee,
message: 'JSX props should not use .bind()'
});
}
break;
}
addVariableNameToSet(
variableViolationType, node.id.name, blockAncestors[0].start
);
}
},

Expand All @@ -76,31 +155,14 @@ module.exports = {
return;
}
const valueNode = node.value.expression;
if (
!configuration.allowBind &&
valueNode.type === 'CallExpression' &&
valueNode.callee.type === 'MemberExpression' &&
valueNode.callee.property.name === 'bind'
) {
context.report({
node: node,
message: 'JSX props should not use .bind()'
});
} else if (
!configuration.allowArrowFunctions &&
valueNode.type === 'ArrowFunctionExpression'
) {
context.report({
node: node,
message: 'JSX props should not use arrow functions'
});
} else if (
!configuration.allowBind &&
valueNode.type === 'BindExpression'
) {
const valueNodeType = valueNode.type;
const nodeViolationType = getNodeViolationType(valueNode);

if (valueNodeType === 'Identifier') {
findVariableViolation(node, valueNode.name);
} else if (nodeViolationType) {
context.report({
node: node,
message: 'JSX props should not use ::'
node: node, message: violationMessageStore[nodeViolationType]
});
}
}
Expand Down
Loading