[explicit-function-return-type] Consider allowExpressions=true and allowTypedFunctionExpressions=true as default

[justinhelmer]

Repro

We were disabling the explicit-function-return-type rule, until a version was released with both the allowExpressions and allowTypedFunctionExpressions options:

{
  "rules": {
    "@typescript-eslint/explicit-function-return-type": ["error", {
      "allowExpressions": true,
      "allowTypedFunctionExpressions": true
    }],
  }
}

Without these overrides, function return types must be explicitly set, even if type annotations are defined on the variable of a function expression. From what I've seen nearly all (if not all) function typedefs include the return type (specifically in the React world). For this reason, needing to define the return type becomes repetitive and cumbersome, because you then have to dig into the typedefs of the variable type to understand the return value definition (without any additional value-add).

For example:

// eslint is happy with this, with the "allowTypedFunctionExpressions" option:
import React, { FunctionComponent } from 'react';

const Foo: FunctionComponent = () => {
  return <>asdf</>;
};

export default Foo;

FunctionComponent defines the return type, so no reason to re-define it here.

// eslint is happy with this, WITHOUT the "allowTypedFunctionExpressions" option:
import React, { FunctionComponent } from 'react';

const Foo: FunctionComponent = (): ReactElement => {
  return <>asdf</>;
};

export default Foo;

This is overly-cumbersome and adds no value since the typedef for FunctionComponent already defines the return type.

The same is true for allowExpressions. For example, consider the following:

const CampaignsSummaryMemoized = useMemo(
  () => <CampaignsSummary summary={summary} />,
  [summary]
);

The useMemo typedef is set up to infer the type based on the return value of the function. We know this, because you cannot do this:

const CampaignsSummaryMemoized: number = useMemo(
  () => <CampaignsSummary summary={summary} />,
  [summary]
);

TypeScript yells, for good reason

However, without allowExpressions set to true, we must specify the following:

const CampaignsSummaryMemoized = useMemo(
  (): ReactElement => <CampaignsSummary summary={summary} />,
  [summary]
);

Must specify ReactElement instead of taking advantage of the properly-constructed type definition including the return type of useMemo

Your team may have some metrics on this, but it would be interesting to see from the community just what percentage of folks actually feel there is value in inheriting the return type from the variable definition - for both function expressions and declarations.

Please consider allowTypedFunctionExpressions=true and allowExpressions=true as the default case for @typescript-eslint/explicit-function-return-type. My assumption is that the majority of folks would then be able to use the plugin as-is out-of-the-box, without any customizations - including ourselves.

References

• https://github.com/typescript-eslint/typescript-eslint/blob/master/packages/eslint-plugin/docs/rules/explicit-function-return-type.md#allowtypedfunctionexpressions
• [explicit-function-return-type] Don't report when using an explicit variable type with an arrow function #149

Versions

package	version
@typescript-eslint/eslint-plugin	1.7.0
@typescript-eslint/parser	1.7.0
TypeScript	3.4.1
ESLint	5.16.0
node	10.15.3
npm	6.9.0

[bradzacher]

I was actually planning on making this the default with the next breaking change.

It makes a lot more sense to have all of these options turned on, esp considering that so many people assume it works out of the box for all of the cases.

[justinhelmer]

That's awesome @bradzacher. I will keep an eye on upcoming major version bumps' changelog.

[phaux]

There's a few more cases where explicit types are redundant and annoying:

1. Abstract methods from parent class

class Foo extends React.Component {
  render(): React.ReactNode /* duh */ { return <h1>Hello</h1> }
}

2. Object with functions as a function argument

// mobx-state-tree
const Foo = types.snapshotProcessor(Bar, {
  preProcessor(snapshot: BackendFoo): Something {
    return { text: snapshot.text || "" }
  },
  postProcessor(snapshot): BackendFoo {
    return { text: !snapshot.text ? null : sn.text }
  },
})

[bradzacher]

@phaux

This issue isn't a place for enhancement requests to the rule. Please raise a new issue if you'd like to discuss these.

1 - see discussion on #541
2 - feel free to open a new issue for this request.

[glen-84]

@bradzacher You may have forgotten to update the docs?

[bradzacher]

I did indeed.
Fixed 989c13a