fix[eslint-plugin-react-hooks]: detect memo/forwardRef-wrapped components in mayContainReactCode heuristic#36433
Open
michaeljaltamirano wants to merge 2 commits intofacebook:mainfrom
Conversation
…ents in mayContainReactCode heuristic
The VariableDeclaration handler in checkTopLevelNode only matched
ArrowFunctionExpression and FunctionExpression as initializers. This
caused the heuristic to return false for patterns like:
const MyComponent = memo(function MyComponent() { ... })
const MyComponent = React.memo(() => { ... })
const MyComponent = forwardRef(function MyComponent() { ... })
When mayContainReactCode returns false, compilation is skipped and no
compiler-based rules (refs, set-state-in-effect, etc.) are reported —
even though the identical violations in a plain function declaration are
caught correctly.
Fix: unwrap one level of CallExpression arguments so that a function
literal passed to a HOC (memo, forwardRef, etc.) is treated the same as
a direct function assignment when the variable name matches the component
or hook naming convention.
…ed components
Add regression tests for memo/forwardRef-wrapped component patterns that
were silently skipped by the mayContainReactCode heuristic before the fix
to checkTopLevelNode's VariableDeclaration handler.
Each test file gains:
- invalid: memo(function Comp), memo(arrow), React.memo(function Comp),
forwardRef(function Comp), export const Comp = memo(...)
These prove compilation runs and violations are reported, matching the
behavior of equivalent plain function declarations.
- valid: lowercase variable initialized with a wrapped function
This proves the heuristic still correctly skips non-component files
when the variable name does not match the PascalCase/hook naming convention.
michaeljaltamirano
force-pushed
the
fix/memo-heuristic-variable-declaration
branch
from
4f9670f to
ff12412
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
This PR seeks to resolve #36432 by updating
packages/eslint-plugin-react-hooks/src/shared/RunReactCompiler.tsto supportconst MyComponent = memo(function MyComponent() {}),const MyComponent = React.memo(() => {}), andconst MyComponent = forwardRef(function MyComponent() {})syntaxes.eslint-plugin-react-hooks@7.1.0introduced some logic to improve performance by skipping linting for files that do not returntrueformayContainReactCode. This PR contains some extension of that logic to make sure we are linting files with the above syntax.The scope specifically only concerns
memoandforwardRef. This PR could be extended to include otherreactexports, but is specifically considering custom higher-order components out of scope.How did you test this change?
Manually, by changing the source code of
packages/eslint-plugin-react-hooks/src/shared/RunReactCompiler.tsdirectly to raise lint errors in a repo that was not reporting existing// eslint-disable-next-linedirectives, after upgradingeslint-plugin-react-hooksfrom7.0.1to7.1.1. I double-checked that the compiler itself reports all such instances are errors (Compiler Playground link) and so this is specifically aneslint-plugin-react-hooksregression.Per the PR template and https://legacy.reactjs.org/docs/how-to-contribute.html, I also manually ran the related
yarnscripts to double-check these changes passed prior to opening a PR and running CI.