Gatsby: Don't use "export default () => {}" in docs

[gaearon]

See https://twitter.com/JelteHomminga/status/1255230778428010496.

This is a problem because those functions don't get names, so they're anonymous in React DevTools, stack traces, and upcoming features like Fast Refresh.

[marcysutton]

Thanks for filling the issue, @gaearon! Here's an example of this in the docs, for reference, mentioned in the twitter thread: https://www.gatsbyjs.org/docs/adding-react-components/#importing-react-components

[gaearon]

It would also be good to add https://github.com/benmosher/eslint-plugin-import/blob/master/docs/rules/no-anonymous-default-export.md to the default lint rule set.

[osiux]

Hi, I would like to work on this, wondering what's the preferred way to update to:

Instead of export default () => {}, should we do:

a) export default function Component() {}

or

b)

const Component = () => {};

export default Component;

I'm thinking a would be easier to change, but personally I tend to use b on my apps.

[PaulPCIO]

Hi, I would like to work on this, wondering what's the preferred way to update to:

Instead of export default () => {}, should we do:

a) export default function Component() {}

or

b)

const Component = () => {};

export default Component;

I'm thinking a would be easier to change, but personally I tend to use b on my apps.

https://twitter.com/dan_abramov/status/1255230047109222400?s=19

[wardpeet]

Let's go with export default function Component() {}. It's one less line + it will make debugging in general easier than an arrow function.

cc @pvdz

[tomadimitrie]

I don't think option A would be a great idea. Some people tend to replace old-style functions with arrow functions, and when they see they can't directly export a named arrow function in one line they will remove its name without knowing the consequences. I consider option B to be more viable.

[ackvf]

@tomadimitrie This rule will warn about it
https://github.com/gatsbyjs/gatsby/issues/23575#issuecomment-620848159

[tomadimitrie]

@ackvf yeah, but why use functions only for default exports? Arrow functions are more widespread these days, so in my opinion option B is more suitable. I may be wrong, though

[pvdz]

Yeah we're also discussing dropping constant arrows altogether as it has the same drawbacks and doesn't really help. So agreed, please change it to export function foo(){}. We're discussing to do the same thing for the Gatsby codebase, as well.

[gatsbimantico]

Version A
Will use hoisting to set the props, and create an unnecessary scope.

MyComp.propTypes = {};
MyComp.defaultProps = {};

export default function MyComp() {
    console.log(
        MyComp.name, // MyComp
        MyComp.propTypes, // {}
        this, // function MyComp
    );
};

Version B

const MyComp = () => {
    console.log(
        MyComp.name, // MyComp
        MyComp.propTypes, // {}
        this, // undefined
    );
};

MyComp.propTypes = {};
MyComp.defaultProps = {};

export default MyComp;

I would recommend version B.

[LekoArts]

@gaearon

It would also be good to add benmosher/eslint-plugin-import:docs/rules/no-anonymous-default-export.md@master to the default lint rule set.

We're using eslint-config-react-app so when https://github.com/facebook/create-react-app/pull/8926 is merged we'll bump the dependency :)

[pvdz]

• Hoisting is not a big concern
• Setting properties directly on functions is a bad pattern and prone to deoptimization
• Between const f = () => {} when you mean function f(){} I think the one saying "function" is easier to read, and a lower barrier of entry for people to read the code.

Arrows have their places, but so do function declarations.

[smmoosavi]

I would recommend version A.

export default function MyComp() {}

In typescript arrow function with generic type is not very friendly

doc generators like "API extractor" evaluate () => {} as variable not functions

I can remember typescript error about "private type exported" when the arrow function pattern used and emit declaration is on.

[eps1lon]

Setting properties directly on functions is a bad pattern and prone to deoptimization

@pvdz This is new to me considering in react we had .defaultProps for the longest time (assuming .propTypes and .displayName is stripped in prod). Do you have some resources that explain bad perf or the bad pattern in detail?

[pvdz]

@eps1lon that's a bit OT but look for posts on "expando properties". Basically adding properties to functions is changing their shape. Perfectly legal because that's how JS works but basically anything adding expandos to built-ins is frowned upon. It potentially holds back standardization and potentially hurts your perf (when used requently enough). Who knows what else.

I'll mark this (and your) post as OT.

[eps1lon]

Can we chat about this somewhere else? I don't know how built-ins relate to function components. I understand why you consider this OT but you made that argument not I.

[ackvf]

Though, it should also be noted that you can't type a function declaration to be of type React.FC or any react component type. So for example, you don't know there are children available in props.

Just some quick type hacking:

interface PropTypes {a, b, c}

const Arrow: React.FC<PropTypes> = props => props.children
function Funy(props: PropsWithChildren<PropTypes>): React.ReactElement | null {return props.children}

export type A<T> = Parameters<React.FC<T>>
export type R<T> = ReturnType<React.FC<T>>

interface P { a, b, c }
function Other(...[props, c]: A<P>): R<P> {
  return props.children
}

Funy.., not so fun. We are inclining towards named const exports instead:

export SomeComponent: React.FC<PropTypes> = ...

[muescha]

could we add a summary in some doc to style guide which notation is preferred

or add the eslint plugin (https://github.com/gatsbyjs/gatsby/issues/23575#issuecomment-620848159)?

[pvdz]

We are going to follow suit to CRA (https://github.com/facebook/create-react-app/pull/8177) and deprecate the pattern, codemod the rest to function declaration style, and lock it down with a lint. The FC deprecation will keep the existing cases for the sake of simplicity but disallow new cases.

This is a work in progress as the conversion codemod is non-trivial.