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

justinhelmer · 2019-05-03T21:52:51Z

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

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`

The text was updated successfully, but these errors were encountered:

justinhelmer added package: eslint-plugin triage labels

May 3, 2019

justinhelmer changed the title ~~[explicit-function-return-type] Consider allowTypedFunctionExpressions=true as default~~ [explicit-function-return-type] Consider allowExpressions=true and allowTypedFunctionExpressions=true as default

May 4, 2019

bradzacher added enhancement and removed triage labels

May 4, 2019

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.

bradzacher added the breaking change label

May 4, 2019

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

bradzacher mentioned this issue

May 8, 2019

2.0.0 Breaking Changes #501

Closed

14 tasks

masahitojp added a commit to masahitojp/website that referenced this issue

Jun 21, 2019


          chore(settings): improve eslint rules for typescript

bda6f6f

see detail: typescript-eslint/typescript-eslint#493

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

bradzacher added this to the 2.0.0 milestone

Jun 28, 2019

rfgamaral mentioned this issue

Jun 30, 2019

What's the recommended return type for makeStyles() to use with TypeScript? mui/material-ui#16431

Closed

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

Abstract methods from parent class

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

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 }
  },
})