fix[eslint-plugin-react-hooks]: Fix error when callback argument is an identifier with an as expression

[mskelton]

Summary

Using eslint-plugin-react-hooks with ESLint v9 and TypeScript ESLint v8 causes the following issue:

Oops! Something went wrong! :(

ESLint: 9.11.1

TypeError: Cannot read properties of null (reading 'upper')
Occurred while linting /app/src/hooks/useCustomMemo.tsx:9
Rule: "react-hooks/exhaustive-deps"
    at visitFunctionWithDependencies (/app/node_modules/eslint-plugin-react-hooks/cjs/eslint-plugin-react-hooks.development.js:1106:38)
    at CallExpression (/app/node_modules/eslint-plugin-react-hooks/cjs/eslint-plugin-react-hooks.development.js:1920:25)
    at ruleErrorHandler (/app/node_modules/eslint/lib/linter/linter.js:1084:48)
    at /app/node_modules/eslint/lib/linter/safe-emitter.js:45:58
    at Array.forEach (<anonymous>)
    at Object.emit (/app/node_modules/eslint/lib/linter/safe-emitter.js:45:38)
    at NodeEventGenerator.applySelector (/app/node_modules/eslint/lib/linter/node-event-generator.js:297:26)
    at NodeEventGenerator.applySelectors (/app/node_modules/eslint/lib/linter/node-event-generator.js:326:22)
    at NodeEventGenerator.enterNode (/app/node_modules/eslint/lib/linter/node-event-generator.js:337:14)
    at runRules (/app/node_modules/eslint/lib/linter/linter.js:1128:40)

I can reproduce this with the above mentioned versions and the following file:

function useCustomCallback(callback, deps) {
  return useCallback(callback as any, deps)
}

How did you test this change?

Added a test case.

[vercel]

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name	Status	Preview	Comments	Updated (UTC)
react-compiler-playground	✅ Ready (Inspect)	Visit Preview	💬 Add feedback	Nov 18, 2024 7:05pm

[eps1lon]

It no longer will warn in this case about missing dependencies, but not sure if that matters for this kind of code example.

It does. Let's add the code you shared to tests and see why it isn't working.

The proposed fixed is common but very often not the correct one.

[react-sizebot]

Comparing: c866d75...3780588

Critical size changes

Includes critical production bundles, as well as any change greater than 2%:

Name	+/-	Base	Current	+/- gzip	Base gzip	Current gzip
oss-stable/react-dom/cjs/react-dom.production.js	=	6.68 kB	6.68 kB	+0.05%	1.83 kB	1.83 kB
oss-stable/react-dom/cjs/react-dom-client.production.js	=	509.94 kB	509.94 kB	=	91.24 kB	91.24 kB
oss-experimental/react-dom/cjs/react-dom.production.js	=	6.69 kB	6.69 kB	=	1.83 kB	1.83 kB
oss-experimental/react-dom/cjs/react-dom-client.production.js	=	514.88 kB	514.88 kB	=	91.95 kB	91.95 kB
facebook-www/ReactDOM-prod.classic.js	=	594.55 kB	594.55 kB	=	104.89 kB	104.90 kB
facebook-www/ReactDOM-prod.modern.js	=	584.81 kB	584.81 kB	=	103.32 kB	103.32 kB

Significant size changes

Includes any change greater than 0.2%:

Expand to show

Name	+/-	Base	Current	+/- gzip	Base gzip	Current gzip
oss-stable-rc/eslint-plugin-react-hooks/cjs/eslint-plugin-react-hooks.development.js	+0.39%	87.13 kB	87.47 kB	+0.62%	14.58 kB	14.67 kB
oss-stable-semver/eslint-plugin-react-hooks/cjs/eslint-plugin-react-hooks.development.js	+0.39%	87.13 kB	87.47 kB	+0.62%	14.58 kB	14.67 kB
oss-stable/eslint-plugin-react-hooks/cjs/eslint-plugin-react-hooks.development.js	+0.39%	87.13 kB	87.47 kB	+0.62%	14.58 kB	14.67 kB
oss-experimental/eslint-plugin-react-hooks/cjs/eslint-plugin-react-hooks.development.js	+0.38%	89.21 kB	89.55 kB	+0.59%	14.83 kB	14.92 kB
oss-stable-rc/eslint-plugin-react-hooks/cjs/eslint-plugin-react-hooks.production.js	+0.32%	77.95 kB	78.20 kB	+0.62%	14.24 kB	14.33 kB
oss-stable-semver/eslint-plugin-react-hooks/cjs/eslint-plugin-react-hooks.production.js	+0.32%	77.95 kB	78.20 kB	+0.62%	14.24 kB	14.33 kB
oss-stable/eslint-plugin-react-hooks/cjs/eslint-plugin-react-hooks.production.js	+0.32%	77.95 kB	78.20 kB	+0.62%	14.24 kB	14.33 kB
oss-experimental/eslint-plugin-react-hooks/cjs/eslint-plugin-react-hooks.production.js	+0.31%	79.81 kB	80.06 kB	+0.62%	14.49 kB	14.58 kB

Generated by 🚫 dangerJS against f673142

[mskelton]

@eps1lon I tried adding my code example as a test, and with the versions of eslint, typescript, and typescript-eslint in this repo, it worked fine. It's only with the versions I specified that this is an issue, and not sure exactly the best way to test that. I noticed there is configuration for testing various versions of eslint and typescript-eslint, but not typescript. I'll do some more investigation on my end and see what I can come up with.

[mskelton]

Digging through the as expression to find the actual function or identifier then allows the switch statement that follows to analyze properly for edge cases where the callback is an identifier as part of an as expression.

[mskelton]

@eps1lon I was able to build a test case that did error with the current version of the plugin which I added to this PR, and also updated the fix to properly handle this case so it does error with this code example.

[eps1lon]

This should rather warn that we can't analyze the dependencies of callback. Using [callback] in the dependencies would be equivalent to not using useCallback at all.

[mskelton]

Good idea, I updated the PR to use the unknown dependencies message when argument is an identifier passed as a function parameter.

[eps1lon]

Thanks!

[eps1lon]

Thanks!