Skip to content

Commit

Permalink
[Fix] prop-types: handle component returning null
Browse files Browse the repository at this point in the history
Fixes #2705.
  • Loading branch information
hank121314 authored and ljharb committed Jul 6, 2020
1 parent ee4bad3 commit c57cc31
Show file tree
Hide file tree
Showing 2 changed files with 62 additions and 23 deletions.
23 changes: 1 addition & 22 deletions lib/util/propTypes.js
Expand Up @@ -12,8 +12,6 @@ const variableUtil = require('./variable');
const versionUtil = require('./version');
const propWrapperUtil = require('./propWrapper');
const getKeyValue = require('./ast').getKeyValue;
const findReturnStatement = require('./ast').findReturnStatement;
const isJSX = require('./jsx').isJSX;

/**
* Checks if we are declaring a props as a generic type in a flow-annotated class.
Expand Down Expand Up @@ -72,25 +70,6 @@ function isInsideClassBody(node) {
return false;
}

/**
* Checks if a node is a Function and return JSXElement
*
* @param {ASTNode} node the AST node being checked.
* @returns {Boolean} True if the node is a Function and return JSXElement.
*/
function isJSXFunctionComponent(node) {
if (node.type === 'ArrowFunctionExpression' || node.type === 'FunctionDeclaration' || node.type === 'FunctionExpression') {
const res = findReturnStatement(node);
// If function return JSXElement with return keyword.
if (res) {
return isJSX(res.argument);
}
// If function return JSXElement without return keyword;
return isJSX(node.body);
}
return false;
}

module.exports = function propTypesInstructions(context, components, utils) {
// Used to track the type annotations in scope.
// Necessary because babel's scopes do not track type annotations.
Expand Down Expand Up @@ -699,7 +678,7 @@ module.exports = function propTypesInstructions(context, components, utils) {
}

// Should ignore function that not return JSXElement
if (!isJSXFunctionComponent(node)) {
if (!utils.isReturningJSXOrNull(node)) {
return;
}

Expand Down
62 changes: 61 additions & 1 deletion tests/lib/rules/prop-types.js 100755 → 100644
Expand Up @@ -2524,7 +2524,52 @@ ruleTester.run('prop-types', rule, {
MyComponent.propTypes = {
hello: PropTypes.string.isRequired,
};
`
`,
{
code: `
interface Props {
value?: string;
}
// without the | null, all ok, with it, it is broken
function Test ({ value }: Props): React.ReactElement<Props> | null {
if (!value) {
return null;
}
return <div>{value}</div>;
}`,
parser: parsers.TYPESCRIPT_ESLINT
},
{
code: `
interface Props {
value?: string;
}
// without the | null, all ok, with it, it is broken
function Test ({ value }: Props): React.ReactElement<Props> | null {
if (!value) {
return <div>{value}</div>;;
}
return null;
}`,
parser: parsers.TYPESCRIPT_ESLINT
},
{
code: `
interface Props {
value?: string;
}
const Hello = (props: Props) => {
if (props.value) {
return <div></div>;
}
return null;
}`,
parser: parsers.TYPESCRIPT_ESLINT
}
],

invalid: [
Expand Down Expand Up @@ -5101,6 +5146,21 @@ ruleTester.run('prop-types', rule, {
errors: [{
message: '\'foo.baz\' is missing in props validation'
}]
},
{
code: `
interface Props {
}
const Hello = (props: Props) => {
if (props.value) {
return <div></div>;
}
return null;
}`,
parser: parsers.TYPESCRIPT_ESLINT,
errors: [{
message: '\'value\' is missing in props validation'
}]
}
]
});

0 comments on commit c57cc31

Please sign in to comment.