react/prop-types & TypeScript: false positive with HTMLAttributes prop type

[cebamps]

Using React.HTMLAttributes<HTMLButtonElement> as the prop type in a component definition causes false positives of react/prop-types.

Strangely enough, when adding indirection by extending that same type in an interface, the problem goes away. The same technique applied to a type alias for the same type does not work, however.

Even stranger, the last case in the minimal working example below shows a case where prop type T gives the false positive while T & T does not.

Version: [email protected]

Minimal working example:

import React from "react";

// ERROR: 'className' is missing in props validation  react/prop-types
const MyComponent = ({ className }: React.HTMLAttributes<HTMLButtonElement>) => null;

// ERROR: 'className' is missing in props validation  react/prop-types
type TypeAlias = React.HTMLAttributes<HTMLButtonElement>;
const MyComponent_Alias = ({ className }: TypeAlias) => null;

// ERROR: 'className' is missing in props validation  react/prop-types
interface ExtendedTypeAlias extends TypeAlias {}
const MyComponent_ExtendedAlias = ({ className }: ExtendedTypeAlias) => null;

// OK
interface ExtendedInterface extends React.HTMLAttributes<HTMLButtonElement> {}
const MyComponent_ExtendedInterface = ({ className }: ExtendedInterface) => null;

// OK !?
const MyComponent_AliasSelfIntersection = ({ className }: TypeAlias & TypeAlias) => null;

[ljharb]

That is indeed bizarre, I'd expect them all to work, or not work, the same.

Type resolution typically comes from the TS eslint resolver, so it's possible some of the issue is on that side (cc @bradzacher) but it'll need investigation here regardless.

[bradzacher]

I'll start by saying that this rule seems like it is pretty redundant in TS code because TS will catch the case where you're using a property that's not declared in the props object type. I.e. the rule will double-report (and likely do a worse job than TS as it's not type-aware).

---

Type resolution typically comes from the TS eslint resolver

I don't think it does - it looks like the logic is hard coded to look for top-level type/interface declarations instead of using scope analysis:

eslint-plugin-react/lib/util/propTypes.js

Line 672 in 5919660

const candidateTypes = this.sourceCode.ast.body.filter((item) => astUtil.isTSTypeDeclaration(item));

eslint-plugin-react/lib/util/propTypes.js

Lines 681 to 684 in 5919660

	const interfaceDeclarations = this.sourceCode.ast.body
	.filter(filterInterfaceOrTypeAlias)
	.filter((item) => filterInterfaceOrAliasByName(item, typeName))
	.map((item) => (item.declaration || item));

---

There's a lot of code and utilities that your rules are built on top of - so it's hard to quickly decipher what the rule is doing specifically, but some quick poking around...

I can see that here you use the right-most name (i.e. HTMLAttributes):

eslint-plugin-react/lib/util/propTypes.js

Lines 638 to 640 in 5919660

	const idx = genericTypeParamIndexWherePropsArePresent[
	leftMostName !== rightMostName ? rightMostName : importedName
	];

to index this variable:

eslint-plugin-react/lib/util/propTypes.js

Lines 104 to 114 in 5919660

	const genericTypeParamIndexWherePropsArePresent = {
	ForwardRefRenderFunction: 1,
	forwardRef: 1,
	VoidFunctionComponent: 0,
	VFC: 0,
	PropsWithChildren: 0,
	SFC: 0,
	StatelessComponent: 0,
	FunctionComponent: 0,
	FC: 0,
	};

Which will return undefined, meaning the next line will return undefined:

eslint-plugin-react/lib/util/propTypes.js

Lines 641 to 642 in 5919660

	const nextNode = node.typeParameters.params[idx];
	this.visitTSNode(nextNode);

and the traversal will completely exit:

eslint-plugin-react/lib/util/propTypes.js

Lines 591 to 592 in 5919660

	visitTSNode(node) {
	if (!node) return;

So I think that what's happening is that the rule is just not registering any properties, nor is it marking the props as something the rule should ignore?

[ljharb]

Interesting, thanks, that's a really helpful analysis.

I also agree that if TS will catch things that aren't declared, then it might be better for the prop-types rule to merely ensure that there is a TS type (presumably not any, though).