explicit-function-return-type should not be recommended

[fcole90]

What version of this package are you using?
Latest (ATTOW 21.0.1), installed as follows:

yarn add --dev  \
    typescript@4.4.4  \
    eslint@^7.12.1 \
    eslint-plugin-node \
    eslint-plugin-n@^15.0.0 \
    eslint-plugin-import@^2.25.2 \
    eslint-plugin-promise@^5.0.0 \
    @typescript-eslint/parser@^5.0.0 \
    @typescript-eslint/eslint-plugin@^4.0.1  \
    eslint-config-standard-with-typescript@latest

What problem do you want to solve?
The rule explicit-function-return-type has dire consequences on code maintainability conciseness and cleanliness. This specific rule was once recommended by typescript-eslint but was since removed typescript-eslint/typescript-eslint#2603 , hence we could even argue that this behaviour is now against the standard, which I think is the aim of this project. The thread shows some crucial pros and cons of the rule.

This rule works well for small functions, and I understand why it was put in place, but it quickly becomes cumbersome when developing with React, React Query and other libraries with complex types, which need to be stated and restated again in every function call. This requirement may be detrimental, as it leads a developer to be much more tempted to work with less abstraction, even when it would have been beneficial. This is especially true in the case of wrappers and decorators, that need to restate all the types of the wrapped function, instead of letting typescript correctly infer them.

What do you think is the correct solution to this problem?
I think the correct solution is to remove the rule entirely while ensuring that a function can never return any.

Are you willing to submit a pull request to implement this change?
Certainly! Here you are: #861

[mightyiam]

Thank you for the PR. We have a top priority task at the moment (#840). Everything else is on hold. We hope to address this soon enough.

[LinusU]

This is very interesting, I'm a bit conflicting on this.

When I first turned on ts-standard on our codebase this was definitely the rules that was broken the most. But on the other hand I really think that the quality of our codebase improved when we started conforming to it...

[slushnys]

This is very interesting, I'm a bit conflicting on this.

When I first turned on ts-standard on our codebase this was definitely the rules that was broken the most. But on the other hand I really think that the quality of our codebase improved when we started conforming to it...

It's great you managed to gain benefits from it @LinusU, but in some cases implicit return types are enough with the option that you can specify more advanced return type instead of being forced to be explicit all the time. @fcole90 made some good points as well. Since continuous delivery has been already closed - what going to happen with this case?

[mightyiam]

1. Granted, this rule is impactful.
2. This package has some user base, whom I suppose are mostly happy with this rule.
3. Removing this rule would probably be against their expectations. It would be a violation of the nature of this package, one could say. Therefore, it should probably remain.

[slushnys]

"Happy with this rule" may have been an enforced feeling. There was only one way to avoid this, that is to overwrite this rule in their configuration. I would be really curious to see how many actually did just that. It is impactful, to an extend that people won't be forced to write return types where they don't need them. I guess we need to find out the user base expectations for this.

I personally find it unnecessary to import types from external modules when I try to return the result of a called function thats defined in that module while typescript infers that for me. Thats probably just one of the use cases where this rule puts a stick in the wheel. I would like to hear the thoughts of others on such cases and why we shouldn't allow typescript do its job, rather doing that manual labor ourselves.

[fcole90]

Following the history of how this rule got into this package, it seems to me that it came from an earlier recommendation from typescript-eslint. However, in the meanwhile, people at typescript-eslint decided to overturn it.

This package has some user base, whom I suppose are mostly happy with this rule.

Not necesarily. I generally like this package, and it provides so many benefits, even if this rule can really hinder readability and make you less efficient. Hence, many users might as well be using this package despite this, not because of this. I have talked to people that were happy with this package overall, but much less with this rule.

Another consideration is that the current user base is a subset of the potential user base this package would potentially have had: it consists of those who can bear this rule, the others just walked away. [Read more]

So we should also think, who is this package for?

If this package should be a standard, as the name implies, then I think it should aim at most typescript users. However, we already know that typescript-eslint defaults with this rule disabled (for the reasons mentioned before), and we know that typescript-eslint has a much larger user base (I think it's the go-to linting package for typescript) than this.

This, toghether with their motivations for the change, makes me think that this package would be much closer to a standard by not enforcing this rule 😊

[fcole90]

I personally find it unnecessary to import types from external modules when I try to return the result of a called function thats defined in that module while typescript infers that for me.

Yes, and this is what this rule force people into, and is exaclty what is recommended not to do in typescript. If we type correctly, we should need less typing, not more 😊

[mightyiam]

Would anyone like to prove or disprove this issue by querying repositories on GitHub?

Until we have some evidence, I'll close this issue so that it does not show up for me while working.

[fcole90]

Not really, I have a feeling there is not much will for any change of this sort by you, @mightyiam , and the fact you closed this while we were still discussing the issue strongly hints that there's little room for discussion about it 😞

[mightyiam]

@fcole90 I don't see the point in discussing this further without gathering numbers. This project serves all of its users — not only a vocal few.

The preference of the users must be obtained. The number of upvotes on this issue does not seem to indicate a good representation of the user base. Not good enough to compel this change, does it?

[fcole90]

I agree that getting some real numbers is a good idea and the way to go 👍 But I also think that keeping this open signals that we are still considering this, waiting to get such numbers 😊 Closing it as not planned gave me the impression you had no wish for further work on this regardless 😕

[fcole90]

😊

[slushnys]

Since version 23.0.0 there has been quite some breaking change introduced. Your argument @mightyiam was that this change talked about in this issue would be "impactful", but then again this was introduced that could cause quite some change for people using the package. Did you gather up the numbers from other on that or is this because upstream standard has a bigger factor than the official typescript-eslint rules? Thanks for the answers

[LinusU]

To share one datapoint, when we updated our ~40k SLOC backend GraphQL API from ts-standard 10 to 11, the only rule that affected us was the ternary indentation rule which was also part of regular standard.

[mightyiam]

@slushnys you're right. The best argument would be the removal of a rule is not even a breaking change.

@LinusU, how much extra work is it to explicitly specify return types in ~40k SLOC? And how often did you feel like it's not worth it to specify a return type?

[LinusU]

Not that much I'd say, in the vast majority of cases it comes naturally. The only times we feel it's not worth it is in small functions that just calls another function, but for us that wouldn't justify removing the rule in our project.

It's nice in that it catches when one has e.g. missed return from one branch.

[slushnys]

Hi there, it's been some time and I guess one reason this doesn't get any attention is because it's closed - can you reopen this please?

Also, is an argument "that it comes naturally" a good reason to not do it? Other PR's went through including breaking changes that others have been affected more than this would. Removing this rule so it won't "error" won't trigger anything breaking - projects that had the return types defined will still be able to have them. People who don't - will have the option to allow typescript to automatically infer the return types which is the recommended way. Projects using this standard may not even see this since it leaves us an option to specify types whenever we prefer or have a more complex situation that we want to outline if any. Removing this rule gives more flexibility and follows the recommended patterns which this "standard" is trying to follow, right?

[mightyiam]

What is this recommendation about not specifying return types I keep hearing? Is this from some blog post from someone on the TypeScript team or what?

[slushnys]

I'm at fault for not specifying what I mean in a concise way. Before, we had a rule in typescript-eslint that was removed due to not giving any benefit as mentioned in this issue: typescript-eslint/typescript-eslint#2603

Typescript has a powerful enough engine to infer types most of the time, e.g. here https://www.typescriptlang.org/docs/handbook/2/functions.html

Theres a lot to read about inference, but a lot of cases where we specify return types are when its required due to some generics, writing a specific interface or overloads.

All in all, it's already covered by typescript so this rule is unnecessary.

[theoludwig]

I'm at fault for not specifying what I mean in a concise way. Before, we had a rule in typescript-eslint that was removed due to not giving any benefit as mentioned in this issue: typescript-eslint/typescript-eslint#2603

Typescript has a powerful enough engine to infer types most of the time, e.g. here https://www.typescriptlang.org/docs/handbook/2/functions.html

Theres a lot to read about inference, but a lot of cases where we specify return types are when its required due to some generics, writing a specific interface or overloads.

All in all, it's already covered by typescript so this rule is unnecessary.

I disagree, this rule is necessary.

Even if TypeScript is sometimes "smart" enough to infer the return types, it makes clearer what the code does. Remember, code is not only read by machines or by the TypeScript type checker but also by humans.

Also enabling this rule bring consistencies, you don't even need to think about it, it should be a habit, why sometimes you'll specify the return type and sometimes not?
In most typed languages, you are forced to explicitly specify the return type else it won't even compile (C/C++, Java, C#, etc.).

All in all, don't let your code be a guessing game to guess the type.
We're writing TypeScript after all, not JavaScript, so we write types.
The more there are types, the better.

[mightyiam]

I tend to agree with the policy of "just define types" instead of "carefully decide where to define types" that @divlo presented.

And I like the example of other, statically typed languages.

The down-side is obviously the need to specify types. And I understand this.

In Rust — which is statically typed — for example, the signatures for closures are optional, if they can be inferred!
I wonder whether such an option exists in this rule... doesn't seem like it...

[fcole90]

I'm at fault for not specifying what I mean in a concise way. Before, we had a rule in typescript-eslint that was removed due to not giving any benefit as mentioned in this issue: typescript-eslint/typescript-eslint#2603
Typescript has a powerful enough engine to infer types most of the time, e.g. here https://www.typescriptlang.org/docs/handbook/2/functions.html
Theres a lot to read about inference, but a lot of cases where we specify return types are when its required due to some generics, writing a specific interface or overloads.
All in all, it's already covered by typescript so this rule is unnecessary.

I disagree, this rule is necessary.

Even if TypeScript is sometimes "smart" enough to infer the return types, it makes clearer what the code does. Remember, code is not only read by machines or by the TypeScript type checker but also by humans.

Also enabling this rule bring consistencies, you don't even need to think about it, it should be a habit, why sometimes you'll specify the return type and sometimes not? In most typed languages, you are forced to explicitly specify the return type else it won't even compile (C/C++, Java, C#, etc.).

All in all, don't let your code be a guessing game to guess the type. We're writing TypeScript after all, not JavaScript, so we write types. The more there are types, the better.

I think writing explicit types is a good practice and I think it's very useful for documentation purposes as well. Nevertheless, types need also to be written and understood by humans, so I think they add value as long as they don't get in the way of writing an understanding code. Typescript is not like C++ or Java, it uses structural typing and inherits a lot of complexity from JavaScript, so some types make large use of generics and are very hard both to define and read, and even more to update once they have been written down.

I opened this issue when working with ReactQuery (now TanStack) because writing down the return types was taking more time and effort than writing the actual code (you can see an example of such complex generics here https://tkdodo.eu/blog/react-query-and-type-script#the-four-generics). There are many such situations when writing typescript code, and in such cases, the level of complexity of redefining the type is so high that it only adds struggle to the developers writing them, but little value for those reading the code.

In a perfect world there would be some level of customisation for this rule so that we would have mandatory return types for "simpler" types (or when a type cannot be inferred), but no longer mandatory when such complex types can be statically inferred already.

For all these reasons, which actually are also the reasons why this rule is no longer a default in typescript-eslint (see my earlier comments), I think enforcing this rule is a mistake.

[slushnys]

Cycling back to see what the status of this is and what's the holdup @mightyiam, since this isn't a breaking change whereas a major braking change was introduced previously. I don't see how this should become stagnant and not accepted since the package follows the "recommended" approach as per the breaking changes that happened.

[mightyiam]

This issue is similar to other issues that we get complaints for. This is about exceptions to the rule. In most cases, this rule is helpful and most would agree that explicit return types are a good practice. Vast majority, it seems.

Regarding the comparison with typescript-eslint's recommended config: when I started this project I wanted to seize the opportunity to set the bar high regarding strictness of type checking. So, intentionally, this project provides a rule config that is stricter than the one typescript-eslint provides. Also, I can't testify regarding the considerations in that project and specifically why it tends to be more lax with its recommendations.

Back to exceptions to the rules: there are several ways to make exceptions and in essence tell ESLint "leave this alone, I know what I'm doing". One is the eslint-ignore and its variants. Another is to have ESLint config files in subfolders where they would apply to the sub tree of files therein. Another is with pattern matching overrides.

So, @rostislav-simonik and I feel that this issue is to be closed with this conclusion: you can use exceptions/ignores where appropriate. Otherwise, this rule is desired in the vast majority of functions.

Please feel free to try to change our minds.