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

[new] add static-property-placement rule #2193

Merged
merged 1 commit into from Apr 12, 2019
Merged

[new] add static-property-placement rule #2193

merged 1 commit into from Apr 12, 2019

Conversation

dmason30
Copy link
Contributor

@dmason30 dmason30 commented Mar 10, 2019

This adds a rule discussed in these two issues:

I have based the options on @ljharb suggestions from both issues. This is my first attempt at writing a rule so I expect plenty of feedback.

  • static public field (ClassProperty)
  • static getter {MethodDefinition)
  • property assignment (MemberExpression)
...
"react/static-property-placement": [<enabled>, <string>, {
  childContextTypes: <string>,
  contextTypes: <string>,
  contextType: <string>,
  defaultProps: <string>,
  displayName: <string>,
  propTypes: <string>,
}]
...
  • Also I have never worked with TypeScript/Flow so I am not sure what (if any) additional changes are needed?

  • This rule only applies to ES6 classes.
    -- Ignored: Stateless functional components can only ever have a MemberExpression to declare any of the above properties.
    -- Ignored: The React.createClass/createReactClass can only ever have a Property to declare any of the above properties.

  • This does not have autofix either but I feel that is a "nice to have" option and could come later.

@dmason30

This comment has been minimized.

@dmason30 dmason30 mentioned this pull request Mar 11, 2019
@ljharb ljharb marked this pull request as ready for review March 12, 2019 06:36
Copy link
Member

@ljharb ljharb left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'd also like this form to be incorrect code:

class Foo extends React.Component {
  static get defaultProps() {}
}

So perhaps we need a third option, static getters?

docs/rules/static-property-placement.md Outdated Show resolved Hide resolved
docs/rules/static-property-placement.md Outdated Show resolved Hide resolved
docs/rules/static-property-placement.md Outdated Show resolved Hide resolved
lib/rules/static-property-placement.js Outdated Show resolved Hide resolved
lib/rules/static-property-placement.js Show resolved Hide resolved
lib/rules/static-property-placement.js Outdated Show resolved Hide resolved
lib/rules/static-property-placement.js Outdated Show resolved Hide resolved
lib/rules/static-property-placement.js Outdated Show resolved Hide resolved
@dmason30
Copy link
Contributor Author

dmason30 commented Mar 12, 2019

Added the static getter option - Great Idea 🎉

I await your feedback.

@dmason30
Copy link
Contributor Author

@ljharb LGTM?

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