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

Experimental features? #1807

Closed
hornta opened this issue Jun 4, 2018 · 8 comments · Fixed by #2193
Closed

Experimental features? #1807

hornta opened this issue Jun 4, 2018 · 8 comments · Fixed by #2193

Comments

@hornta
Copy link

hornta commented Jun 4, 2018

I have made a rule that enforces you to specify propTypes and defaultProps as static class properties. I don't know if I should make a PR for this since it depends on the experimental feature. We are planning on using it in our codebase, which is large. Would it be of interest here?

INCORRECT
class MyComp extends Component {} MyComp.propTypes = {}; MyComp.defaultProps = {}

CORRECT
class MyComp extends Component { static propTypes = {}; static defaultProps = {}; }

@ljharb
Copy link
Member

ljharb commented Jun 4, 2018

I think it'd be useful if it was configurable both ways - ie, i could use it to require OR forbid implementing them as static class properties.

Also, don't forget displayName and contextTypes.

@hornta
Copy link
Author

hornta commented Jun 4, 2018

Yes. I can implement displayName and contextTypes as well.
Currently I have named the rule use-static-proptypes. But I don't really think that is a good name since it also incorporates defaultProps, and now, contextTypes and displayName.

And having one rule for each feels like it would be a bit tedious.
use-static-proptypes
use-static-defaultprops
use-static-displayname
use-static-contexttypes
Do you have any name suggestions?

@ljharb
Copy link
Member

ljharb commented Jun 4, 2018

How about react/component-static-property-style?

@hornta
Copy link
Author

hornta commented Jun 4, 2018

Sounds good!

So having "always" set for "react/component-static-property-style" forbids object property notation style and forces you to provide them as static class properties?

@ljharb
Copy link
Member

ljharb commented Jun 4, 2018

not "always" because that isn't clear what it means.

I'd expect it to take this schema:

  1. a string, either "static public field" or "property assignment" (the names can be bikeshed)
  2. an object that has keys for propTypes, defaultProps, contextTypes, and displayName, that takes the same string as above - which can override the default "everything mode" set above.

@dmason30
Copy link
Contributor

dmason30 commented Mar 6, 2019

@hornta Did you ever implement this rule in your ruleset? Although we would want to enforce the opposite :"forbid implementing them as static class properties."

@hornta
Copy link
Author

hornta commented Mar 6, 2019

No I have not. You can give it a go :-)

@dmason30
Copy link
Contributor

@hornta I have done the rule part in #2193 - not fixable yet though

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

Successfully merging a pull request may close this issue.

3 participants