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

Rule proposal: Check for unused propTypes #226

Closed
EvHaus opened this issue Sep 23, 2015 · 9 comments
Closed

Rule proposal: Check for unused propTypes #226

EvHaus opened this issue Sep 23, 2015 · 9 comments
Labels

Comments

@EvHaus
Copy link
Collaborator

EvHaus commented Sep 23, 2015

While refactoring code you may remove the use of some props but forget to remove them from the propTypes. It would nice if ESLint could alert you with:

`foo` propType is defined but not used.

Similar to the unused vars rule.

@FirstWhack
Copy link

+1 this seems necessary since we check for props missing a proptype definition we should also check for proptypes never being used

@ljharb
Copy link
Member

ljharb commented May 19, 2016

All +1's should now be in the form of reactions on the first post - I'll clean up the ones here, and any future ones. Thanks for everyone's interest!

@EvHaus
Copy link
Collaborator Author

EvHaus commented May 22, 2016

I'm going to take a stab at writing this rule. Will submit a PR.

@lo1tuma
Copy link

lo1tuma commented May 22, 2016

What should happen if the whole props object is passed to an external third party function? Or if you are using the spread operator to pass-through all props to a child component? I guess in such cases the rule should not warn.

@silvenon
Copy link
Contributor

Yeah, unfortunately I think this rule is prone to false-positives.

@jamesdwilson
Copy link

Maybe we could just have it have options to issue warning or not if props object is passed to a third party function or using a spread operator? Instead of throwing the baby with the bathwater.

@EvHaus
Copy link
Collaborator Author

EvHaus commented May 23, 2016

I've done a first pass at this here: #611 (Instructions on how to test and install are in the PR).

Still hunting down false positives, but early feedback would be highly appreciated.

@mattdell
Copy link

mattdell commented Jul 5, 2016

Any update on this? Is there something preventing this from being merged?

@CrazyAlvaro
Copy link

@EvNaverniouk Thank you for add this rule here, Evgueni. During our development here, we found that this rule checks propType used under this.props but not under nextProps. In some cases we only need to update specific props by using nextProps.foo. Would it be nice if we could check on both this.props and nextProps?

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

No branches or pull requests

9 participants