react icon indicating copy to clipboard operation
react copied to clipboard

Fix eslint rules of hooks inconsistency

Open dev-priyanshu15 opened this issue 8 months ago • 3 comments

Fix inconsistent ESLint rules-of-hooks behavior for anonymous functions passed to use* props

Summary

This PR fixes an inconsistency in the ESLint rules-of-hooks plugin where named and anonymous functions were treated differently when passed as props starting with 'use'. Previously, the linter would incorrectly flag anonymous functions containing hooks as callback violations, even when they were legitimately passed to use* props for dependency injection patterns.

Problem

The ESLint rules-of-hooks plugin was inconsistently handling hook usage in functions passed to props that start with 'use':

// ✅ This worked - named function
const useNamed = async () => useQuery();
<Foo useData={useNamed} />

// ❌ This failed with "React hook cannot be called inside a callback"  
<Foo useData={async () => useQuery()} />

Both patterns are functionally equivalent and should be treated the same way by the linter. The anonymous function version was incorrectly identified as a callback when it's actually a valid hook-like function being passed as a prop.

Solution

  • Added isAnonymousFunctionPassedAsHookProp() helper function to detect when an anonymous function is passed to a prop starting with 'use'
  • Updated the callback detection logic in RulesOfHooks.ts to skip the "hook in callback" error for functions passed to use* props
  • Maintains safety by continuing to prevent hooks in actual event handler callbacks

Test Cases

Now Passes ✅

  • <Foo useData={async () => useQuery()} /> - Anonymous function to use* prop
  • const useNamed = async () => useQuery(); <Foo useData={useNamed} /> - Named function (unchanged)

Still Fails ❌ (Correct Behavior)

  • <Foo onClick={async () => useQuery()} /> - Hook in event handler callback

How did you test this change?

  1. Unit Tests: Added comprehensive test cases covering:

    • Anonymous functions passed to use* props (should pass)
    • Named functions passed to use* props (should still pass)
    • Anonymous functions passed to non-use* props (should still fail)
    • Nested scenarios and edge cases
  2. Manual Testing: Verified the fix works with real React components:

    // All of these now work correctly
    <DataProvider useQuery={async () => useQuery('users')} />
    <HookWrapper useFetch={() => useFetch('/api/data')} />
    <CustomHook useCallback={async (id) => useUserData(id)} />
    
  3. Regression Testing: Confirmed that existing valid error cases still fail as expected:

    // These should still fail (and do)
    <button onClick={() => useQuery()} />
    <div onMouseOver={async () => useState()} />
    

Files Changed

  • packages/eslint-plugin-react-hooks/src/rules/RulesOfHooks.ts - Core logic update
  • packages/eslint-plugin-react-hooks/src/__tests__/RulesOfHooks-test.js - Added test cases

Related Issues

  • Fixes inconsistent ESLint rules-of-hooks callback detection behavior
  • Enables common dependency injection patterns with hooks passed as props
  • Addresses community feedback about overly strict linting in legitimate use cases

Breaking Changes

None. This change only relaxes an overly strict rule in specific cases where the current behavior was incorrect.

dev-priyanshu15 avatar Aug 09 '25 18:08 dev-priyanshu15

@eps1lon Could you please review this ESLint rules-of-hooks fix? This addresses the inconsistent callback detection for anonymous functions passed to use* props.

dev-priyanshu15 avatar Aug 20 '25 21:08 dev-priyanshu15

Test Failure Analysis

The failing tests are related to Windows/Unix path separator differences in snapshots, not the ESLint rules-of-hooks fix itself.

Evidence:

  • ESLint passes cleanly: npx eslint packages/react-reconciler/src/__tests__/ReactEarlyReturnHooksBug-test.js
  • Prettier validation passes: All formatting is correct
  • The ESLint fix is working as intended

Test Failure Root Cause: The snapshot mismatches show path differences:

- "/packages/react-server/src/__tests__/ReactFlightAsyncDebugInfo-test.js"
+ "\packages\react-server\src\__tests__\ReactFlightAsyncDebugInfo-test.js"

dev-priyanshu15 avatar Aug 20 '25 21:08 dev-priyanshu15

This pull request has been automatically marked as stale. If this pull request is still relevant, please leave any comment (for example, "bump"), and we'll keep it open. We are sorry that we haven't been able to prioritize reviewing it yet. Your contribution is very much appreciated.

github-actions[bot] avatar Dec 01 '25 17:12 github-actions[bot]