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

no-unused-prop-types - count all properties as used when met {...this.props} #879

Closed
victor-homyakov opened this issue Oct 3, 2016 · 10 comments

Comments

@victor-homyakov
Copy link

Suppose one of components has a bunch of properties, and just passes them to child component <SomeChild {...this.props} />. In this case no-unused-prop-types rule will mark all properties as unused. Although rule could be manually disabled for the component, I'd like to see that rule can handle that situation, maybe with an additional option in configuration.

@ljharb
Copy link
Member

ljharb commented Oct 3, 2016

That goes far beyond what a linter is capable of. Spread props are dynamic at runtime and defy static analysis.

@victor-homyakov
Copy link
Author

  • linter could just mark all properties as used when it sees {...this.props}
  • linter could use sort of weak warning or info when some property is unused explicitly but is passed to child component with {...this.props}, e.g. "Potentially unused property foo" or "Property foo is not used explicitly"

@ljharb
Copy link
Member

ljharb commented Oct 4, 2016

The rule's severity is all or nothing - either it's an error (when the rule is configured as an error), or it doesn't report at all.

@victor-homyakov
Copy link
Author

IMO configured severity is sort of upper bound, and linter may report some issues with lower severity

@ljharb
Copy link
Member

ljharb commented Oct 4, 2016

@victor-homyakov that's not how eslint works. Rules only have one "report" function, and everything matches the configured severity.

@victor-homyakov
Copy link
Author

:(
OK, what about adding new option to silence this rule when spread operator {... this.props} is used?

@ljharb
Copy link
Member

ljharb commented Oct 4, 2016

I think only when the entire props object is spread, it makes sense to mark all properties as used.

@SavePointSam
Copy link

I hope there is a chance this could be explored deeper.

The only false positives I get anymore are from using { ...this.props }.

@tylerthehaas
Copy link

I am also running into this. Having to turn off that rule around my proptypes fairly frequently.

@DianaSuvorova
Copy link
Contributor

@ljharb this issue can be closed. Fixed in #1303.

The test case discussed in the thread is line 1319 here

@ljharb ljharb closed this as completed Jul 21, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

No branches or pull requests

5 participants