Thoughts on JS decorators.md

Summary

JS decorators are commonly written as a prepend to the class member name, this can affect readability of the code.

As such, I propose that we should always place a JS native class decorator on a new line and place a line break between each decorated class member when there is a group of them.

class {
  @tracked
  calculatedThing;

  @action
  calcThing() {
    this.calculatedThing = this_thing + this._otherThing;
  }

  @reads('some.path.to.dependency')
  _thing;

  @reads('some.path.to.otherDependency')
  _otherThing;  
}

Preface

In the olden days, Ember's "computed macros" were used as Ember's own implementation of decorators:

{
  _thing: reads('some.path.to.dependency'),
  _otherThing: reads('some.path.to.otherDependency')
}

This format had the ergonomic side-benefit benefit of keeping the name of the property in the same position throughout the object.

However, the introduction of native decorators flips the location of the decorator from being a suffix of the class name, to being a prefix:

class {
  @reads('some.path.to.dependency') _thing;
  @reads('some.path.to.otherDependency') _otherThing;
}

Arguably, this worsens DX and causes a developer to have to scan every line to find the (arguably) most important part of the class member definition, it's name. It essentially requires that single-lines of decorated JS should be read right-to-left.

Additionally, if multiple decorators are used, things get even more weird:

class {
  @tracked @memo thing;
}

And taken further, when we adopt TypeScript, setting a decorated class member as private starts to get absurd:

class {
  @reads('some.path.to.dependency') private _thing;
  @reads('some.path.to.otherDependency') private _otherThing;
  @tracked @memo private _trackedThingy;
}

If we consider this from an information architecture perspective (which arguably informs the DX), by placing the decorator on the same line as the class member definition, we're prioritising the implementation over the name. In all other cases for a JS class, the class member name is prioritised:

class {
  someMethod() {
     this.buyMarmite();
  } 

  get thing() {
    return "marmite";
  }

  set thing(string) {
    this.thing = string;
  }
}

Proposal

With all of the above in mind, I would like to propose that we follow a simple rule:

Always place the decorator on a new line before the class member.

We already follow this rule for @action & @task because the ergonomics make sense.

To take a counter perspective, it could be possible to make this more nuanced rule, possibly something like:

If a decorator has arguments, is decorating a method, is one of multiple decorators or is decorating a TS private property, use a new line, else use the same line.

Which would result in the following examples:

class {
  @reads('some.extra.long.path.to.a.thingy')
  thingy;

  @service('serviceName')
  myRenamedService;

  @service store;

  @memo
  @tracked
  thing;

  @tracked thingy;

  @tracked
  private _thingy;

  @attr('string')
  someModelVal;
}

However, whilst this more nuanced rule genertaes slightly more succinct examples, it is more complex to enforce and will require more effort to follow and traverse during implemenation & PR reviews.

The simpler rule, whilst making our classes slightly longer, requires less effort to abide by.

Drawbacks

Grouping

Placing the decorator & the class member on the same line allows us to group together many decorated properties (such as a group of @read properties):

class { 
  @reads('some.path.to.dependency') _thing;
  @reads('some.path.to.otherDependency') _otherThing;
  @reads('some.path.to.otherDependency.more') _yetMoreThing;
  @reads('some.path.to.otherDependency.evenMore') _evenMoreThing;
}

Which can help to keep the wiring up of dependencies succinct. If we instead follow this proposal, we will need to enforce a new line seporator, otherwise things begin to deteriorate:

class { 
  // Hard to read
  @reads('some.path.to.dependency')
  _thing;
  @reads('some.path.to.otherDependency')
  _otherThing;
  @reads('some.path.to.otherDependency.more')
  _yetMoreThing;
  @reads('some.path.to.otherDependency.evenMore')
  _evenMoreThing;

  // Easier to read
  @reads('some.path.to.dependency')
  _thing;

  @reads('some.path.to.otherDependency')
  _otherThing;

  @reads('some.path.to.otherDependency.more')
  _yetMoreThing;

  @reads('some.path.to.otherDependency.evenMore')
  _evenMoreThing;
}

This is annoying/shut up Lou 😝

This could be seen as just another annoying rule to follow, and possibly one which we should avoid bike shedding and instead just use Prettier.

However, there is no (current) Prettier formatter for this. There is at least one WIP Prettier which we could adopt in future if it suits our needs, but until then, this feels like something we need to manually address ourselves.