Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Fix] prop-types, propTypes: add handling for FC<Props>, improve tests #3051

Merged
merged 1 commit into from Aug 27, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
2 changes: 2 additions & 0 deletions CHANGELOG.md
Expand Up @@ -22,6 +22,7 @@ This change log adheres to standards from [Keep a CHANGELOG](http://keepachangel
* [`no-typos`]: fix crash on private methods ([#3043][] @ljharb)
* [`jsx-no-bind`]: handle local function declarations ([#3048][] @p7g)
* [`prop-types`], `propTypes`: handle React.* TypeScript types ([#3049][] @vedadeepta)
* [`prop-types`], `propTypes`: add handling for `FC<Props>`, improve tests ([#3051][] @vedadeepta)

### Changed
* [Docs] [`jsx-no-bind`]: updates discussion of refs ([#2998][] @dimitropoulos)
Expand All @@ -32,6 +33,7 @@ This change log adheres to standards from [Keep a CHANGELOG](http://keepachangel
* [Docs] improve instructions for `jsx-runtime` config ([#3052][] @ljharb)

[#3052]: https://github.com/yannickcr/eslint-plugin-react/issues/3052
[#3051]: https://github.com/yannickcr/eslint-plugin-react/pull/3051
[#3049]: https://github.com/yannickcr/eslint-plugin-react/pull/3049
[#3048]: https://github.com/yannickcr/eslint-plugin-react/pull/3048
[#3043]: https://github.com/yannickcr/eslint-plugin-react/issues/3043
Expand Down
42 changes: 36 additions & 6 deletions lib/util/propTypes.js
Expand Up @@ -100,7 +100,8 @@ module.exports = function propTypesInstructions(context, components, utils) {
const defaults = {customValidators: []};
const configuration = Object.assign({}, defaults, context.options[0] || {});
const customValidators = configuration.customValidators;
const allowedGenericTypes = ['React.SFC', 'React.StatelessComponent', 'React.FunctionComponent', 'React.FC'];
const allowedGenericTypes = new Set(['SFC', 'StatelessComponent', 'FunctionComponent', 'FC']);
const genericReactTypesImport = new Set();

/**
* Returns the full scope.
Expand Down Expand Up @@ -548,7 +549,8 @@ module.exports = function propTypesInstructions(context, components, utils) {
let typeName;
if (astUtil.isTSTypeReference(node)) {
typeName = node.typeName.name;
if (!typeName && node.typeParameters && node.typeParameters.length !== 0) {
const shouldTraverseTypeParams = !typeName || genericReactTypesImport.has(typeName);
if (shouldTraverseTypeParams && node.typeParameters && node.typeParameters.length !== 0) {
const nextNode = node.typeParameters.params[0];
this.visitTSNode(nextNode);
return;
Expand Down Expand Up @@ -940,12 +942,19 @@ module.exports = function propTypesInstructions(context, components, utils) {
return;
}

const typeName = context.getSourceCode().getText(annotation.typeName).replace(/ /g, '');
if (annotation.typeName.name) { // if FC<Props>
const typeName = annotation.typeName.name;
if (!genericReactTypesImport.has(typeName)) {
return;
}
} else if (annotation.typeName.right.name) { // if React.FC<Props>
const right = annotation.typeName.right.name;
const left = annotation.typeName.left.name;

if (allowedGenericTypes.indexOf(typeName) === -1) {
return;
if (!genericReactTypesImport.has(left) || !allowedGenericTypes.has(right)) {
return;
}
}

markPropTypesAsDeclared(node, resolveTypeAnnotation(siblingIdentifier));
}
}
Expand Down Expand Up @@ -1036,6 +1045,27 @@ module.exports = function propTypesInstructions(context, components, utils) {
}
},

ImportDeclaration(node) {
// parse `import ... from 'react`
if (node.source.value === 'react') {
node.specifiers.forEach((specifier) => {
if (
// handles import * as X from 'react'
specifier.type === 'ImportNamespaceSpecifier'
// handles import React from 'react'
|| specifier.type === 'ImportDefaultSpecifier'
) {
genericReactTypesImport.add(specifier.local.name);
}

// handles import { FC } from 'react' or import { FC as X } from 'react'
if (specifier.type === 'ImportSpecifier' && allowedGenericTypes.has(specifier.imported.name)) {
genericReactTypesImport.add(specifier.local.name);
}
});
}
},

FunctionDeclaration: markAnnotatedFunctionArgumentsAsDeclared,

ArrowFunctionExpression: markAnnotatedFunctionArgumentsAsDeclared,
Expand Down
136 changes: 136 additions & 0 deletions tests/lib/rules/prop-types.js
Expand Up @@ -3150,6 +3150,8 @@ ruleTester.run('prop-types', rule, {
},
{
code: `
import React from 'react';

interface PersonProps {
username: string;
}
Expand All @@ -3161,6 +3163,8 @@ ruleTester.run('prop-types', rule, {
},
{
code: `
import React from 'react';

const Person: React.FunctionComponent<PersonProps> = (props): React.ReactElement => (
<div>{props.username}</div>
);
Expand All @@ -3169,6 +3173,8 @@ ruleTester.run('prop-types', rule, {
},
{
code: `
import React from 'react';

interface PersonProps {
username: string;
}
Expand All @@ -3180,6 +3186,7 @@ ruleTester.run('prop-types', rule, {
},
{
code: `
import React from 'react';
const Person: React.FunctionComponent<{ username: string }> = (props): React.ReactElement => (
<div>{props.username}</div>
);
Expand All @@ -3188,6 +3195,7 @@ ruleTester.run('prop-types', rule, {
},
{
code: `
import React from 'react';
type PersonProps = {
username: string;
}
Expand All @@ -3196,6 +3204,82 @@ ruleTester.run('prop-types', rule, {
);
`,
parser: parsers['@TYPESCRIPT_ESLINT']
},
{
code: `
import { FunctionComponent } from 'react';

type PersonProps = {
username: string;
}
const Person: FunctionComponent<PersonProps> = (props): React.ReactElement => (
<div>{props.username}</div>
);
`,
parser: parsers['@TYPESCRIPT_ESLINT']
},
{
code: `
import { FC } from 'react';
type PersonProps = {
username: string;
}
const Person: FC<PersonProps> = (props): React.ReactElement => (
<div>{props.username}</div>
);
`,
parser: parsers['@TYPESCRIPT_ESLINT']
},
{
code: `
import type { FC } from 'react';
type PersonProps = {
username: string;
}
const Person: FC<PersonProps> = (props): React.ReactElement => (
<div>{props.username}</div>
);
`,
parser: parsers['@TYPESCRIPT_ESLINT']
},
{
code: `
import { FC as X } from 'react';
interface PersonProps {
username: string;
}
const Person: X<PersonProps> = (props): React.ReactElement => (
<div>{props.username}</div>
);
`,
parser: parsers['@TYPESCRIPT_ESLINT']
},
{
code: `
import * as X from 'react';
interface PersonProps {
username: string;
}
const Person: X.FC<PersonProps> = (props): React.ReactElement => (
<div>{props.username}</div>
);
`,
parser: parsers['@TYPESCRIPT_ESLINT']
},
{
// issue: https://github.com/yannickcr/eslint-plugin-react/issues/2786
code: `
import React from 'react';

interface Props {
item: any;
}

const SomeComponent: React.FC<Props> = ({ item }: Props) => {
return item ? <></> : <></>;
};
`,
parser: parsers['@TYPESCRIPT_ESLINT']
}
]),
{
Expand Down Expand Up @@ -6706,6 +6790,58 @@ ruleTester.run('prop-types', rule, {
}
],
parser: parsers['@TYPESCRIPT_ESLINT']
},
{
code: `
import React from 'react';
interface PersonProps {
username: string;
}
const Person: FunctionComponent<PersonProps> = (props): React.ReactElement => (
<div>{props.test}</div>
);
`,
errors: [
{
messageId: 'missingPropType',
data: {name: 'test'}
}
],
parser: parsers['@TYPESCRIPT_ESLINT']
},
{
code: `
interface PersonProps {
username: string;
}
const Person: X.FC<PersonProps> = (props): React.ReactElement => (
<div>{props.username}</div>
);
`,
errors: [
{
messageId: 'missingPropType',
data: {name: 'username'}
}
],
parser: parsers['@TYPESCRIPT_ESLINT']
},
{
code: `
interface PersonProps {
username: string;
}
const Person: FC<PersonProps> = (props): React.ReactElement => (
<div>{props.username}</div>
);
`,
errors: [
{
messageId: 'missingPropType',
data: {name: 'username'}
}
],
parser: parsers['@TYPESCRIPT_ESLINT']
}
])
)
Expand Down