Rule proposal: order of component methods

[lencioni]

Some people like to order component methods in the order of its lifecycle. This is mentioned in https://reactjsnews.com/react-style-guide-patterns-i-like/ and the example he uses is:

React.createClass({  
  displayName : '',
  propTypes: {},
  mixins : [],
  getInitialState : function() {},
  componentWillMount : function() {},
  componentWillUnmount : function() {},
  _onChange : function() {},
  _onCreate : function() {},
  render : function() {}
});

The list of lifecycle methods can be found at https://facebook.github.io/react/docs/component-specs.html

[mathieumg]

I'd love something like this as well! :+1:

[yannickcr]

I'd love this too.

Should not be too hard to make a rule for sorting React methods but I need to figure out how to handle custom methods (like _onChange or _onCreate in your example).

[tleunen]

I think the rule should only check the livecycle functions, not the custom functions.

[mathieumg]

I think the rule should only check the livecycle functions, not the custom functions.

I actually would like if it could check the custom functions as well. Something like:

// Lifecycle methods in the right order. (As mentioned above)
// Custom methods in alphabetical order.
// render() method.

[yannickcr]

Here's the idea I came up with:

In configuration we specify the valid methods order in an array, using regexp if needed (for custom methods for example):

sort-comp: [1, [
  "displayName",
  "propTypes",
  "mixins",
  "getInitialState",
  "componentWillMount",
  "componentWillUnmount",
  "^on.+",
  "render"
]]

The above configuration:

• force React methods order
• force all on* methods to be placed before render

Of course the rule come with a default configuration (to decide) so you don't need to specify the method order if the default one fits you:

sort-comp: 1 // Use default configuration

• What do you think about it ? Does it fits your needs ?
• What to do with methods that do not match any of the patterns ? Allow them anywhere ? Force them to a specific location (at the end of the component ?)
• This does force alphabetical order of custom methods as suggested by @mathieumg. Maybe we can add an additional option for this ? But I do not want to end with something too complex to configure.

[mathieumg]

I do not want to end with something too complex to configure

I understand, and while the current proposal suits our needs, minor reconfiguration is not easy. For example, if we want to keep the default ordering for the React methods, but would like to alter the custom filter, we would need to do the following:

sort-comp: [1, [
  "displayName",
  "propTypes",
  "mixins",
  "getInitialState",
  "componentWillMount",
  "componentWillUnmount",
  "^someNewPattern.+",
  "render"
]]

Not only do we need to redefine all the lifecycle methods, but a React + ensuing eslint-react-plugin version upgrade could break this configuration. The following would address that issue, but I agree that it might start to get a little too complex configuration-wise:

Default configuration:

sort-comp: [1, {
  order: [
    "lifecycle",
    "^on.+",
    "render"
  ],
  groups: {
    lifecycle: [
      "displayName",
      "propTypes",
      "mixins",
      "getInitialState",
      "componentWillMount",
      "componentWillUnmount"
    ]
  }
}]

Override in personal rules:

sort-comp: [1, {
  order: [
    "lifecycle",
    "mycustomrule",
    "^handle.+",
    "render"
  ],
  groups: {
    mycustomrule: [
      "^on.+",
      "something"
    ]
  }
}]

[yannickcr]

I'm agree redefining all lifecycle methods every time would be painful.

Your proposition is interesting, I'll look at this.

[yannickcr]

I made a first implementation in the rule-sort-comp branch. It's still WIP (no documentation, missing tests) but it should work like your proposal.

[Cellule]

I have been checking your branch and the proposal and I was wondering what happens to custom methods that do not fit any regex. Are they simply ignored ?

For instance, does the following produce an error, assuming the default options ?

class ComponentX extends React.component {
  myPrivateMethod() { ... }
  render() { ... }
  anotherMethod() { ... }
}

Also, with the regex used for lifecycle and render methods, would a custom method named renderCustom() { ... } be targeted by the rule?

[yannickcr]

what happens to custom methods that do not fit any regex. Are they simply ignored ?

For now custom methods that do not fit any regex must be defined at the end of the component, so:

class ComponentX extends React.component {
  myPrivateMethod() { ... }
  render() { ... }
  anotherMethod() { ... }
}

will trigger an error:

$ eslint example.jsx

example.jsx
  2:2  error  myPrivateMethod should be placed after render  react/sort-comp

✖ 1 problem (1 error, 0 warnings)

I don't know if I will keep this that way, any suggestion is welcome.

Also, with the regex used for lifecycle and render methods, would a custom method named renderCustom() { ... } be targeted by the rule?

Yes, but that's not intended, it's a bug. The render entry in the order array should only match the render method.

[Cellule]

Personally, I prefer when the render method is last so I can easily find it in any files. Would it be possible to setup the config like

sort-comp: [1, {
  order: [
    "lifecycle",
    "everything-else",
    "^on.+",
    "render"
  ],
}]

basically have a keyword that means, anything that don't match a particular pattern would have to be in that spot, possibly sorted or not (I would leave it to the dev to sort them however he feels). That way we could control exactly where methods not following a particular pattern would have to be.

So:

class ComponentX extends React.component {
  componentWillMount() { ... }
  myPrivateMethod() { ... }
  render() { ... }
  anotherMethod() { ... }
}

would trigger an error:

$ eslint example.jsx

example.jsx
  2:2  error  anotherMethod should be placed after myPrivateMethod react/sort-comp

✖ 1 problem (1 error, 0 warnings)

You could of course put the everything-else keyword after the render method by default and I could simply move it for my personal taste.

[yannickcr]

You should be able to match everything else with a RegExp like ^.+$ , I don't know if a keyword is relevant in this case.

[Cellule]

Maybe you're right. Then would

sort-comp: [1, {
  order: [
    "lifecycle",
    "^on.+",
    "render",
    "^.*$",
  ],
}]

trigger an error with the code

class ComponentX extends React.component {
  componentWillMount() { ... }
  render() { ... }
  onClick() { ... }
  anotherMethod() { ... }
}

Where onClick should be placed after componentWillMount? If it's fine, would it still be good if we swap "^on.+", and "^.*$", where anotherMethod should trigger an error?

[yannickcr]

Ho, I see the problem, a RegExp like ^.+$ would allow anything here, not only the methods that do not match any of the other patterns. So it is not a good solution.

Your proposal for a keyword like everything-else seems to be right solution.

[mathieumg]

Awesome! Thank you very much!

[yannickcr]

@mathieumg I followed your suggestion to make some groups and added the special group everything-else as suggested by @Cellule. So, thanks to you both :)

(It's a rather complex rule, so tell me if you find any edge cases I forgot to test)