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.

Sign up for GitHub

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

Jump to bottom

[Fix] prop-types, propTypes: add forwardRef<>, ForwardRefRenderFunction<> prop-types #3112

Merged
merged 1 commit into from Oct 24, 2021
Merged

[Fix] prop-types, propTypes: add forwardRef<>, ForwardRefRenderFunction<> prop-types #3112

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
4 changes: 3 additions & 1 deletion CHANGELOG.md
View file
Open in desktop
Expand Up @@ -10,14 +10,16 @@ This change log adheres to standards from [Keep a CHANGELOG](http://keepachangel
* add [`no-arrow-function-lifecycle`] ([#1980][] @ngtan)

### Fixed
* [`propTypes`]: add `VoidFunctionComponent` to react generic list ([#3092][] @vedadeepta)
* `propTypes`: add `VoidFunctionComponent` to react generic list ([#3092][] @vedadeepta)
* [`jsx-fragments`], [`jsx-no-useless-fragment`]: avoid a crash on fragment syntax in `typescript-eslint` parser (@ljharb)
* [`jsx-props-no-multi-spaces`]: avoid a crash on long member chains in tag names in `typescript-eslint` parser (@ljharb)
* [`no-unused-prop-types`], `usedPropTypes`: avoid crash with typescript-eslint parser (@ljharb)
* [`display-name`]: unwrap TS `as` expressions ([#3110][] @ljharb)
* [`destructuring-assignment`]: detect refs nested in functions ([#3102] @ljharb)
* [`no-unstable-components`]: improve handling of objects containing render function properties ([#3111] @fizwidget)
* [`prop-types`], `propTypes`: add forwardRef<>, ForwardRefRenderFunction<> prop-types ([#3112] @vedadeepta)

[#3112]: https://github.com/yannickcr/eslint-plugin-react/pull/3112
[#3111]: https://github.com/yannickcr/eslint-plugin-react/pull/3111
[#3110]: https://github.com/yannickcr/eslint-plugin-react/pull/3110
[#3102]: https://github.com/yannickcr/eslint-plugin-react/issue/3102
Expand Down
58 changes: 53 additions & 5 deletions lib/util/propTypes.js
View file
Open in desktop
Expand Up @@ -100,8 +100,20 @@ module.exports = function propTypesInstructions(context, components, utils) {
const defaults = { customValidators: [] };
const configuration = Object.assign({}, defaults, context.options[0] || {});
const customValidators = configuration.customValidators;
const allowedGenericTypes = new Set(['VoidFunctionComponent', 'PropsWithChildren', 'SFC', 'StatelessComponent', 'FunctionComponent', 'FC']);
const allowedGenericTypes = new Set(['forwardRef', 'ForwardRefRenderFunction', 'VoidFunctionComponent', 'PropsWithChildren', 'SFC', 'StatelessComponent', 'FunctionComponent', 'FC']);
const genericTypeParamIndexWherePropsArePresent = {
ForwardRefRenderFunction: 1,
forwardRef: 1,
VoidFunctionComponent: 0,
PropsWithChildren: 0,
SFC: 0,
StatelessComponent: 0,
FunctionComponent: 0,
FC: 0,
};
const genericReactTypesImport = new Set();
// import { FC as X } from 'react' -> localToImportedMap = { x: FC }
const localToImportedMap = {};
Comment on lines +115 to +116
Copy link
Member

@ljharb ljharb Oct 24, 2021 •
edited

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

is this information not something eslint itself already provides?

(if so, we can update it in a followup)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You mean if it provides in a hashmap? Not sure, will inspect the ast sometime later and update here

Copy link
Member

@ljharb ljharb Oct 24, 2021 •
edited

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

not in a hashmap - i believe eslint has a context method that takes a variable name or a node that references a variable or something, and gives you the node that defined it.


/**
* Returns the full scope.
Expand Down Expand Up @@ -521,9 +533,14 @@ module.exports = function propTypesInstructions(context, components, utils) {
* @param {ASTNode} node
* @return {string | undefined}
*/
function getTypeName(node) {
function getLeftMostTypeName(node) {
if (node.name) return node.name;
if (node.left) return getLeftMostTypeName(node.left);
}

function getRightMostTypeName(node) {
if (node.name) return node.name;
if (node.left) return getTypeName(node.left);
if (node.right) return getRightMostTypeName(node.right);
}

class DeclarePropTypesForTSTypeAnnotation {
Expand Down Expand Up @@ -579,14 +596,20 @@ module.exports = function propTypesInstructions(context, components, utils) {
let typeName;
if (astUtil.isTSTypeReference(node)) {
typeName = node.typeName.name;
const shouldTraverseTypeParams = genericReactTypesImport.has(getTypeName(node.typeName));
const leftMostName = getLeftMostTypeName(node.typeName);
const shouldTraverseTypeParams = genericReactTypesImport.has(leftMostName);
if (shouldTraverseTypeParams && node.typeParameters && node.typeParameters.length !== 0) {
// All react Generic types are derived from:
// type PropsWithChildren<P> = P & { children?: ReactNode | undefined }
// So we should construct an optional children prop
this.shouldSpecifyOptionalChildrenProps = true;

const nextNode = node.typeParameters.params[0];
const rightMostName = getRightMostTypeName(node.typeName);
const importedName = localToImportedMap[rightMostName];
const idx = genericTypeParamIndexWherePropsArePresent[
leftMostName !== rightMostName ? rightMostName : importedName
];
const nextNode = node.typeParameters.params[idx];
this.visitTSNode(nextNode);
return;
}
Expand Down Expand Up @@ -941,6 +964,30 @@ module.exports = function propTypesInstructions(context, components, utils) {
return;
}

if (
node.parent
&& node.parent.callee
&& node.parent.typeParameters
&& node.parent.typeParameters.params
&& (
node.parent.callee.name === 'forwardRef' || (
node.parent.callee.object
&& node.parent.callee.property
&& node.parent.callee.object.name === 'React'
&& node.parent.callee.property.name === 'forwardRef'
)
)
) {
const propTypes = node.parent.typeParameters.params[1];
const declaredPropTypes = {};
const obj = new DeclarePropTypesForTSTypeAnnotation(propTypes, declaredPropTypes);
components.set(node, {
declaredPropTypes: obj.declaredPropTypes,
ignorePropsValidation: false,
});
return;
}

const siblingIdentifier = node.parent && node.parent.id;
const siblingHasTypeAnnotation = siblingIdentifier && siblingIdentifier.typeAnnotation;
const isNodeAnnotated = annotations.isAnnotatedFunctionPropsDeclaration(node, context);
Expand Down Expand Up @@ -1092,6 +1139,7 @@ module.exports = function propTypesInstructions(context, components, utils) {
// 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);
localToImportedMap[specifier.local.name] = specifier.imported.name;
}
});
}
Expand Down
189 changes: 182 additions & 7 deletions tests/lib/rules/prop-types.js
View file
Open in desktop
Expand Up @@ -462,9 +462,9 @@ ruleTester.run('prop-types', rule, {
code: `
class Hello extends React.Component {
render() {
var {
var {
propX,
"aria-controls": ariaControls,
"aria-controls": ariaControls,
...props } = this.props;
return <div>Hello</div>;
}
Expand Down Expand Up @@ -1346,7 +1346,7 @@ ruleTester.run('prop-types', rule, {
{
code: `
import type { FieldProps } from "redux-form"

type Props = {
label: string,
type: string,
Expand Down Expand Up @@ -3411,6 +3411,111 @@ ruleTester.run('prop-types', rule, {
`,
features: ['ts', 'no-babel'],
},
{
code: `
import React, { ForwardRefRenderFunction as X } from 'react'

type IfooProps = { e: string };
const Foo: X<HTMLDivElement, IfooProps> = function Foo (props, ref) {
const { e } = props;
return <div ref={ref}>hello</div>;
};
`,
features: ['ts', 'no-babel'],
},
{
code: `
import React, { ForwardRefRenderFunction } from 'react'

type IfooProps = { e: string };
const Foo: ForwardRefRenderFunction<HTMLDivElement, IfooProps> = function Foo (props, ref) {
const { e } = props;
return <div ref={ref}>hello</div>;
};
`,
features: ['ts', 'no-babel'],
},
{
code: `
import React, { ForwardRefRenderFunction } from 'react'

type IfooProps = { e: string };
const Foo: ForwardRefRenderFunction<HTMLDivElement, IfooProps> = (props, ref) => {
const { e } = props;
return <div ref={ref}>hello</div>;
};
`,
features: ['ts', 'no-babel'],
},
{
code: `
import React from 'react'

type IfooProps = { e: string };
const Foo= React.forwardRef<HTMLDivElement, IfooProps>((props, ref) => {
const { e } = props;
return <div ref={ref}>hello</div>;
});
`,
features: ['ts', 'no-babel'],
},
{
code: `
import React, { forwardRef } from 'react'

type IfooProps = { e: string };
const Foo= forwardRef<HTMLDivElement, IfooProps>((props, ref) => {
const { e } = props;
return <div ref={ref}>hello</div>;
});
`,
features: ['ts', 'no-babel'],
},
{
code: `
import React from 'react'
type IfooProps = { e: string };
const Foo= React.forwardRef<HTMLDivElement, IfooProps>(function Foo(props, ref) {
const { e } = props;
return <div ref={ref}>hello</div>;
});
`,
features: ['ts', 'no-babel'],
},
{
code: `
import React from 'react'
interface IfooProps { e: string }
const Foo= React.forwardRef<HTMLDivElement, IfooProps>(function Foo(props, ref) {
const { e } = props;
return <div ref={ref}>hello</div>;
});
`,
features: ['ts', 'no-babel'],
},
{
code: `
import React, { forwardRef } from 'react'
interface IfooProps { e: string }
const Foo= forwardRef<HTMLDivElement, IfooProps>(function Foo(props, ref) {
const { e } = props;
return <div ref={ref}>hello</div>;
});
`,
features: ['ts', 'no-babel'],
},
{
code: `
import React, { forwardRef as X } from 'react'

type IfooProps = { e: string };
const Foo= X<HTMLDivElement, IfooProps>((props, ref) => {
const { e } = props;
return <div ref={ref}>hello</div>;
});
`,
features: ['ts', 'no-babel'],
},
{
code: `
import React from 'react'
Expand All @@ -3430,7 +3535,7 @@ ruleTester.run('prop-types', rule, {
static propTypes = {
value: PropTypes.string
};

render() {
return <span>{this.props.value}</span>;
}
Expand Down Expand Up @@ -3830,7 +3935,7 @@ ruleTester.run('prop-types', rule, {
semver.satisfies(babelEslintVersion, '< 9') ? {
code: `
class Hello extends React.Component {
static propTypes: {
static propTypes: {
firstname: PropTypes.string
};
render() {
Expand Down Expand Up @@ -4019,8 +4124,8 @@ ruleTester.run('prop-types', rule, {
code: `
class Hello extends React.Component {
render() {
var {
"aria-controls": ariaControls,
var {
"aria-controls": ariaControls,
propX,
...props } = this.props;
return <div>Hello</div>;
Expand Down Expand Up @@ -7139,6 +7244,76 @@ ruleTester.run('prop-types', rule, {
},
],
features: ['ts', 'no-babel'],
},
{
code: `
import React from 'react'

type IfooProps = { e: string };
const Foo: React.ForwardRefRenderFunction<HTMLDivElement, IfooProps> = function Foo (props, ref) {
const { name } = props;
return <div ref={ref}>{name}</div>;
};
`,
errors: [
{
messageId: 'missingPropType',
data: { name: 'name' },
},
],
features: ['ts', 'no-babel'],
},
{
code: `
import React from 'react'
type IfooProps = { k: string, a: number }
const Foo= React.forwardRef<HTMLDivElement, IfooProps>((props, ref) => {
return <div ref={ref}>{props.l}</div>;
});
`,
errors: [
{
messageId: 'missingPropType',
data: { name: 'l' },
},
],
features: ['ts', 'no-babel'],
},
{
code: `
import React from 'react'

type IfooProps = { e: string };
const Foo= React.forwardRef<HTMLDivElement, IfooProps>(function Foo(props, ref) {
const { l } = props;
return <div ref={ref}>hello</div>;
});
`,
errors: [
{
messageId: 'missingPropType',
data: { name: 'l' },
},
],
features: ['ts', 'no-babel'],
},
{
code: `
import React, { forwardRef } from 'react'

type IfooProps = { e: string };
const Foo= forwardRef<HTMLDivElement, IfooProps>(function Foo(props, ref) {
const { l } = props;
return <div ref={ref}>hello</div>;
});
`,
errors: [
{
messageId: 'missingPropType',
data: { name: 'l' },
},
],
features: ['ts', 'no-babel'],
}
)),
});