react/display-name false positive when wrapping inline function with React.memo

[samsch]

In 7.12.3, react/display-name gives a false positive if you wrap a function directly.

import React from 'react';

const Test = React.memo(props => (
  <div>{props.children}</div>
));
export default Test;

Related: (#2105 #2109)

[ljharb]

Hmm, in this case the component is actually unnamed - and React.memo can't possibly infer the name (because that's not how JS name inference works).

In other words, this is correct as is.

[ljharb]

If you use a named function instead of an arrow function (as is best practice for components, always) does the rule pass?

[ljharb]

I'm going to close this; happy to reopen if it turns out to be an actual issue with the plugin.

[ThiefMaster]

unfortunately having to use a named function here is super ugly because it means i need to specify the function name twice when using a non-default export

const Foo = React.memo(function Foo() {
    return 'bar';
});
export {Foo};

for default exports i can do this to avoid it, and still have a proper name for the component (since it's inferred automatically):

const Foo = function() {
    return 'bar';
};
export default React.memo(Foo);

[ljharb]

@ThiefMaster simple solution there, use a default export :-D

[theFreedomBanana]

@ljharb what about if you want to add proptypes to the component? If it is default exported (so without a name), seems that you can't. One could use this S.O answer but I don't find it very elegant

[ljharb]

Indeed, in that case you have to store it in a variable first, or:

export default React.memo(Object.assign(function Foo() {
  return whatever;
}, {
  propTypes,
}));

[jwalton]

simple solution there, use a default export :-D

In latest build, this appears to be broken for a default export (although... I'm in a typescript project, so this might be a problem specific to linting .tsx files):

export default React.memo(function LabelControl(props: ControlProps) {
    const control = props.control as OnDemandLabelControl;

    return (
        <ControlWrapper>
            <Translate tagName="h2" message={control.title} />
        </ControlWrapper>
    );
});

triggers react/display-name, but:

function LabelControl(props: ControlProps) {
    const control = props.control as OnDemandLabelControl;

    return (
        <ControlWrapper>
            <Translate tagName="h2" message={control.title} />
        </ControlWrapper>
    );
}

export default React.memo(LabelControl);

does not.

[ljharb]

@jwalton if you’re on the latest plugin and that still happens, please file a separate issue for that discrepancy.

[mlodato517]

... use a named function instead of an arrow function (as is best practice for components, always)...

@ljharb this is in no way inflammatory and just for my learning - can you point me to some doc/blog/something or give a quick tl;dr for why named functions are best practice for components. Would love to learn more about this! What little I saw in my cursory googling was that because there's no display name debugging is harder and you can't use enzyme string selectors for the component name. We don't allow default exports in our project so we have export const Foo = () => <jsx /> and that seems to get us the display names.

[ljharb]

You shouldn’t be using enzyme string selectors in any case :-) debugging is harder without a name, and arrow functions only have a name via inference, which often will surprise you and not result in any name being inferred.

(You should allow default exports, but) export function Foo() {} works just fine and is what I’d recommend in that case.

[mlodato517]

Haha, don't worry, we also don't use string selectors :-) So yeah it sounds like the main reason not to use them is the sneakiness of not getting a name?

I just like keeping my eyes on "best practices" :-) Our situation here was we tried:

export const Foo = React.memo(() => <jsx />)

which violated eslint(react/display-name). We went with:

const NonMemoFoo = () => <jsx />
export const Foo = React.memo(NonMemoFoo)

before we saw this thread. So now our options seem to be:

// 1. What we have
const NonMemoFoo = () => <jsx />
export const Foo = React.memo(NonMemoFoo)

// 2. Manually set it
export const Foo = React.memo(() => <jsx />)
Foo.displayName = 'Foo'

// 3. 'One line' with named function syntax
export const Foo = React.memo(function Foo() { return <jsx /> })

// 4. Same as 1 except with function syntax
function NonMemoFoo() { return <jsx /> })
export const Foo = React.memo(NonMemoFoo)

is that right? Or is there another option I'm missing for exporting a memoized component with a display name? Would the preference be for the fourth option here?

[ljharb]

Yes, I would suggest 3 or 4. Note that 4 allows you to more easily put propTypes on the component directly, which you should always do (and which this plugin will warn you about also)

[SethArchambault]

@ljharb Curious about this last comment - I prefer option 3, though if proptypes didn't work on it, that would be a deal breaker, but I see that it seems to work fine:

export const AccountHeader = memo(function AccountHeader({ children, }) {
  return (<Text>{children}</Text>
  )
})
AccountHeader.propTypes = { children: PropTypes.string, }

I wonder if maybe this caveat was true for an older version, or is there something dangerous about this that's going to bite me down the road?

[ljharb]

@SethArchambault i don't think there's an issue with the way you're doing that, no - but conceptually i think it's cleaner to make the component, add propTypes to it, and then separately memo it.