Skip to content

Commit

Permalink
Ignore functions with the this keyword during component detection
Browse files Browse the repository at this point in the history
  • Loading branch information
yannickcr committed Nov 5, 2015
1 parent 170c5f2 commit a09ac73
Show file tree
Hide file tree
Showing 4 changed files with 66 additions and 20 deletions.
48 changes: 30 additions & 18 deletions lib/util/Components.js
Original file line number Diff line number Diff line change
Expand Up @@ -21,16 +21,22 @@ function Components() {
* Add a node to the components list, or update it if it's already in the list
*
* @param {ASTNode} node The AST node being added.
* @param {Object} props Additional properties to add to the component.
* @param {Number} confidence Confidence in the component detection (0=banned, 1=maybe, 2=yes)
*/
Components.prototype.add = function(node, props) {
Components.prototype.add = function(node, confidence) {
var id = this._getId(node);
if (this._list[id]) {
this._list[id] = util._extend(this._list[id], props);
if (confidence === 0 || this._list[id].confidence === 0) {
this._list[id].confidence = 0;
} else {
this._list[id].confidence = Math.max(this._list[id].confidence, confidence);
}
return;
}
props.node = node;
this._list[id] = props;
this._list[id] = {
node: node,
confidence: confidence
};
};

/**
Expand Down Expand Up @@ -70,7 +76,7 @@ Components.prototype.set = function(node, props) {
Components.prototype.list = function() {
var list = {};
for (var i in this._list) {
if (!this._list.hasOwnProperty(i) || !this._list[i].confident) {
if (!this._list.hasOwnProperty(i) || this._list[i].confidence < 2) {
continue;
}
list[i] = this._list[i];
Expand All @@ -87,7 +93,7 @@ Components.prototype.list = function() {
Components.prototype.length = function() {
var length = 0;
for (var i in this._list) {
if (!this._list.hasOwnProperty(i) || !this._list[i].confident) {
if (!this._list.hasOwnProperty(i) || this._list[i].confidence < 2) {
continue;
}
length++;
Expand Down Expand Up @@ -309,40 +315,38 @@ function componentRule(rule, context) {
if (!context.react.isES6Component(node)) {
return;
}
components.add(node, {confident: true});
components.add(node, 2);
},

ClassProperty: function(node) {
node = context.react.getParentComponent();
if (!node) {
return;
}
components.add(node, {confident: true});
components.add(node, 2);
},

ObjectExpression: function(node) {
if (!context.react.isES5Component(node)) {
return;
}
components.add(node, {confident: true});
components.add(node, 2);
},

FunctionExpression: function(node) {
node = context.react.getParentComponent();
if (!node) {
return;
}
var component = components.get(node);
components.add(node, {confident: component && component.confident || false});
components.add(node, 1);
},

FunctionDeclaration: function(node) {
node = context.react.getParentComponent();
if (!node) {
return;
}
var component = components.get(node);
components.add(node, {confident: component && component.confident || false});
components.add(node, 1);
},

ArrowFunctionExpression: function(node) {
Expand All @@ -351,11 +355,19 @@ function componentRule(rule, context) {
return;
}
if (node.expression && context.react.isReturningJSX(node)) {
components.add(node, {confident: true});
components.add(node, 2);
} else {
var component = components.get(node);
components.add(node, {confident: component && component.confident || false});
components.add(node, 1);
}
},

ThisExpression: function(node) {
node = context.react.getParentComponent();
if (!node || !/Function/.test(node.type)) {
return;
}
// Ban functions with a ThisExpression
components.add(node, 0);
},

ReturnStatement: function(node) {
Expand All @@ -366,7 +378,7 @@ function componentRule(rule, context) {
if (!node) {
return;
}
components.add(node, {confident: true});
components.add(node, 2);
}
};

Expand Down
14 changes: 12 additions & 2 deletions tests/lib/rules/display-name.js
Original file line number Diff line number Diff line change
Expand Up @@ -308,6 +308,16 @@ ruleTester.run('display-name', rule, {
options: [{
acceptTranspilerName: true
}]
}, {
code: [
'export default {',
' renderHello() {',
' let {name} = this.props;',
' return <div>{name}</div>;',
' }',
'};'
].join('\n'),
parser: 'babel-eslint'
}
],

Expand Down Expand Up @@ -378,7 +388,7 @@ ruleTester.run('display-name', rule, {
}, {
code: [
'module.exports = () => {',
' return <div>Hello {this.props.name}</div>;',
' return <div>Hello {props.name}</div>;',
'}'
].join('\n'),
parser: 'babel-eslint',
Expand All @@ -391,7 +401,7 @@ ruleTester.run('display-name', rule, {
}, {
code: [
'module.exports = function() {',
' return <div>Hello {this.props.name}</div>;',
' return <div>Hello {props.name}</div>;',
'}'
].join('\n'),
parser: 'babel-eslint',
Expand Down
14 changes: 14 additions & 0 deletions tests/lib/rules/no-multi-comp.js
Original file line number Diff line number Diff line change
Expand Up @@ -62,6 +62,20 @@ ruleTester.run('no-multi-comp', rule, {
classes: true,
jsx: true
}
}, {
code: [
'export default {',
' renderHello() {',
' let {name} = this.props;',
' return <div>{name}</div>;',
' },',
' renderHello2() {',
' let {name2} = this.props;',
' return <div>{name2}</div>;',
' }',
'};'
].join('\n'),
parser: 'babel-eslint'
}],

invalid: [{
Expand Down
10 changes: 10 additions & 0 deletions tests/lib/rules/prop-types.js
Original file line number Diff line number Diff line change
Expand Up @@ -883,6 +883,16 @@ ruleTester.run('prop-types', rule, {
'};'
].join('\n'),
parser: 'babel-eslint'
}, {
code: [
'export default {',
' renderHello() {',
' let {name} = this.props;',
' return <div>{name}</div>;',
' }',
'};'
].join('\n'),
parser: 'babel-eslint'
}
],

Expand Down

0 comments on commit a09ac73

Please sign in to comment.