Enforce explicit return statements in arrow functions

[kachkaev]

I would like to propose a change to react/function-component-definition or a new adjacent rule.

When a component is defined as an arrow function, we can choose to use a shorthand to omit the return keyword:

const ComponentA = () => {
  return (
    <Wrapper>
      <Greeting>hello</Greeting>
    </Wrapper>
  );
};

const ComponentB = () => (
  <Wrapper>
    <Greeting>hello</Greeting>
  </Wrapper>
);

The second example may look more attractive in the beginning because is looks less ‘bureaucratic’. However, the first example ends up being much easier to change. Let's look at the diff in each case if we decide to call a hook:

  const ComponentA = () => {
+   const hello = useTranslation("hello");
+
    return (
      <Wrapper>
-       <Greeting>hello</Greeting>
+       <Greeting>{hello}</Greeting>
      </Wrapper>
    );
  };

- const ComponentB = () => (
-   <Wrapper>
-     <Greeting>hello</Greeting>
-   </Wrapper>
- );
+ const ComponentB = () => {
+   const hello = useTranslation("hello");
+
+   return (
+     <Wrapper>
+       <Greeting>{hello}</Greeting>
+     </Wrapper>
+   );
+ };

As you can see, explicit return has allowed more of the lines to ‘survive’. When JSX in the original component is quite massive, adding even a single hook (and explicit return) may create a diff that's pretty hard to digest. It also worsens git blame so it's best to prevent the situation from the start.

It'd be great to have a rule that discourages arrow function shorthands in React components. Maybe it can be a part of the existing rule, not sure.

In the meantime, I am using no-restricted-syntax to mitigate the issue:

{
  "rules": {
    "no-restricted-syntax": [
      {
        "...": "..."
      },
      {
        "selector": "VariableDeclarator > ArrowFunctionExpression > JSXElement, VariableDeclarator > ArrowFunctionExpression > JSXFragment",
        "message": "Please avoid arrow function shorthand like `(...) => <Jsx />`. Replace it with `(...) => { return <Jsx /> }`. Explicit `return` allows calling hooks later on without producing massive git diffs."
      }
    ]
  }
}

It works but cannot have an auto-fix. I guess it’s better to have an official rule instead of an arbitrary AST selector. WDYT folks?

[ljharb]

Components should never be arrow functions in the first place, which obviates this concern. Arrow functions are primarily for inline callbacks, and definitely not for anything where it's important there be an explicit (not inferred) name.

[kachkaev]

I agree with you that function ComponentA () { return <Jsx /> } is the best way to declare a component. I’ve been using this style in greenfield projects for quite some time. Despite that, notation like const ComponentA = (...) => ... is still a thing and its use is not neglectable. I work on a codebase with thousands of such components and switching over to function is not something we can practically do any time soon.

react/function-component-definition allows enforcing const ComponentA = ...:

      "react/function-component-definition": [
        "warn",
        {
          namedComponents: "arrow-function",
          unnamedComponents: "arrow-function",
        },
      ],

These options are not marked as deprecated, same for React.FunctionComponent from @types/react. Bundlers refer component names without problems so in practice the choice between the two 'styles' is mainly a matter of taste.

The main idea behind enforced return inside arrow functions is to avoid large diffs, as I have mentioned earlier. But this rule can also help teams to prepare for replacing React.FunctionComponent with function. Using no-restricted-syntax does the job for our team but I believe that there is an opportunity to help rest of the community.

[ljharb]

It's more than just taste, it's also about correctness - but indeed the option does exist on that rule.

That said, I see the "avoid diff churn" value in this option, if explained well (along with guidance to avoid using arrows for components)