objectShorthand, eslint/object-shorthand

[jpike88]

https://eslint.org/docs/latest/rules/object-shorthand

[jpike88]

@Conaclos @ematipico sorry to prod on this one, just came across the need for it and decided to check if I raised posted it here. thoughts?

[Conaclos]

noUselessRename shoudl cover a part of your suggestion.

We could add a rule useObjectMethod? However, I am wondering if this rule should also apply to functions that don't use this in their body.

const x = {
    // ok for this
-   id: function() { return this }
+  id() { return this }
    // not sure about this one (don't use `this`):
-  f: function() {}
+  f() {}
}

[Conaclos]

My reasoning is that another rule could recommend using method syntax only when the body of the function uses this and thus rewrite method that don't use this to a arrow function.

const x = {
-  zero() { return 0 },
+ zero: () => 0,
- other() { let x = 0; return 0 },
+ other: () => { let x = 0; return 0; }
}

[Conaclos]

@jpike88 any input?

[jpike88]

I think that's safe

[Conaclos]

I did some digging.

I got two distinct proposals:

1. Create useObjectMethod or useMethod (including classes) which turns any function expressions that use this and are assigned to an object property into a method.

   const x = {
   - f: function() { return this.a }
   + f() { return this.a }
     g: function () { return 0 }
   }

   This avoids conflicts with the new useArrowFunction rule.

2. Create a rule useConsistentFunctionSyntax (or useConventionalFunctionSyntax, or ...?) that incorportaes and thus replaces useArrowFunction.

   The rule suggests:

   • the use of named functions where possible

     We should ensure that the change doesn't rigger noInnerDEclarations.

   • the use of method syntax where possible

     Should we use arrow function expressions if it makes the code shorter?
     I don't think so, method syntax seems more idiomatic.

   • the use of arrow functions for other function expressions that don't use this

     This could make the following suggestions:

   - const f = function() {}
   + function f() {}
   // no change because distinct names
   const g = function G () {}
   const x = {
   - f: function() { return this.a },
   + f() { return this.a },
   - g: function () { return 0 },
   + g() { return 0 },
   -  h: () => 1,
   + h() { return 1},
   - m: () => { return 3 },
   + m() { return 3 },
   m() { return 4 },
   }

I could opt for the second proposal.
The main disadvantage is the all-inclusive nature of the rule.
I think it is fine.

@ematipico @jpike88
Any input?

[ematipico]

I don't mind both proposals. I have a couple of notes to highlight:

• the first proposal would make the code inconsistent across the board, if users opt-in for both

• the second proposal might be seen conservative and people might want to have all arrow functions or all function expressions, so it would require an option

I don't have strong options, but I'd prefer the second because that's what this rule is about: consistent code style across the codebase

[Conaclos]

Do you think the rule could be recommended?

The rule could be implemented without options. If users complain about the rule, we could add options in the future.

[ematipico]

This is my opinion: a linter should first catch bugs, wrong code, and possible optimisations. The rest should only be opt-in.

Look at what happened to eslint when they added styling rules to their recommended preset...

• prettier had to create their own config/plugin
• the creator of eslint regretted the path that eslint eventually took

[jpike88]

I agree with the second rule being a better choice if I had to choose as it's enforcing general consistency, and I'm not one to distinguish those cases too much from each other, they fit the same general issue

[Conaclos]

Look at what happened to eslint when they added styling rules to their recommended preset...

Yeah sure! However, this styling rule is not likely to be integrated in a formatter…
By the way it is fine if it is not recommended, since we have the "all" preset that is a pretty good strict preset.