[react-compiler] Skip hook validation for method calls on untyped module imports

[everettbu]

Mirror of facebook/react#35858
Original author: sleitor

---

Summary

Third-party libraries like amCharts export objects with methods named useTheme(), useData(), etc. that are not React hooks. The React Compiler's ESLint plugin incorrectly flags these as hook violations because the hook detection heuristic treats any use*-named property as a potential hook.

This change tracks identifiers loaded from module imports (ImportDefault, ImportNamespace, ImportSpecifier) whose types are unresolved by the type system. For MethodCall instructions where the receiver is one of these untyped imports, we skip hook validation since the use* property was typed heuristically based on naming alone, not from actual type info.

How it works

In ValidateHooksUsage.ts:

1. Track untyped import identifiers: During LoadGlobal processing, we record identifiers that come from import bindings but have unresolved types (not Object or Function with known shapes).

2. Skip hook validation for MethodCalls on untyped imports: When processing a MethodCall, if the receiver is an untyped import, we skip the hook conditional/dynamic usage checks.

3. Handle nested function expressions: In visitFunctionExpression, we similarly skip hook-in-callback validation for method calls on untyped receivers.

What's preserved

• import React from 'react'; React.useState() — React has a known type, so hook validation is preserved ✅
• const React = require('react'); React.useState() — React is a ModuleLocal binding (not an import), so it's not tracked as an untyped import and hook validation is preserved ✅
• Direct hook calls like useHook() — These are CallExpressions, not MethodCalls, so they're unaffected ✅
• import {useHook} from 'some-lib'; useHook() — Direct import of a hook-named function still validates correctly ✅

Test plan

• All 1711 existing compiler snap tests pass
• All 30 ESLint plugin tests pass
• Manual testing confirms:
  • amCharts.useTheme() in conditional blocks no longer errors
  • React.useState() in conditional blocks still errors (both import and require styles)
  • Direct useHook() calls in conditional blocks still error

Fixes #32109

[greptile-apps]

Greptile Summary

This PR fixes false hook-violation errors from the React Compiler's ESLint plugin when third-party libraries (e.g., amCharts) export objects with use*-named methods that are not React hooks. It tracks identifiers from module imports (ImportDefault, ImportNamespace, ImportSpecifier) with unresolved types, and skips hook validation for MethodCall instructions where the receiver is one of these untyped imports.

• The core logic in validateHooksUsage correctly gates the skip on both isReceiverUntyped and isReceiverImport, ensuring that known-typed imports like React still have hook validation enforced
• The visitFunctionExpression handler uses a broader condition (type-only, no import check), which was already discussed in a prior review thread
• No dedicated test fixtures were added for this change — while existing tests pass, adding regression tests for the specific scenarios (e.g., amCharts.useTheme() in conditional/callback contexts) would strengthen confidence in the fix

Confidence Score: 4/5

• This PR is safe to merge — the logic is sound and correctly preserves hook validation for known-typed imports like React.
• The fix addresses a real false-positive issue with a targeted approach. The main validateHooksUsage check is well-gated with both type and import checks. The visitFunctionExpression asymmetry was already discussed. The only concern is the absence of dedicated regression test fixtures, which reduces long-term confidence but doesn't indicate a correctness issue.
• No files require special attention — the single changed file has been thoroughly reviewed.

Important Files Changed

Filename	Overview
compiler/packages/babel-plugin-react-compiler/src/Validation/ValidateHooksUsage.ts	Adds tracking of untyped import identifiers and skips hook validation for MethodCalls on those receivers. Logic is sound for the main validateHooksUsage function; visitFunctionExpression uses a broader skip condition (no import check) which was already discussed in a prior review thread. No new test fixtures were added.

_{Last reviewed commit: ec06091}

[greptile-apps]

_{1 file reviewed, 1 comment}

_{Edit Code Review Agent Settings | Greptile}

[greptile-apps]

Broader skip condition than validateHooksUsage

In the main validateHooksUsage function (line 381), the skip check requires both conditions: isReceiverUntyped && isReceiverImport. Here in visitFunctionExpression, only the type check is applied — any MethodCall with a non-Object/non-Function receiver is skipped, regardless of whether it's an import.

This is technically safe because getHookKind() only returns non-null for identifiers with type.kind === 'Function', so the break is effectively redundant with what getHookKind would do. However, the asymmetry between the two functions makes the intent less clear. The comment says "untyped Global receivers" but the check is broader than just globals — it applies to any receiver with an unresolved type (e.g., locals, parameters).

Consider adding a brief comment clarifying why the import check is unnecessary here (i.e., because getHookKind only operates on Function-typed identifiers, the type check alone is sufficient).

_{Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!}

Prompt To Fix With AI

This is a comment left during a code review.
Path: compiler/packages/babel-plugin-react-compiler/src/Validation/ValidateHooksUsage.ts
Line: 493-503

Comment:
**Broader skip condition than `validateHooksUsage`**

In the main `validateHooksUsage` function (line 381), the skip check requires **both** conditions: `isReceiverUntyped && isReceiverImport`. Here in `visitFunctionExpression`, only the type check is applied — any MethodCall with a non-Object/non-Function receiver is skipped, regardless of whether it's an import.

This is technically safe because `getHookKind()` only returns non-null for identifiers with `type.kind === 'Function'`, so the `break` is effectively redundant with what `getHookKind` would do. However, the asymmetry between the two functions makes the intent less clear. The comment says "untyped Global receivers" but the check is broader than just globals — it applies to any receiver with an unresolved type (e.g., locals, parameters).

Consider adding a brief comment clarifying why the import check is unnecessary here (i.e., because `getHookKind` only operates on `Function`-typed identifiers, the type check alone is sufficient).

<sub>Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!</sub>

How can I resolve this? If you propose a fix, please make it concise.

[greptile-apps]

_{1 file reviewed, no comments}

_{Edit Code Review Agent Settings | Greptile}