Bug: [eslint-plugin-react-hooks] exhaustive-deps false positive on "unnecessary" dependency if its a React component

[zeorin]

Steps to reproduce

1. create a memoized value using useMemo
2. a React component is used in the creation of this value, in a JSX expression
3. specify the React component in the dependency array

Link to code example: https://github.com/zeorin/eslint-plugin-react-hooks-repro

The current behavior

React Hook useMemo has an unnecessary dependency: 'Component'. Either exclude it or remove the dependency array react-hooks/exhaustive-deps

The expected behavior

No lint errors.

More details

A simple repro (taken from the link above) is:

function Foo({ component: Component }) {
	const memoized = useMemo(() => ({
		render: () => <Component />
	}), [Component]);

	return memoized.render();
}

Workarounds

If one changes the component to lowercase, the lint error goes away. It does also mean that we need to change the way we render the component:

function Foo({ component }) {
	const memoized = useMemo(() => ({
		render: component
	}), [component]);

	return memoized.render();
}

Alternatively we can decide not to use JSX, in which case the lint rule functions correctly, too:

function Foo({ component: Component }) {
	const memoized = useMemo(() => ({
		render: () => React.createElement(Component)
	}), [Component]);

	return memoized.render();
}

Impact

Currently it is hard to use props that are components in a JSX expression if one is using the exhaustive-deps rule.

This is also compounded by the fact that this rule has a ESLint fix that removes the dependency, thus changing the behaviour of the code and leading to bugs. See #16313 for that bug report.

[gaearon]

This looks like a legit bug to me. Thank you for reporting!
If you're interested in looking into a fix, let me know and I can give a few pointers.

[zeorin]

I am interested in looking into a fix, I had a look but I think there's something implicit that I'm missing. I'd appreciate a few pointers.

[simbathesailor]

I tried finding out the location where , it can be solved. So basically

react/packages/eslint-plugin-react-hooks/src/ExhaustiveDeps.js

Line 360 in be76966

function gatherDependenciesRecursively(currentScope) {

gatherDependenciesRecursively right now is now not able to gather JSXElement as the dependency.

currentScope.references and currentScope.through does not contain JSXElement reference. I tried finding out , how we can pick the JSXElement dependency. But no success. Any pointers . @gaearon @zeorin ?

[martinschaer]

I’m getting the same error while doing something like this:

const useExample = (data) => useMemo(() => ({ ...data, sortBy }), [data, sortBy])

Warning message:

React Hook useMemo has an unnecessary dependency: 'sortBy'. Either exclude it or remove the dependency array. Outer scope values like 'sortBy' aren't valid dependencies because mutating them doesn't re-render the component.eslint(react-hooks/exhaustive-deps)

[delca85]

Hi! I would like to give this a shot if nobody is already working on it.
I made a small investigation and from what I understood I could say that the scope returned by the eslint scope manager

react/packages/eslint-plugin-react-hooks/src/ExhaustiveDeps.js

Line 266 in b23ea02

const scope = scopeManager.acquire(node);

provides an empty references array as well as the references array in its childScopes[0].
And as far as I could understand this is the reason why

react/packages/eslint-plugin-react-hooks/src/ExhaustiveDeps.js

Line 503 in b23ea02

function gatherDependenciesRecursively(currentScope) {

is not able to retrieve the Component dependency.

I don't know if I am getting the point but I would be happy to try to fix this issue.

[delca85]

I have made some tests related to the behavior reported by @martinschaer .

I have simplified a bit the code to be tested and this is what I am using:

function Example({ sortBy }) {
  const useExample = () => useMemo(() => console.log(sortBy), [sortBy])
}

According to what I have understood, this is what happens and sounds valuable to me:

1. the scope references array contains sortBy variable
2. pureScopes does not contain the sortBy variable resolved scope
3. pureScopes contains only one scope. The upper scope of this one is the one where sortBy is resolved.

The third point is something I was expecting for, after I noticed this bug, because I supposed that the sortBy variable is actually resolved two scopes upper than the one where the hook is defined.

I am not so sure that this problem and the one related to the JSX component have the same root.

If anyone could share his/her opinion I would be glad to proceed with these issues.

[delca85]

Hi all!
I have submitted the linked issue to eslint-escope to have some information about the reason why Component does not appear in scope references array.
As you can read, this is a specific choice by the eslint team so this something to be managed inside the exhaustive-deps rule.

[delca85]

I noticed right now that this is the same issue discussed in #18937 .

[eps1lon]

If you're using @typescript-eslint/parser@4.x this should be fixed with the latest release of eslint-plugin-react-hooks@4.1.2. But only starting with version 4 of @typescript-eslint/parser. @babel/eslint-parser should still report this false positive.

[delca85]

Great, thank you very much @eps1lon !

[zeorin]

Hey all, it's worth noting that this seems to affect the React Compiler, the Compiler ESLint plugin will warn about a de-opt if this the exhaustive deps lint rule is suppressed.

Current workaround:

function Foo({ component: Component }) {
	const memoized = useMemo(() => ({
		render: () => (Component, <Component />)
	}), [Component]);

	return memoized.render();
}

In case it's not clear what's happening here: I'm using the comma operator to reference Component outside of JSX, which causes the exhaustive deps rule to pick up that it's being used.