no-unused-prop-types + Typescript : error is not reported

[meriadec]

Hey there! And thx for this wonderful plugin that help our team everyday 🚀 .

However, we face a problem on configuration of a new Typescript project with eslint + eslint-plugin-react: we can't make the rule react/no-unused-prop-types output any error (no idea if this is the only rule that is ignored, but this one is pretty helpful).

import React from "react";

type Props = {
  foo: string;
  bar: string; // <-- unused prop
};

const Demo = (props: Props): React.ReactNode => {
  return <div>{props.foo}</div>;
};

export default Demo;

Here we expect eslint to detect that bar is unused and report it. But unfortunately it's not. I've setup a little repo to illustrate the problem: https://github.com/meriadec/the-unused-prop-type

eslint config looks like this:

module.exports = {
  parser: "@typescript-eslint/parser",
  extends: ["plugin:react/recommended"],
  parserOptions: {
    ecmaVersion: 2018,
    sourceType: "module",
    ecmaFeatures: {
      jsx: true
    }
  },
  settings: {
    react: {
      version: "detect"
    }
  },
  rules: {
    "react/no-unused-prop-types": 2
  }
};

Not sure if the problem is related to @typescript-eslint/parser or eslint-plugin-react though. Could not find really relevant other issues raising the problem, so I hope there is something obvious that we are missing and that someone can point us 😄

[henrikra]

Can contributors comment on this that is this a bug or is typescript currently unsupported by no-unused-prop-types?

[ljharb]

@henrikra TypeScript should be supported with every rule; it's marked as a bug, and it needs fixing.

Whether the root problem is in the typescript parser, or in this plugin, remains to be seen.

[Xiphe]

I investigated this a bit further. In my case. component.declaredPropTypes is always {} so the parser does not find the prop declarations. I'm a bit lost debugging this further. My current thesis is that propTypes#resolveTypeAnnotation fails to correctly resolve to the type definition.

I also noted that the nodes in my case have types like TSTypeReference and TSTypeAnnotation and the plugin code does not seem to know what to do with them.

Any idea on which direction to go from here?

[saostad]

in .eslintrc :

"rules": {   
    "react/prop-types": 0
  }

[Xiphe]

@saostad could you describe how the prop-types rule helps here?

Afaik. this rule is to validate that all used props of a component are declared/typed. This issue is about the other way round. It should ensure that all declared/typed prop types are actually used.

[saostad]

@saostad could you describe how the prop-types rule helps here?

Afaik. this rule is to validate that all used props of a component are declared/typed. This issue is about the other way round. It should ensure that all declared/typed prop types are actually used.

I think there is a conflict between react/prop-types rule and typescript React.FC.
for my case typescript is checking the props!
Here is my .eslintrc file

{
  "parser": "@typescript-eslint/parser",
  "parserOptions": {
    "project": "./tsconfig.json"
  },
  "plugins": ["@typescript-eslint", "prettier", "react", "react-hooks"],
  "extends": [
    "eslint:recommended",
    "plugin:react/recommended",
    "plugin:@typescript-eslint/eslint-recommended",
    "plugin:@typescript-eslint/recommended",
    "prettier",
    "prettier/@typescript-eslint"
  ],
  "env": {
    "node": true,
    "es6": true,
    "browser": true
  },
  "settings": {
    "react": {
      "pragma": "React",
      "version": "detect"
    }
  },
  "rules": {
    "@typescript-eslint/restrict-plus-operands": "error",
    "@typescript-eslint/interface-name-prefix": 0,
    "no-async-promise-executor": 1,
    "@typescript-eslint/explicit-function-return-type": 0,
    "@typescript-eslint/indent": 0,
    "no-console": "warn",
    // React
    "react/jsx-uses-react": "error",
    "react/jsx-uses-vars": "error",
    "react-hooks/rules-of-hooks": "error",
    "react-hooks/exhaustive-deps": "warn",
    "react/prop-types": 0
  }
}

[ljharb]

It’s not a conflict; you should be using PropTypes and types, since types can’t cover most of what PropTypes can.

[saostad]

It’s not a conflict; you should be using PropTypes and types, since types can’t cover most of what PropTypes can.

what kind of things can't types cover that PropTypes can?
and how we should use both when don't work together?

[ljharb]

Integers, to name something incredibly basic. Strings that match regular expressions. https://npmjs.com/airbnb-prop-types has a ton of use cases.

They both absolutely work together! The only challenge is when a propType and a flow/TS type would be redundant - in that case, either just duplicate them, or use a type abstraction that can infer the proper type from the propType.

[saostad]

Integers, to name something incredibly basic. Strings that match regular expressions. https://npmjs.com/airbnb-prop-types has a ton of use cases.

They both absolutely work together! The only challenge is when a propType and a flow/TS type would be redundant - in that case, either just duplicate them, or use a type abstraction that can infer the proper type from the propType.

wow! I didn't know about those use cases and also these are what I am missing in typescript world and wish I had!
I am looking forward to use those in my code after this bug gets fix.
thanks.

[kylegillen]

Any updates or workarounds on this one?

[ljharb]

Nope, but PRs are welcome.

[henrikra]

@ljharb Did you get a grasp of the problem according to @Xiphe 's explanation?

[ljharb]

Oops, yes, sorry.

Any idea on which direction to go from here?

If the problem is that the TypeScript node names the propType detection is looking for aren't matching, then presumably you could do some debugging to figure out what the correct ones should be, and add those to the detection code. I'd start by adding a few failing test cases with both the typescript and the @typescript parsers :-)

[Xiphe]

Just to let y'all know, I wont have time to look deeper into this in the near future. Good Luck 👍

[eltonio450]

Hi everyone,

It turns out the typescript parser references the type node as "TSTypeReference", and from there, the reconciliation with the type declaration was not implemented. I added it to propTypes.js, but i feel that I may have forgotten edge cases. If one of you who knows better the code base than me could have a look and tell me what's left to do, it would be awesome :)

Thank you !

[henrikra]

Very nice! Is the fixed version already released on npm?

[ljharb]

Nope. If you click on the commit URL listed above (13a863b) you'll see that there's no version tags listed, which means it hasn't been released yet. You can also confirm this in the changelog.

[henrikra]

Great! Waiting for new release

[mkraina]

I've just tried out the newly released version and I'm still unable to make this rule work, no idea what's the problem, isn't it necessary to set typescript version somewhere in the plugin settings?

[eltonio450]

me neither 🤣 I'll dig this afternoon and keep you posted !

[eltonio450]

ok, so it already works with interface, but not with type, bc I mixed the parsers in my PR :( #2661 + added bugs to unhandled edge cases

but @hank121314 made a huge work refactoring, cleaning and fixing bugs related to props declaration in typescript in #2721 (thank you very much!), so it should work in the next release 👍

overall, typescript props declaration to "eslint usable format" seems to be behind us now :)

[kkgowtamasa]

I didn't the resolution. @meriadec can you please share how you resolve this problem.

[meriadec]

I've just updated my demo repo and I confirm the problem is fixed (I've just bumped dependencies, it now relies on [email protected]). Feel free to check what configuration is in place, hope it'll help fixing on your side! @kkgowtamasa