jsx-eslint / eslint-plugin-react

React-specific linting rules for ESLint
MIT License
9k stars 2.77k forks source link

react/prop-types not detecting props when defined as an interface #2747

Closed jarretmoses closed 4 years ago

jarretmoses commented 4 years ago

Hello,

It seems this issue has arose a couple times but in different fashion however this rules seems to still NOT detect Props correctly when using an interface vs a type. (I've been waiting several versions as I kept seeing people say this was resolved.

Versions eslint-plugin-react: 7.20.5 typescript: 3.9.7 @typescript-eslint/eslint-plugin: 3.8.0, @typescript-eslint/parser: 3.8.0,

Example

import React from 'react';
import FacebookLogin, { ReactFacebookLoginInfo } from 'react-facebook-login';

import { rollbar } from '../../../services/rollbar.service';
import { intercomService } from '../../../services/intercom.service';
import { V2SocialLoginProps } from '../../../interfaces/shared-props.interface';

import facebookIcon from '../../assets/images/icons/facebook.svg';

const APP_ID = process.env.REACT_APP_FACEBOOK_APP_ID || '';

interface Props extends V2SocialLoginProps {
  autoLoad: boolean;
}

export const DKFacebookButton = ({
  autoLoad,
  authAction,
  errorHandler,
  redirectUrl,
  isSignup,
}: Props): JSX.Element | null => {
  if (!APP_ID) {
    rollbar.error('Missing Facebook OAuth App Id');
    return null;
  }

  const fbButtonText = isSignup ? 'Sign up with Facebook' : 'Log in with Facebook';

  const responseCallback = async ({
    accessToken,
    email = '',
    name = '',
  }: ReactFacebookLoginInfo) => {
    const [firstName, lastName] = name.split(' ');

    const requestData: DK.SocialLogin = {
      accessToken,
      email,
      firstName,
      lastName,
      intercomUserId: intercomService.getVisitorId(),
    };

    try {
      await authAction(requestData, redirectUrl);
    } catch (err) {
      errorHandler(err.message);
    }
  };

  const FacebookIcon = () => (
    <img
      style={{ marginRight: '8px' }}
      src={facebookIcon}
      alt='Facebook Login'
    />
  );

  return (
    <FacebookLogin
      cssClass='ant-btn dk-button dk-facebook-button dk-button--secondary ant-btn-primary ant-btn-lg'
      autoLoad={autoLoad}
      textButton={fbButtonText}
      size='small'
      icon={<FacebookIcon />}
      appId={APP_ID}
      fields='name,email'
      callback={responseCallback}
      data-testId='dk-facebook-button'
    />
  );
};

I can confirm changing the Props interface to
typo Props = { autoLoad: boolean; } & V2SocialLoginPropsfixes the issue but I don't think I should need to refactor my entire codebase to use type.

I can also confirm this issue where theres no external interface to extend and its just a standalone interface definition.

ljharb commented 4 years ago

While I prefer type to interface, you're totally right that this should work.

jzabala commented 4 years ago

Working on this one!

jzabala commented 4 years ago

@jarretmoses I added your example to the test cases and it is working, no error is reported on the prop autoLoad. Did I get your error right?

jarretmoses commented 4 years ago

@jzabala so I don’t know if perhaps v 7.20.6 fixed something but it does seem to be working (for the most part) I do notice something now the rule has issues with nested function definitions but I’ll open a issue if I find this is not by design. Thanks for looking into this.

jzabala commented 4 years ago

@jzabala so I don’t know if perhaps v 7.20.6 fixed something but it does seem to be working (for the most part) I do notice something now the rule has issues with nested function definitions but I’ll open a issue if I find this is not by design. Thanks for looking into this.

Sure, let us know if you find anything. Thanks for your support.