Skip to content

Commit

Permalink
Fix destructured props detection in stateless components (fixes #326)
Browse files Browse the repository at this point in the history
  • Loading branch information
yannickcr committed Nov 28, 2015
1 parent 1bd8c89 commit bccf86b
Show file tree
Hide file tree
Showing 2 changed files with 30 additions and 6 deletions.
25 changes: 19 additions & 6 deletions lib/rules/prop-types.js
Original file line number Diff line number Diff line change
Expand Up @@ -393,14 +393,22 @@ module.exports = Components.detect(function(context, components, utils) {
break;
case 'VariableDeclarator':
for (var i = 0, j = node.id.properties.length; i < j; i++) {
if (
(node.id.properties[i].key.name !== 'props' && node.id.properties[i].key.value !== 'props') ||
node.id.properties[i].value.type !== 'ObjectPattern'
) {
// let {props: {firstname}} = this
var thisDestructuring = (
(node.id.properties[i].key.name === 'props' || node.id.properties[i].key.value === 'props') &&
node.id.properties[i].value.type === 'ObjectPattern'
);
// let {firstname} = props
var statelessDestructuring = node.init.name === 'props' && utils.getParentStatelessComponent();

if (thisDestructuring) {
properties = node.id.properties[i].value.properties;
} else if (statelessDestructuring) {
properties = node.id.properties;
} else {
continue;
}
type = 'destructuring';
properties = node.id.properties[i].value.properties;
break;
}
break;
Expand Down Expand Up @@ -550,7 +558,12 @@ module.exports = Components.detect(function(context, components, utils) {
},

VariableDeclarator: function(node) {
if (!node.init || node.init.type !== 'ThisExpression' || node.id.type !== 'ObjectPattern') {
// let {props: {firstname}} = this
var thisDestructuring = node.init && node.init.type === 'ThisExpression' && node.id.type === 'ObjectPattern';
// let {firstname} = props
var statelessDestructuring = node.init && node.init.name === 'props' && utils.getParentStatelessComponent();

This comment has been minimized.

Copy link
@ianobermiller

ianobermiller Dec 4, 2015

This addition broke the Radium build by causing false positives on this line:

https://github.com/FormidableLabs/radium/blob/21bf3a9dad906dc2ec903713380246da25762a96/src/resolve-styles.js#L180

let newProps = props;

This is detected as being inside a stateless component (even though it is just a function), and because the condition never checks that it is a destructuring assignment, it fails.


if (!thisDestructuring && !statelessDestructuring) {
return;
}
markPropTypesAsUsed(node);
Expand Down
11 changes: 11 additions & 0 deletions tests/lib/rules/prop-types.js
Original file line number Diff line number Diff line change
Expand Up @@ -1376,6 +1376,17 @@ ruleTester.run('prop-types', rule, {
errors: [{
message: '\'name\' is missing in props validation'
}]
}, {
code: [
'var Hello = (props) => {',
' const {name} = props;',
' return <div>Hello {name}</div>;',
'}'
].join('\n'),
parser: 'babel-eslint',
errors: [{
message: '\'name\' is missing in props validation'
}]
}, {
code: [
'class Hello extends React.Component {',
Expand Down

0 comments on commit bccf86b

Please sign in to comment.