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

react/prop-types - False positive when ternary conditional return #2786

Closed
gaara4896 opened this issue Sep 4, 2020 · 6 comments · Fixed by #3051
Closed

react/prop-types - False positive when ternary conditional return #2786

gaara4896 opened this issue Sep 4, 2020 · 6 comments · Fixed by #3051

Comments

@gaara4896
Copy link

Tell us about your environment

  • ESLint Version: 7.5.0
  • ESLint Plugin React Version: 7.20.3
  • Node Version: 10.15.3
  • npm Version: 6.4.1

What parser (default, Babel-ESLint, etc.) are you using?

Please show your full configuration:

Configuration
   extends: [
    'eslint:recommended',
    'plugin:@typescript-eslint/eslint-recommended',
    'plugin:@typescript-eslint/recommended',
    'plugin:react/recommended',
    'plugin:jsx-a11y/recommended',
    // Prettier plugin and recommended rules
    'prettier/@typescript-eslint',
    'plugin:prettier/recommended',
  ],
  rules: {
    // Include .prettierrc.js rules
    'prettier/prettier': ['error', {}, { usePrettierrc: true }],
    'react/prop-types': 'error',
    'react/react-in-jsx-scope': 'error',
    '@typescript-eslint/explicit-function-return-type': 'error',
    '@typescript-eslint/ban-ts-ignore': 'off',
    'jsx-a11y/label-has-associated-control': [
      'error',
      {
        labelComponents: [],
        labelAttributes: [],
        controlComponents: [],
        assert: 'either',
        depth: 25,
      },
    ],
    'jsx-a11y/anchor-is-valid': 'off',
    '@typescript-eslint/no-explicit-any': 'off',
    '@typescript-eslint/explicit-module-boundary-types': 'error',
    '@typescript-eslint/no-unused-vars': 'error',
    "sort-imports": ["error", {
      "ignoreCase": false,
      "ignoreDeclarationSort": false,
      "ignoreMemberSort": false,
      "memberSyntaxSortOrder": ["none", "all", "single", "multiple"],
      "allowSeparatedGroups": true
    }],
    "@typescript-eslint/ban-types": [
      "error",
      {
        "types": {
          String: {
            message: 'Use string instead',
            fixWith: 'string',
          },
          Boolean: {
            message: 'Use boolean instead',
            fixWith: 'boolean',
          },
          Number: {
            message: 'Use number instead',
            fixWith: 'number',
          },
          Symbol: {
            message: 'Use symbol instead',
            fixWith: 'symbol',
          },

          Function: {
            message: [
              'The `Function` type accepts any function-like value.',
              'It provides no type safety when calling the function, which can be a common source of bugs.',
              'It also accepts things like class declarations, which will throw at runtime as they will not be called with `new`.',
              'If you are expecting the function to accept certain arguments, you should explicitly define the function shape.',
            ].join('\n'),
          },

          // object typing
          Object: {
            message: [
              'The `Object` type actually means "any non-nullish value", so it is marginally better than `unknown`.',
              '- If you want a type meaning "any object", you probably want `Record<string, unknown>` instead.',
              '- If you want a type meaning "any value", you probably want `unknown` instead.',
            ].join('\n'),
          },
          '{}': {
            message: [
              '`{}` actually means "any non-nullish value".',
              '- If you want a type meaning "any object", you probably want `Record<string, unknown>` instead.',
              '- If you want a type meaning "any value", you probably want `unknown` instead.',
            ].join('\n'),
          },
        },
        "extendDefaults": false
      }
    ]
  },

What did you do? Please include the actual source code causing the issue, as well as the command that you used to run ESLint.

import React from 'react';

interface Props {
  item: any;
}

const SomeComponent: React.FC<Props> = ({ item }: Props) => {
  return item ? <></> : <></>;
};

export default SomeComponent;
eslint --fix . --ext .ts,.tsx,.js,.jsx

What did you expect to happen?

There should not be an error because item had been declared with type any.

What actually happened? Please include the actual, raw output from ESLint.

yarn run v1.17.3
$ eslint --fix . --ext .ts,.tsx

/home/gaara/JS/linked-nextapp/components/business/card/public/PublicCard.tsx
  7:40  error  'item' is missing in props validation  react/prop-types

✖ 1 problem (1 error, 0 warnings)

error Command failed with exit code 1.
info Visit https://yarnpkg.com/en/docs/cli/run for documentation about this command.

How do you verify the source of problem

I had tried the following code, which eslint no longer throw error.

import React from 'react';

interface Props {
  item: any;
}

const SomeComponent: React.FC<Props> = ({ item }: Props) => {
  if(item) {
    return <></>
  }
  return <></>
};

export default SomeComponent;
yarn run v1.17.3
$ eslint --fix . --ext .ts,.tsx
Done in 3.09s.
@ljharb
Copy link
Member

ljharb commented Sep 4, 2020

This is caused by two things:

  1. you're using an arrow function as a component, which has a number of downsides
  2. the TypeScript eslint parser doesn't attach the type of arrow functions to the function, only to the variable that holds it, so we don't know what that type is.

Duplicate of #2777.

@ljharb ljharb closed this as completed Sep 4, 2020
@gaara4896
Copy link
Author

Actually this is a different case with #2777 .

Below is what happen in #2777

// Error    'item' is missing in props validation       react/prop-types
const MyFc: React.FC<Props> = ({ item }) => { }

// No error, fix with define the props type explicitely
const MyFc: React.FC<Props> = ({ item }: Props) => { }

However, the issue that I am facing only happen specifically when a ternary condition is used for the return.

To address your point 2,

the TypeScript eslint parser doesn't attach the type of arrow functions to the function, only to the variable that holds it

... = ({ item }: Props) = ....
// Proper type had been assigned again explicitely

@vedadeepta
Copy link
Contributor

vedadeepta commented Aug 25, 2021

Should be fixed, tested with your use case locally - works, although I don't see how the ternary thing makes a difference, this seems like a corollary #3045

  interface Props {
    item: any;
  }

  const SomeComponent: React.FC<Props> = ({ item }: Props) => {
    return item ? <></> : <></>;
  };

@ljharb
Copy link
Member

ljharb commented Aug 25, 2021

A PR adding that test case would be a great way to close this issue :-)

@vedadeepta
Copy link
Contributor

this should be closed, imo

cc @ljharb

@ljharb
Copy link
Member

ljharb commented Aug 31, 2021

Thanks, closed by #3051.

@ljharb ljharb closed this as completed Aug 31, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
3 participants