[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 for third-party libraries (like amCharts) that export methods with use*-prefixed names that are not React hooks. The fix tracks identifiers loaded from module imports with unresolved types and skips hook validation for method calls on those receivers.

• The core logic in validateHooksUsage correctly identifies untyped imports (via ImportDefault, ImportNamespace, ImportSpecifier bindings) and skips hook checks for MethodCall instructions on those receivers, while preserving validation for typed imports like React
• The visitFunctionExpression handler uses a broader skip (any untyped receiver, not just imports) since it lacks access to the import tracking set — this is consistent with the heuristic typing behavior but the comment is slightly misleading
• No test fixtures were added for the new behavior; the PR relies on existing tests passing and manual testing

Confidence Score: 4/5

• This PR is safe to merge — the validation skip is narrowly scoped to method calls on untyped import receivers and preserves all existing hook checks for typed imports and direct calls.
• Score of 4 reflects that the logic is correct and well-targeted: it only skips validation when the receiver is an untyped import (in the main function) or an untyped receiver (in nested functions), which correctly matches cases where hook detection is based purely on naming heuristics. The slight deduction is for the lack of new test fixtures covering the specific scenario (e.g., amCharts-style imports with conditional use* method calls) and a minor comment inaccuracy.
• The visitFunctionExpression function in ValidateHooksUsage.ts uses a broader skip condition than the main validateHooksUsage function — worth verifying this asymmetry is intentional.

Important Files Changed

Filename	Overview
compiler/packages/babel-plugin-react-compiler/src/Validation/ValidateHooksUsage.ts	Adds tracking of untyped module import identifiers and skips heuristic hook validation for method calls on those receivers. Logic is sound but has a minor comment inaccuracy in visitFunctionExpression and no new test fixtures were added.

_{Last reviewed commit: ea5f060}

[greptile-apps]

_{1 file reviewed, 2 comments}

_{Edit Code Review Agent Settings | Greptile}

[greptile-apps]

Broader skip logic than main function

In validateHooksUsage, the skip requires both isReceiverUntyped && isReceiverImport, but here in visitFunctionExpression only isReceiverUntyped is checked (no import tracking). This means method calls on any untyped receiver (not just imports) will skip hook validation inside nested functions — e.g., localObj.useHook() inside a callback would also be skipped if localObj has an unresolved type.

This is likely acceptable since getHookKind relies on the same heuristic type system (getPropertyType assigns DefaultNonmutatingHook for use*-named properties on untyped receivers), so the reasoning is consistent. However, the comment on line 493 says "untyped Global receivers" which is misleading — the check doesn't verify the receiver is a global, just that its type is unresolved.

Consider updating the comment to reflect the actual behavior:

Suggested change

	* For MethodCall on untyped Global receivers, skip hook validation
	* as the property was typed heuristically. See #32109.
	*/
	if (instr.value.kind === 'MethodCall') {
	const receiverType = instr.value.receiver.identifier.type;
	if (
	receiverType.kind !== 'Object' &&
	receiverType.kind !== 'Function'
	) {
	break;
	}
	/**
	* For MethodCall on receivers with unresolved types, skip hook
	* validation as the property was typed heuristically. See #32109.
	*/

_{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 logic than main function**

In `validateHooksUsage`, the skip requires both `isReceiverUntyped && isReceiverImport`, but here in `visitFunctionExpression` only `isReceiverUntyped` is checked (no import tracking). This means method calls on *any* untyped receiver (not just imports) will skip hook validation inside nested functions — e.g., `localObj.useHook()` inside a callback would also be skipped if `localObj` has an unresolved type.

This is likely acceptable since `getHookKind` relies on the same heuristic type system (`getPropertyType` assigns `DefaultNonmutatingHook` for `use*`-named properties on untyped receivers), so the reasoning is consistent. However, the comment on line 493 says "untyped Global receivers" which is misleading — the check doesn't verify the receiver is a global, just that its type is unresolved.

Consider updating the comment to reflect the actual behavior:

```suggestion
          /**
           * For MethodCall on receivers with unresolved types, skip hook
           * validation as the property was typed heuristically. See #32109.
           */
```

<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]

Redundant isReceiverUntyped check

The isReceiverUntyped check on line 374-376 is redundant with isReceiverImport on line 377-380. The untypedImportIdentifiers set (line 178) only adds identifiers that already passed the same untyped check (lines 253-254: type.kind !== 'Object' && type.kind !== 'Function'). So whenever isReceiverImport is true, isReceiverUntyped is guaranteed to also be true.

This isn't a bug — the redundancy adds a defensive check in case the identifier's type changes between LoadGlobal and MethodCall processing — but it may be worth a brief comment noting this is intentional for clarity.

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: 373-381

Comment:
**Redundant `isReceiverUntyped` check**

The `isReceiverUntyped` check on line 374-376 is redundant with `isReceiverImport` on line 377-380. The `untypedImportIdentifiers` set (line 178) only adds identifiers that already passed the same untyped check (lines 253-254: `type.kind !== 'Object' && type.kind !== 'Function'`). So whenever `isReceiverImport` is `true`, `isReceiverUntyped` is guaranteed to also be `true`.

This isn't a bug — the redundancy adds a defensive check in case the identifier's type changes between `LoadGlobal` and `MethodCall` processing — but it may be worth a brief comment noting this is intentional for clarity.

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

[everettbu]

Upstream PR was closed or merged. Code is synced via branch mirror.