Skip to content

Commit

Permalink
Teach prop-types about destructing in function signatures
Browse files Browse the repository at this point in the history
The following should cause an error for missing propType validations,
but it doesn't.

  const Test = ({ name }) => {
    return (
      <div>{name}</div>
    );
  };

If you replace the destructuring with using the props object, it works
as expected. This commit fixes this problem.

Fixes #354.
  • Loading branch information
lencioni committed Feb 6, 2016
1 parent dedf67a commit 59f26d7
Show file tree
Hide file tree
Showing 3 changed files with 128 additions and 0 deletions.
11 changes: 11 additions & 0 deletions docs/rules/prop-types.md
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,10 @@ var Hello = React.createClass({
return <div>Hello {this.props.firstname} {this.props.lastname}</div>; // lastname type is not defined in propTypes
}
});

function Hello({ name }) {
return <div>Hello {this.props.name}</div>;
}
```

The following patterns are not considered warnings:
Expand Down Expand Up @@ -50,6 +54,13 @@ var Hello = React.createClass({
return <div>Hello {this.props.name}</div>;
}
});

function Hello({ name }) {
return <div>Hello {this.props.name}</div>;
}
Hello.propTypes = {
name: React.PropTypes.string.isRequired,
};
```

## Rule Options
Expand Down
27 changes: 27 additions & 0 deletions lib/rules/prop-types.js
Original file line number Diff line number Diff line change
Expand Up @@ -505,6 +505,12 @@ module.exports = Components.detect(function(context, components, utils) {
properties = node.parent.id.properties;
}
break;
case 'ArrowFunctionExpression':
case 'FunctionDeclaration':
case 'FunctionExpression':
type = 'destructuring';
properties = node.params[0].properties;
break;
case 'VariableDeclarator':
for (var i = 0, j = node.id.properties.length; i < j; i++) {
// let {props: {firstname}} = this
Expand Down Expand Up @@ -701,6 +707,21 @@ module.exports = Components.detect(function(context, components, utils) {
return annotation;
}

/**
* @param {ASTNode} node We expect either an ArrowFunctionExpression,
* FunctionDeclaration, or FunctionExpression
*/
function markDestructuredFunctionArgumentsAsUsed(node) {
var destructuring = node.params &&
node.params.length === 1 &&
node.params[0] &&
node.params[0].type === 'ObjectPattern';

if (destructuring) {
markPropTypesAsUsed(node);
}
}

// --------------------------------------------------------------------------
// Public
// --------------------------------------------------------------------------
Expand All @@ -727,6 +748,12 @@ module.exports = Components.detect(function(context, components, utils) {
markPropTypesAsUsed(node);
},

FunctionDeclaration: markDestructuredFunctionArgumentsAsUsed,

ArrowFunctionExpression: markDestructuredFunctionArgumentsAsUsed,

FunctionExpression: markDestructuredFunctionArgumentsAsUsed,

MemberExpression: function(node) {
var type;
if (isPropTypesUsage(node)) {
Expand Down
90 changes: 90 additions & 0 deletions tests/lib/rules/prop-types.js
Original file line number Diff line number Diff line change
Expand Up @@ -836,6 +836,66 @@ ruleTester.run('prop-types', rule, {
'};'
].join('\n'),
parserOptions: parserOptions
}, {
code: [
'const statelessComponent = ({ someProp }) => {',
' const subRender = () => {',
' return <span>{someProp}</span>;',
' };',
' return <div>{subRender()}</div>;',
'};',
'statelessComponent.propTypes = {',
' someProp: PropTypes.string',
'};'
].join('\n'),
parserOptions: parserOptions
}, {
code: [
'const statelessComponent = function({ someProp }) {',
' const subRender = () => {',
' return <span>{someProp}</span>;',
' };',
' return <div>{subRender()}</div>;',
'};',
'statelessComponent.propTypes = {',
' someProp: PropTypes.string',
'};'
].join('\n'),
parserOptions: parserOptions
}, {
code: [
'function statelessComponent({ someProp }) {',
' const subRender = () => {',
' return <span>{someProp}</span>;',
' };',
' return <div>{subRender()}</div>;',
'};',
'statelessComponent.propTypes = {',
' someProp: PropTypes.string',
'};'
].join('\n'),
parserOptions: parserOptions
}, {
code: [
'function notAComponent({ something }) {',
' return something + 1;',
'};'
].join('\n'),
parserOptions: parserOptions
}, {
code: [
'const notAComponent = function({ something }) {',
' return something + 1;',
'};'
].join('\n'),
parserOptions: parserOptions
}, {
code: [
'const notAComponent = ({ something }) => {',
' return something + 1;',
'};'
].join('\n'),
parserOptions: parserOptions
}, {
// Validation is ignored on reassigned props object
code: [
Expand Down Expand Up @@ -1446,6 +1506,36 @@ ruleTester.run('prop-types', rule, {
errors: [{
message: '\'name\' is missing in props validation'
}]
}, {
code: [
'function Hello({ name }) {',
' return <div>Hello {name}</div>;',
'}'
].join('\n'),
parser: 'babel-eslint',
errors: [{
message: '\'name\' is missing in props validation'
}]
}, {
code: [
'const Hello = function({ name }) {',
' return <div>Hello {name}</div>;',
'}'
].join('\n'),
parser: 'babel-eslint',
errors: [{
message: '\'name\' is missing in props validation'
}]
}, {
code: [
'const Hello = ({ name }) => {',
' 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 59f26d7

Please sign in to comment.