[explicit-function-return-type] Remove / loosen rule

[swansontec]

This rule does not have an automatic fix, so adding eslint-config-standard-with-typescript to an existing project requires an immense amount of manual effort. A medium-large React site can have tens of thousands of functions / methods / callbacks, and this rule affects almost every one.

Ideally, adding Standard.js to a new project should be 90% automatic fixes, leaving the last 10% as simple spot-fixes to catch potential bugs. This rule breaks that user experience.

Plus, Typescript already knows this information, and can display it in many IDE's (just hover the function name). Therefore, this rule doesn't catch any bugs, and doesn't dramatically improve code documentation / discoverability.

Finally, the Standard.js style tends towards minimalism, eschewing implicit things like semicolons or unused constructors. Requiring explicit return types goes against this philosophy.

If removing the rule entirely seems too dramatic, it should at least be loosened to:

{
  allowExpressions: false,
  allowTypedFunctionExpressions: false,
  allowHigherOrderFunctions: false
}

[mightyiam]

I'm confused. Is that a loosening? Did you mean to use true as the value for the three options?

[LinusU]

Looking at some of my larger TypeScript codebases, I almost never specify the return type.

What is the benefit of doing this? 🤔

[swansontec]

I'm confused. Is that a loosening? Did you mean to use true as the value for the three options?

Oops, sorry. I meant true for those values.

[mightyiam]

I find that specifying the return type helps me be more clear about my intention when writing a function.

Yes, it's not about documentation, because TypeScript can infer. BTW, that is only available in an editor. And not, for example, while viewing code in GitHub. So, it does have some documentation benefit.

Imagine that you're writing a function and you don't specify the return type up front. Now, you think that you've finished writing your function, but, in fact, it does not return what you intended for it to return. You will find that out only when you attempt to use the function and something breaks. Now, if you specify the return type before writing the body of the function, you have the benefit of being certain that it does return what you intended for it to return.

Also, it makes me think clearly about what the function will return. It might sound silly, but perhaps while writing a function I go on to abstract some helper function and then come back to the function I've been writing and I forgot what I had intended for it to return. Well, if I specify the return type before writing the function, I could look at that and remember.

So, I find that it helps me in at least these two and a half ways.

[mightyiam]

Also, we do already have one of these relaxers. And I remember feeling that it's the only one we should have. But I don't remember the semantics right now.
https://github.com/standard/eslint-config-standard-with-typescript/blob/63f535fe283b56837fa1bf46721b7a3846795b9d/src/index.ts#L18-L20

[swansontec]

@mightyiam What you are describing is relevant to top-level calculation functions, like this:

function getAverage (list: number[]): number {
  // ...
}

However, Javascript has many cases where the return value literally doesn't matter. For example, Javascript features like new Promise or Array.forEach:

const x: Promise<number> = new Promise((resolve, reject) => {
  // Return value doesn't matter here,
  // since the promise type is on `resolve`
})

Node-style callbacks:

fs.readFile(someFile 'utf8', (error, contents) => {
  // Return value doesn't matter here
});

And, of course, anything involving JSX:

class MyScene extends React.Component<{}> {
  render (): React.Node {
    return <Button onPress={() => { /* return generally ignored */ }} />
  }
}

In all of these cases, typing the return value is meaningless work. If the return value does matter, the Typescript compiler will show an error at the exact place the callback is used, so the developer experience is still good.

So, I think a reasonable compromise is to only require return values on named functions, but to leave inline callbacks alone. That way, if someone can call a function from another location (using it's name), they have clear type information. Otherwise, if the function is used inline, there's no need to "document" the return type (we know what it is from the context), and Typescript will give good errors if you make a mistake.

[mightyiam]

It seems that allowExpressions is not really about allowing not declaring return type on function expressions. It seems it's about whether the function expressions are not part of an assignment.

Alright. The sweet spot does seem to be

{
  allowExpressions: true,
  allowTypedFunctionExpressions: true,
  allowHigherOrderFunctions: true
}

As you suggest, @swansontec.

Would you like to make a PR?

[swansontec]

Sure thing!

[LinusU]

Hmm, will this require us to declare return type for React functional components?

function Foo () {
  return <div>Hello<Bar /></div>
}

function Bar () {
  return <span>, World!</span>
}

and I guess we have to declare : void to e.g. setters, and event handlers?

class Foo {
  setName (name: string) {
    this.name = name
  }

  setAge (age: number) {
    this.age = age
  }
}

class Bar {
  handleClick () {
    // ...
  }

  render () {
    return <button onClick={() => this.handleClick()}>Foobar</button>
  }
}

And we also have to declare it from render-functions, even though the return type is already enforced by the class you are inheriting from:

class Foo extends Component {
  render () {
    return <span>Foobar</span>
  }
}

Might not seem like a big deal, but I think that the philosophy of Standard have been to be minimalistic, and not type out extra information that isn't necessary.

Also, it makes me think clearly about what the function will return. It might sound silly, but perhaps while writing a function I go on to abstract some helper function and then come back to the function I've been writing and I forgot what I had intended for it to return. Well, if I specify the return type before writing the function, I could look at that and remember.

While I do see the point in this, I also many people that thinks that adopting TypeScript is hard because "I have to specify types everywhere", when in fact TypeScript is really good at figuring that out automatically. I think that this might be a step that makes it more annoying to use Standard since it will require a much higher commitment than just using TypeScript.

Looking at some of my codebases, I almost never type the return value. The few places I do it is when the functions are quite large, but for the most part I strive to have many small functions and in almost all cases it's really clear what the functions return.

Forcing me to add type annotations to every function could actually be something that will stop me from adding Standard to those projects, because it will just be too much work to add that everywhere, and I think that it's duplicated unnecessary information.

Especially since there isn't an automatic fix for this rule, I think that we should be very careful in turning it on. If the barrier of adding Standard to a project is too high, most people will just not do it, defeating the purpose of Standard ☺️

[mightyiam]

You might be right about the adoption topic. Even if this rule has excellent benefits, and yet the horses won't drink, then it should be off. I would like to be quite certain that the horses won't drink. @LinusU would you turn it off? Did you try it? @swansontec same questions, please?

[swansontec]

My own project, eslint-config-standard-kit, has it turned off. It was one of the first things I did, due to the overhead of satisfying the rule.

If we were to adopt eslint-config-standard-with-typescript on some of my larger work projects, I would almost certainly turn if off in our local .eslintrc.json, perhaps with the goal of getting it turned back on again "one day". I do agree that the loosened version is a "sweet spot" in terms of value for effort, but even so it's a lot of effort.

[swansontec]

I have PR's for both versions, whatever the community decides.

[mightyiam]

Let's loosen and see whether it comes up again.

[mightyiam]

Loosened a bit more with #161.