Skip to content

Commit ae5a43b

Browse files
petersendidityannickcr
authored andcommitted
Fix jsx-no-bind reporting errors on render functions that don't return JSX (fixes #663)
1 parent ae27866 commit ae5a43b

File tree

3 files changed

+93
-9
lines changed

3 files changed

+93
-9
lines changed

lib/rules/jsx-no-bind.js

+9-6
Original file line numberDiff line numberDiff line change
@@ -5,13 +5,14 @@
55
*/
66
'use strict';
77

8+
var Components = require('../util/Components');
89
var propName = require('jsx-ast-utils/propName');
910

1011
// -----------------------------------------------------------------------------
1112
// Rule Definition
1213
// -----------------------------------------------------------------------------
1314

14-
module.exports = function(context) {
15+
module.exports = Components.detect(function(context, components, utils) {
1516
var configuration = context.options[0] || {};
1617

1718
return {
@@ -30,10 +31,12 @@ module.exports = function(context) {
3031
(ancestors[i].type === 'MethodDefinition' && ancestors[i].key.name === 'render') ||
3132
(ancestors[i].type === 'Property' && ancestors[i].key.name === 'render')
3233
) {
33-
context.report({
34-
node: callee,
35-
message: 'JSX props should not use .bind()'
36-
});
34+
if (utils.isReturningJSX(ancestors[i])) {
35+
context.report({
36+
node: callee,
37+
message: 'JSX props should not use .bind()'
38+
});
39+
}
3740
break;
3841
}
3942
}
@@ -74,7 +77,7 @@ module.exports = function(context) {
7477
}
7578
}
7679
};
77-
};
80+
});
7881

7982
module.exports.schema = [{
8083
type: 'object',

lib/util/Components.js

+26-3
Original file line numberDiff line numberDiff line change
@@ -199,12 +199,13 @@ function componentRule(rule, context) {
199199
/**
200200
* Check if the node is returning JSX
201201
*
202-
* @param {ASTNode} node The AST node being checked (can be a ReturnStatement or an ArrowFunctionExpression).
202+
* @param {ASTNode} ASTnode The AST node being checked
203203
* @param {Boolean} strict If true, in a ternary condition the node must return JSX in both cases
204204
* @returns {Boolean} True if the node is returning JSX, false if not
205205
*/
206-
isReturningJSX: function(node, strict) {
206+
isReturningJSX: function(ASTnode, strict) {
207207
var property;
208+
var node = ASTnode;
208209
switch (node.type) {
209210
case 'ReturnStatement':
210211
property = 'argument';
@@ -213,7 +214,11 @@ function componentRule(rule, context) {
213214
property = 'body';
214215
break;
215216
default:
216-
return false;
217+
node = utils.findReturnStatement(node);
218+
if (!node) {
219+
return false;
220+
}
221+
property = 'argument';
217222
}
218223

219224
var returnsConditionalJSXConsequent =
@@ -248,6 +253,24 @@ function componentRule(rule, context) {
248253
);
249254
},
250255

256+
/**
257+
* Find a return statment in the current node
258+
*
259+
* @param {ASTNode} ASTnode The AST node being checked
260+
*/
261+
findReturnStatement: function(node) {
262+
if (!node.value || !node.value.body) {
263+
return false;
264+
}
265+
var i = node.value.body.body.length - 1;
266+
for (; i >= 0; i--) {
267+
if (node.value.body.body[i].type === 'ReturnStatement') {
268+
return node.value.body.body[i];
269+
}
270+
}
271+
return false;
272+
},
273+
251274
/**
252275
* Get the parent component node from the current scope
253276
*

tests/lib/rules/jsx-no-bind.js

+58
Original file line numberDiff line numberDiff line change
@@ -72,6 +72,40 @@ ruleTester.run('jsx-no-bind', rule, {
7272
].join('\n'),
7373
options: [{allowBind: true}],
7474
parser: 'babel-eslint'
75+
},
76+
// Backbone view with a bind
77+
{
78+
code: [
79+
'var DocumentRow = Backbone.View.extend({',
80+
' tagName: "li",',
81+
' render: function() {',
82+
' this.onTap.bind(this);',
83+
' }',
84+
'});'
85+
].join('\n'),
86+
parser: 'babel-eslint'
87+
},
88+
{
89+
code: [
90+
'const foo = {',
91+
' render: function() {',
92+
' this.onTap.bind(this);',
93+
' return true;',
94+
' }',
95+
'};'
96+
].join('\n'),
97+
parser: 'babel-eslint'
98+
},
99+
{
100+
code: [
101+
'const foo = {',
102+
' render() {',
103+
' this.onTap.bind(this);',
104+
' return true;',
105+
' }',
106+
'};'
107+
].join('\n'),
108+
parser: 'babel-eslint'
75109
}
76110
],
77111

@@ -121,6 +155,30 @@ ruleTester.run('jsx-no-bind', rule, {
121155
errors: [{message: 'JSX props should not use .bind()'}],
122156
parser: 'babel-eslint'
123157
},
158+
{
159+
code: [
160+
'const foo = {',
161+
' render: function() {',
162+
' const click = this.onTap.bind(this);',
163+
' return <div onClick={onClick}>Hello</div>;',
164+
' }',
165+
'};'
166+
].join('\n'),
167+
errors: [{message: 'JSX props should not use .bind()'}],
168+
parser: 'babel-eslint'
169+
},
170+
{
171+
code: [
172+
'const foo = {',
173+
' render() {',
174+
' const click = this.onTap.bind(this);',
175+
' return <div onClick={onClick}>Hello</div>;',
176+
' }',
177+
'};'
178+
].join('\n'),
179+
errors: [{message: 'JSX props should not use .bind()'}],
180+
parser: 'babel-eslint'
181+
},
124182

125183
// Arrow functions
126184
{

0 commit comments

Comments
 (0)