Fix false positives for display-name

[BPScott]

Wrapping a named function declaration with a React.memo or
React.forwardRef will no longer throw false positive errors.

Check how React devtools exposes the displayNames of components wrapped in memo and forwardRef in https://codesandbox.io/s/awesome-paper-hh0wh?module=App.js

The following no longer raise a linting warning (as react can determine displayNames for them):

import React from 'react'

export const ComponentWithMemo = React.memo(function Component({ world }) {
 return <div>Hello {world}</div>
})

export const ComponentWithForwardRef = React.forwardRef(function Component({ world }) {
  return <div>Hello {world}</div>
})

Using either an unnamed Function declaration (function() {/*...*/}) or arrow functions continue to raise a warning, as a displayName can not be inferred.

Nesting memo and forwardref (const Component = React.memo(React.forwardRef(function Component() { /*...*/})) is not currently handled by react so continue to raise an error in that case (see facebook/react#16722)

Fixes #2324, Fixes #2269

[BPScott]

This and the below addition of !! aren't strictly needed but I spotted every other variable in this block returns a boolean so I figured these should too

[ljharb]

the thing inside forwardRef is not a component, and should not be identified as such. forwardRef callbacks take props and ref, which isn't what components take.

[BPScott]

Gotcha, thanks for the clarification :)

In that case I think there is something off with the Component detection, as this testcase identifies two components (as found by doing a console.log(components.list()) in the Program:exit function in the display-name rule). It sounds like it should only identify one - possibly the outer call to React.memo. This also occurs on master too.

At risk of creating one of those magic eye "how many triangles can you see puzzles", how many components should be identified in the following pieces of code?

// Currently reports 1 component - I *think* that is the result of calling React.memo, and the argument that is passed into React.memo is ignored, can you confirm?
const ComponentWithMemo = React.memo(function Component({ world }) {
  return <div>Hello {world}</div>
})

// currently reports 1 component - The result of calling React.forwardRef is a component, the argument that is passed into React.forwardRef isn't a component
const ComponentWithForwardRef = React.forwardRef(function NotAComponent({ world }, ref) {
  return <div>Hello {world}</div>
}, )

// Should this report one or two components found?
// Currently it seems to identify two, - The result of calling React.memo, and the result of React.forwardRef, but that seems at odds with how React.memo worked previously where it didn't count the component that was the argument passed into React.memo
const ComponentWithMemoAndForwardRef = React.memo(React.forwardRef(function Component({ world }, ref) {
  return <div>Hello {world}</div>
}))

I don't understand how the internal Component identification heuristics works and what knock-on effects changing them would have. It looks like 98d4bf3 did some work relating to this but I'm not sure how to fix my above confusion

[ljharb]

this shouldn't be fixed because it's not broken; forwardRef is a component but does not accept one; it accepts a callback that takes props and ref.

[BPScott]

I think I'm referring to a slightly different issue. I'm referring how React.memo handles accepting a Component vs the result of calling forwardRef.

When inspecting the MemoFunction and MemoThenForwardRefFunction
items on https://codesandbox.io/s/awesome-paper-hh0wh?module=App.js I end up with the following in my react devtools:

Note that calling React.memo with a function component defined inline

const MemoFunction = React.memo(function MemoFunction() {
  return <div>MemoFunction</div>;
});

Results in in the name MemoFunction [memo] in the component tree.

However calling React.memo with an invocation of React.forwardRef (which returns a Component)

const MemoThenForwardRefFunction = React.memo(
  React.forwardRef(function MemoThenForwardRefFunction() {
    return <div>MemoThenForwardRefFunction</div>;
  })
);

Results in the name Anonymous in the component tree - Given that the result of an unwrapped call to React.forwardRef has a meaningful name, I would have expected that to have been picked up by React.memo

[BPScott]

I've raised this upstream as facebook/react#16722

[BPScott]

.

[BPScott]

Heya @ljharb, I've gave this a kick again and I think it is ready for review. I've updated the naming in the tests.

The functions that you pass into forwardRef are trixy - they're not quite components as you say, but you can set a displayName on them and that gets honored like regular components. Because they act like components in that sense I think it makes sense to use the existing componentt detection logic for them.

I'm torn on what to do when we have a nested React.memo(React.forwardRef(/*...*/)). It's marked as an error for now, but we can go back and revisit later.

[BPScott]

Removed the unneeded parser settings in tests

[fefrei]

Nesting memo and forwardref (const Component = React.memo(React.forwardRef(function Component() { /.../})) is not currently handled by react so continue to raise an error in that case (see facebook/react#16722)

@BPScott: FYI, it looks like this React issue was solved in November 2019, so it might make sense to allow this now, too 🙂

[ljharb]

@fefrei please feel free to file an issue or a PR!