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/no-unused-prop-types cannot find props being used inside helper functions #1135

Closed
renanvalentin opened this issue Mar 31, 2017 · 3 comments · Fixed by #1218
Closed

Comments

@renanvalentin
Copy link

const showAlert = props => **props.status** === PENDING;

const Alert = (props) => {
  if (showAlert(props)) {
    return (
      <div />
    );
  }

  return <noscript/>;
};

Alert.propTypes = {
  **status**: PropTypes.oneOf([PENDING, ACCEPTED, CANCELED])
};

In this case props.status is being used inside of the function showAlert.

@ljharb
Copy link
Member

ljharb commented Mar 31, 2017

The proper way to handle this is not to pass the entire props object around. It's not practical or often possible for the plugin to follow references, and the only thing that should get a props object is an element.

In other words, pull status out prior to passing it to the function.

@MethodenMann
Copy link

MethodenMann commented Apr 3, 2017

ok i dont have exactly the same situation. This is my simplified component:

import React from 'react'

const MyComp = props => {
  const locales = ['de', 'en']

  const handleFlagClick = locale => {
    props.onLocaleChange(locale)
  }
  return (
    <div>
      {
        locales.map(locale => (
          <img
            key={locale}
            onClick={() => handleFlagClick(locale)}
          />
        ))
      }
    </div>
  )
}

MyComp.propTypes = {
  onLocaleChange: React.PropTypes.func
}

export default MyComp

And i get the following linting error:

24:19 error 'onLocaleChange' PropType is defined but prop is never used react/no-unused-prop-types

I figured out its because of the map. It this by design?

@jseminck
Copy link
Contributor

jseminck commented May 24, 2017

Hello,

I ran your latest test case in a PR where we fixed the following issue: #1183 - and in there the test case passed! Prior to the changes from that specific PR, the test case was failing.

Removing the .map() code in the render() would make the component valid under the current implementation.

So I am quite confident those changes are also fixing the issue that you see here. And yes I would think that this to be a bug. Do others agree?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging a pull request may close this issue.

4 participants