sort-comp: statics out of lifecycle?

[tleunen]

https://github.com/yannickcr/eslint-plugin-react/blob/master/docs/rules/sort-comp.md

I think it would make sense to put the statics out of the lifecycle and put them in the order structure. Usually, you put the static variables at the top of your class, then your constructor and then the lifecycle functions.

[tleunen]

Ok just read the comment from #100. It seems the group "statics" from the rule is not what I think it was.

I keep this thing open, since it could be a nice enhancement. Currently, eslint tells me to put propTypes after my constructor, but it doesn't make sense for me :)

[yannickcr]

More comments on this on commit d657ea5

Two issues here:

• add support for properties types (get, set, static) in rule configuration (How to sort ES6 getters? #100)
• find a good default configuration for ES6 classes (sort-comp: constructor must be placed after componentWillUnmount #97)

[mathieumg]

I'll continue the discussion from d657ea5 in here for better visibility, I doubt many people will read that one and/or search for it.

In reply to d657ea5#commitcomment-11816207:

In second case the method type can be static, get, static get or none. In third case the property type can be static or none. The static type does not seems to be mandatory.

This could be another rule, complementary to this one, that validates/enforces the right type of method, for example defaultProps should always be static while state should never be.

Back to the topic at hand, however. I created a list for what the config "should look like" for the different syntaxes, I imagine we only need to find a way that works with as many as possible/the most common ones.

ES5

order: [
  'lifecycle',
  'everything-else',
  'render'
],
groups: {
  lifecycle: [
     'displayName',
     'propTypes',
     'contextTypes',
     'childContextTypes',
     'mixins',
     'statics',
     'getDefaultProps',
     'getInitialState',
     // Rest stays the same.
  ]
}

ES6 - external class properties

order: [
  'constructor',
  'lifecycle',
  'es-getters-setters', // Special variable, each get is paired with the same-name set if existing. 'es-getters' and 'es-setters' would be used to have them grouped by get/set instead. I imagine a variant with all of these with '-static' appended could be use to target only the static ones. Otherwise, the static ones could be placed at the beginning by default. 
  'everything-else',
  'render'
],
groups: {
  lifecycle: [
     'getDefaultProps',
     'getInitialState',
     // Rest stays the same.
  ]
}

In this case, the "static" properties get set after the class definition, is it our duty to validate their order, there?

ES6 - class methods

order: [
  'constructor',
  'lifecycle',
  'es-getters-setters', // Technically this would exclude those that are part of 'lifecycle'  but do we want that? Starting to get heavy both configuration and implementation wise.
  'everything-else',
  'render'
],
groups: {
  lifecycle: [
     'displayName',
     'propTypes',
     'contextTypes',
     'childContextTypes',
     'mixins',
     'statics',
     'defaultProps',
     'state',
     'getDefaultProps',
     'getInitialState',
     // Rest stays the same.
  ]
}

ES7 - class properties

order: [
  'lifecycle',
  'es-getters-setters', // Technically this would exclude those that are part of 'lifecycle'.
  'everything-else',
  'render'
],
groups: {
  lifecycle: [
     'displayName',
     'propTypes',
     'contextTypes',
     'childContextTypes',
     'mixins',
     'statics',
     'defaultProps',
     'state',
     'constructor',
     'getDefaultProps',
     'getInitialState',
     // Rest stays the same.
  ]
}

I guess I'm just brainstorming "out loud". 😜 Most of the time, the difficulty comes from the fact that many syntax styles can be combined/used at once, for example nothing prevents me from using a class definition with a standard getDefaultProps method and a state class property.

[josephfinlayson]

class ImageCarousel extends React.Component {
    a = "b"
   constructor(){ 
   }
   render(){}
}

Is this kind of behaviour possible to specify, all class properties/property initialisers allowed to go at the top?

[tleunen]

any update to support the ES7 static property?

[yannickcr]

I have not started working on it yet. I'll try to look at it this week.

[josephfinlayson]

@yannickcr @tleunen
We've done it at Hubrick, have a look and see if it can be ported
https://github.com/hubrick/eslint-plugin-classes/blob/master/tests/rules/order.js

[yannickcr]

@josephfinlayson Thanks! I'll have a look.

[dvdzkwsk]

Any progress on this? Just curious because my team's been running into this issue and it would be nice to have support directly from eslint-plugin-react. This plugin is awesome; having this would just seal the deal.

Thanks!

[yannickcr]

@davezuko Sorry for the delay, I'll try to work on this next week.

[satya164]

Any updates on this? This rule becomes unusable if I have flow :(

[yannickcr]

Sorry, I was busy on other things and totally forgot this issue. So no update on this for now. But I'll try to do something about this (I also accept PR ;) )

[CosticaPuntaru]

please do not forget about static function/props

[unimonkiez]

Hey, when are you expecting to publish this fix?

Btw for all interested, for the meanwhile I added this to my .eslintrc to support both sorting of react's classes and my es6 classes using this plugin -

"sort-class-members/sort-class-members": [2, {
  "order": [
    "[static-properties]",
    "[static-methods]",
    "[properties]",
    "[conventional-private-properties]",
    "constructor",
    "[lifecycle]",
    "render",
    "/^render.+$/",
    "[everything-else]"
  ],
  "groups": {
    "lifecycle": [
      "displayName",
      "propTypes",
      "contextTypes",
      "childContextTypes",
      "mixins",
      "statics",
      "defaultProps",
      "constructor",
      "getDefaultProps",
      "state",
      "getInitialState",
      "getChildContext",
      "componentWillMount",
      "componentDidMount",
      "componentWillReceiveProps",
      "shouldComponentUpdate",
      "componentWillUpdate",
      "componentDidUpdate",
      "componentWillUnmount"
    ]
  }
}],
"react/sort-comp": 0

After you change it will be something like this:

"sort-class-members/sort-class-members": [2, {
  "order": [
    "[static-properties]",
    "[static-methods]",
    "[properties]",
    "[conventional-private-properties]",
    "constructor"
    "[everything-else]"
  ]
}],
"react/sort-comp": [2, {
  order: [
    'constructor',
    'lifecycle',
    'render',
    '/^render.+$/',
    'everything-else'
  ]
}]