environment-agency-austria/tslint-eaa-contrib

React render method and member-ordering

maschino opened this issue · 2 comments

Usually we have our react components render methods at the end of the class. The member-ordering rule wants the public render method to be defined before any private methods. I think we should enforce the same ordering rules for react components as we did with eslint.

Reference: eslint-plugin-react/sort-comp

Example

class Example extends React.Component<any, any> {
  private sampleMethod(): string {
    return 'foobar';
  }

  // tslint is unhappy
  public render() {
    const example = this.sampleMethod();
    
    return (
      <div>
        {example}
      </div>
    );
  }
}

There is an issue in the tslint-react repo that suggests implementing such a rule. Since that did not happen, the only way around this would be to either find a 3rd party tslint rule that supports something like regexp based member ordering or to implement such a rule ourselve.
One other solution would of course be to disable the member-ordering rule.

The general problem I see with all of these approaches is, that with JS and ESLint the situation is pretty straight forward in terms of ordering since there are no access modifiers. In my opinion, ordering the members according to the "interface" (public first, then protected, then private) makes sense and can be enforced on all .ts sources, whereas with React-specific member ordering this would only apply e.g. to .tsx sources and for .ts sources another ordering would be enforced. On the other hand I agree that it would be nice to have a way of enforcing a special member-ordering for React components - although when I think about implementing such a rule myself I can't figure out where to place "non-react-methods" in which access-modifier-order.

An easy way would be to implement a rule with member-ordering "as-is" but with ignoring members of a certain name (e.g. "render) so that they could be anywhere in a class. What do you think of that?

The problem with ignoring certain names (basically all react callbacks) is that it wouldn't enforce any rules on them. This is maybe unproblematic with one callback (render), but as soon as a more complicated component with multiple callbacks is introduced there would be no enforcement at all and the callbacks could be placed at will.

With the current eslint rule set a component is basically readable like a book. First the constructor, then all the callback methods, and at the bottom there's the render - the final product of all the callbacks so to say. It makes understanding the component relatively easy.

Maybe we should try and port the eslint rule to typescript? The AST should be similar to javascript so (without knowing any implementation details) it should be relatively straight forward? All the react callbacks are public anyway, we'd only have to think about where to put our private/protected methods. I'll play around with eslint/tslint a bit and will see if that's a viable solution.