react/function-component-definition configuration violates style guide

[billyjanitsch]

In eslint-config-airbnb@19.0.0 (specifically 1bc8cab), react/function-component-definition was configured to allow only function expressions for named components:

javascript/packages/eslint-config-airbnb/rules/react.js

Lines 526 to 531 in 1bc8cab

	// https://github.com/yannickcr/eslint-plugin-react/blob/master/docs/rules/function-component-definition.md
	// TODO: investigate if setting namedComponents to expression vs declaration is problematic
	'react/function-component-definition': ['error', {
	namedComponents: 'function-expression',
	unnamedComponents: 'function-expression',
	}],

This has the effect of disallowing function declarations:

function Foo() {
  return <div />
}

While allowing only this style:

const Foo = function () {
  return <div />
}

This seems like a configuration mistake, as the React style guide consistently uses function declarations for components (as does the ecosystem as a whole) and specifically points out that "relying on function name inference is discouraged". Let me know if you agree and I'll PR a fix.

[billyjanitsch]

Oops, I searched for existing issues but not PRs. One is already open to fix this: #2501.

[slifty]

Wanted to check: will this end up being corrected at some point?

There are a few thousand folks who seem to be facing issues when trying to write react under the current (9.x) version of the styleguide (this rule combined with rules against anonymous functions results in pretty verbose / un-intuitively repetitive component definitions).

[ljharb]

@slifty yes, it will be corrected soon. btw that seems like a lot of people going to the wrong place; stack overflow isn’t the place to report problems with any software. As far as this issue can tell, there’s only 12 people, not thousands.

[slifty]

@ljharb Awesome thank you (and for all the amazing work you do).

I hear you on the StackOverflow concern! Next time I see this kind of scenario I'll more explicitly suggest commenting on the relevant issue in the answer.

[vicasas]

The default rule keeps forcing me to use this

function HelloWorld() {
  return <h1>Hello World</h1>
}

instead of this

const HelloWorld = () => {
  return <h1>Hello World</h1>
}

And It no longer suggests the implicit return for arrow functions.

Using the following versions:

{
   ...
   "eslint": "^8.6.0",
   "eslint-config-airbnb": "^19.0.4",
   ...
}

[ljharb]

@vicasas the styleguide has always required components be normal, non-arrow functions.

[vicasas]

@ljharb In earlier versions there is no such problem when writing components using arrow-functions.

[mryechkin]

Running into the same problem as @vicasas after upgrading to v19 - wasn't an issue on earlier versions

[ljharb]

The guide has always required it. The config failed to do so until recently.

Components should never be arrow functions, so this change is by design.

[mryechkin]

The guide has always required it. The config failed to do so until recently.

I see - thanks for clarifying. I'm in agreement with this comment then that this is something that should've been included in the changelog. Much like that user's company, we've been extending this config for several years now and this is the first time this has come up, we got dinged with a bunch of new errors after upgrading. Not a big deal to override it, just would've been nice to be aware of it prior to upgrading.

Components should never be arrow functions

Curious as to why?

[ljharb]

re the changelog, I'll try to do a better job highlighting breaking changes in future releases.

Why? For a number of reasons:

• debugging. arrow functions can never have explicit names, only implicit ones via ES6 name inference, which can be unintuitive and surprisingly missing at times. Consider when you have export const Foo = (props) => {}; and then refactor that to be export const Foo = React.memo((props) => {}) - your "Foo" name suddenly vanishes, because you were relying on inference, probably without realizing it.
• conceptual mismatch. arrow functions are conceptually meant to replace inline callbacks. They are not a general-purpose function replacement, even though they obviously can be used as such. An arrow function doesn't properly convey the importance/noun-ness of a react component.

See also:

• https://twitter.com/dan_abramov/status/981618859324166146
• https://twitter.com/dan_abramov/status/1093901838179618816

[vicasas]

@ljharb I understand. I join the suggestion of @mryechkin about being able to show or warn about these changes to take them into account. The guide is so long that it is very difficult to learn each of the rules, which is why many developers trust the plugin 100%.

Reviewing the tweets, Dan Abramov indicates that it is equally valid to use arrow-functions to declare components, the important thing is that it is named.

[ljharb]

@vicasas yes, and I’ll do a better job with the changelog next time. The change would have happened either way - the guide has never permitted arrow function components.

Dan’s tweets mean that if you use arrow components, you have to keep a lot more JS trivia in your head about name inference.

[vicasas]

@ljharb As a matter of doubt, if the default guide requires components to be functions, how would it be done for components that require sending ref, would it be ok to use the arrow function to make it forwardRef?

[ljharb]

@vicasas forwardRef doesn't take a component, but it does need a name, so it should still always take a named normal function.

[vicasas]

@ljharb Do you mean something like this?

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

export default React.forwardRef(Hello)

Instead of this?

const Hello = React.forwardRef((props, ref) => {
    return <div>hello</div>
})

export default Hello

[ljharb]

@vicasas no; you can define the function inline as a normal function:

export default React.forwardRef(function Hello(props, ref) {
    return <div>hello</div>
});

[vicasas]

@ljharb Eslint modifies it to this form.

export default React.forwardRef((props, ref) => <div>hello</div>)

[ljharb]

@vicasas ah, the prefer-arrow-callback rule might need to be adjusted or overridden for that one. In that case, having them be separate is ideal.

[vicasas]

@ljharb Do you mean to do it this way?

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

export default React.forwardRef(Hello)

[ljharb]

@vicasas yes, altho it's not actually a component (so i'd probably use a non-PascalCased name, like hello), and if you're not using the ref argument, it shouldn't be using forwardRef :-)

[Dannymx]

Is this still broken? If I define an arrow-function named component eslint doesn't enforce other rules like return type.

[ljharb]

@Dannymx "return type" isn't something eslint enforces; maybe you mean typescript? This guide requires non-arrow functions for components.

[namlq93]

The default rule keeps forcing me to use this

function HelloWorld() {
  return <h1>Hello World</h1>
}

instead of this

const HelloWorld = () => {
  return <h1>Hello World</h1>
}

And It no longer suggests the implicit return for arrow functions.

Using the following versions:

{
   ...
   "eslint": "^8.6.0",
   "eslint-config-airbnb": "^19.0.4",
   ...
}

How can I turn off this rule? I want to use function-declaration or arrow-function both are acceptable.

[mahyarmirrashed]

I am still having a problem with this relatively simple snippet:

import React from 'react';

const App = () => <div>Hello, world!</div>;

export default App;

with the following runtime configuration:

env:
  browser: true
  es2021: true
extends:
  - plugin:react/recommended
  - airbnb
parserOptions:
  ecmaFeatures:
    jsx: true
  ecmaVersion: latest
  sourceType: module
plugins:
  - react
rules: {}

My understanding was that this PR was supposed to remove the necessity for specifying specific rules to allow these sorts of arrow functions.

[ljharb]

@mahyarmirrashed that understanding is not correct. This guide does not allow components to be arrow functions, so you'd have to override the rule definition to permit them.

[maapteh]

@namlq93 for example you can use in rules:

  rules: {
    // not sure if this is a "bug" or "feature" in airBnB config? https://github.com/airbnb/javascript/issues/2505
    'react/function-component-definition': [
      'error',
      {
        namedComponents: ['function-declaration', 'arrow-function'],
      },
    ],