Should we use React component method sort order?

[jstcki]

Seems a bit over-pedantic but maybe it's useful. I've just included the sort order like it's defined in the AirBnB config. Should we keep it?

{
    // ...
    'react/sort-comp': [2, {
      'order': [
        'displayName',
        'mixins',
        'statics',
        'propTypes',
        'getDefaultProps',
        'getInitialState',
        'componentWillMount',
        'componentDidMount',
        'componentWillReceiveProps',
        'shouldComponentUpdate',
        'componentWillUpdate',
        'componentWillUnmount',
        '/^on.+$/',
        '/^get.+$/',
        '/^render.+$/',
        'render'
      ]
}

[tpreusse]

:+1: for keeping!

It is pedantic but in a good way :) The annoying thing is maintaining it with the ever changing React component structure – React.createClass -> ES6 Classes -> Pure Fns :wink: (not sure how lifecycle triggers will be defined in pure fns)

[jstcki]

Okay, let's keep it for now.

[mhintz]

FWIW, I'm against it, because I think it's pedantic but in a bad way, but I think the window of opportunity for commenting on this has closed. I also don't mind that much

[mhintz]

Reopening this issue, I propose changing the config to the following:

'react/sort-comp': [1, {
'order': [
    'everything-else',
    'lifecycle',
    '/^on.+$/',
    'render'
  ]
}]

You can see what 'lifecycle' implies for ordering of arguments here.

For one, that would change this from an error into a warning, for two, it would place the component-specific functions at the top of the declaration.

[mhintz]

Of course, I propose this mostly for your convenience, in case anyone else feels similarly

[jstcki]

I'd add '/^handle.+$/' to that list?

And I prefer lifecycle before everything-else.

So I guess I'd just use the default order 'lifecycle', 'everything-else', 'render' :grin:

[tpreusse]

I don't see any benefit in changing the order and I'm still in favor of keeping the rule. If it's a warning or error also doesn't make a difference to me – I guess that would only matter if we integrated it into the build step and prevent deploys of code with lint errors.

Refactoring to the new special groups: Sure but lets keep the order the same.

[jstcki]

So, I guess ['lifecycle', 'everything-else', 'render'] wouldn't break existing code which adhered to the more strict sort order we had before. Unless you have some other methods placed after render (which I like always being last).

I like the lifecycle ordering, because the methods are called roughly in that order anyway.

Maybe this is be a good case for writing some tests :smile:

[jstcki]

Will change it in v3. Superseded by #10