deephaven / web-client-ui

Deephaven Web Client UI
Apache License 2.0
29 stars 31 forks source link

Add instance-variables to eslint react/sort-comp rule #375

Open mattrunyon opened 2 years ago

mattrunyon commented 2 years ago

Currently, our eslint config requires instance variables to be defined after the constructor and lifecycle handlers like below.

constructor() {
  this.myProp = doSomething();
}

onComponentDidMount() {
  doSomethingElse();
}

myProp;

If we add instance-variables to the order before lifecycle, then this should require instance variables be defined before the constructor and after static members like below. This only applies to jsx/tsx files.

myProp;

constructor() {
  this.myProp = doSomething();
}

onComponentDidMount() {
  doSomethingElse();
}

It will require some manual fixing as the rule doesn't have auto-fix.

mattrunyon commented 2 years ago

Adding this rule affects many memoized getters we have b/c they are set using instance variable notation and not instance methods. E.g. getCachedItems = memoize(...);. I don't think there's really a way around this b/c the memoized function needs to be initialized w/ the class. So the alternative would be initializing all the memoized functions in the constructor which is not a good choice

We could instead use type-annotations here with the main downside there that any instance variable initialization would be put after lifecycle methods. E.g. myItems: Item[] would be expected before the constructor, but myItems: Item[] = [] would be expected after lifecycle methods.