feat: port the React Compiler's HIR + validators into react-doctor by aidenybai · Pull Request #164 · millionco/react-doctor

aidenybai · 2026-05-08T02:21:43Z

Summary

Ports the React Compiler's High-level IR (HIR) infrastructure into react-doctor as a foundation for more precise rule analysis, plus two initial validators ported from the compiler:

hir-no-set-state-in-effect: flags setState calls inside useEffect bodies, including when the setter is propagated through const x = setX aliasing or wrapped in a useEffectEvent / inner FunctionExpression.
hir-no-derived-computations-in-effects: flags effects whose captures are entirely deps + setters and whose body has at least one intermediate const (the unique path the AST walker can't see through).

What got ported

HIR data model (plugin/hir/types.ts): HIRFunction, BasicBlock, Instruction, Place, Identifier, ReactType (≈20 React-aware tags including the round-5 StateTuple), with originNode on Place for diagnostic precision.
Lowering (plugin/hir/lower.ts): ESTree → HIR, parent-chained scope environments, FunctionExpression.capturedPlaces, full statement coverage (round-5 added for/while/do-while/for-of/for-in/switch/try/throw/labeled), SpreadElement unwrapping in call args.
Type inference (plugin/hir/infer-types.ts): hook recognition via LoadGlobal name → ReactType, propagation through LoadLocal/StoreLocal/PropertyLoad. Round-5 introduces StateTuple so only useState returns get the indexed StateValue/StateSetter tagging — useMemo/useContext returns no longer leak into that branch.
Validators (plugin/hir/validators/): two ports, with shape-defer logic that lets the existing AST walker handle the simple cases.
Runner (plugin/hir/runner.ts): per-component WeakMap<EsTreeNode, HIRFunction> cache + originNode-based diagnostic anchoring so reports point at the offending setX(...) call site.

Round 5 — deep code review fixes

Five issues found on a fresh review of the round-4 code:

#	Issue	Fix
1	`inferTypes` conflated `useState`/`useMemo`/`useContext` returns as `Object`; the indexed-PropertyLoad branch tagged `[a, b] = useMemo(...)` as `[StateValue, StateSetter]`, leading to false-positive setState detection	New `StateTuple` ReactType gates the indexed-PropertyLoad branch to `useState` only; regression test for `useMemo` tuple destructure
2	`hir-no-derived-computations-in-effects` duplicated `noDerivedStateEffect` on the article §1 fullName example	Validator now defers when the inner HIR has no `StoreLocal` (single-setter-call shape) — keeps the multi-statement-with-locals path that uniquely needs HIR. `hir-no-set-state-in-effect` left at warn with a known-overlap note in `oxlint-config.ts`, since reliably distinguishing direct vs aliased setters at v1 isn't workable
3	`lowerExpression` silently dropped `SpreadElement` arguments	New `lowerCallArguments` unwraps the spread's `argument` so operand identity is preserved through setState propagation
4	`lowerStatement` ignored `for`/`while`/`do-while`/`for-of`/`for-in`/`switch`/`try`/`throw`/`labeled` — hooks inside any of these blocks were invisible	Added recursive descent for all of them
5	`hir-unit.test.ts` article-§1 case used the single-setter-call shape that fix #2 now defers on	Updated to the multi-statement-with-locals form to exercise the validator's unique path

Validation

490 / 490 tests pass (4 new regressions added across the 5 fixes)
pnpm lint clean
pnpm typecheck clean
pnpm format clean

Known v1 limitations (documented in code)

let reassignments are not modeled as non-reactive (HIR is non-SSA).
JSX expression contents aren't lowered as identifier reads (renders are summarized as a placeholder).
import { useState as useS } is not recognized — name-based heuristic on LoadGlobal.
hir-no-set-state-in-effect overlaps noDerivedStateEffect on the simplest shape; users can disable either via react-doctor.config.json.

Architecture summary

ESTree (oxlint visitor)
   ↓ lowerFunction
HIR (basic blocks + instructions + Place.originNode)
   ↓ inferTypes (per-identifier ReactType tags)
Typed HIR
   ↓ validateNo*  (per-rule analyses)
Findings (with originNode for precise reporting)
   ↓ runner reports through ruleContext
oxlint diagnostics anchored at the offending call site

Future ports (no/lazy-init-in-render, no-create-ref-during-render, deeper effect-event analyses) plug in as additional validators reading the same HIR.

vercel · 2026-05-08T02:21:48Z

The latest updates on your projects. Learn more about Vercel for GitHub.

Project	Deployment	Actions	Updated (UTC)
react-doctor-website	Ready	Preview, Comment	May 8, 2026 9:25am

Five issues found on a fresh deep review of the HIR port (PR #164): 1. inferTypes conflated useState/useMemo/useContext returns under ReactType "Object". The PropertyLoad index-0/index-1 → StateValue/ StateSetter branch keyed off "Object", so a destructure like `const [n, runIt] = useMemo(...)` would tag `runIt` as a StateSetter and the validators would treat `runIt()` as a setState call. Fix: add a distinct `StateTuple` ReactType, gate the PropertyLoad branch on it. Regression test added. 2. The HIR rules duplicated `noDerivedStateEffect` on the canonical single-setter-call shape (article §1 fullName example), producing two diagnostics on the same line. Fix: `hir-no-derived-computations-in-effects` now defers when the effect body has no intermediate local bindings (StoreLocal in the inner HIR), preserving the multi-statement-with-locals path the AST walker can't see through. `hir-no-set-state-in-effect` stays unscoped because distinguishing direct vs aliased setter calls reliably from the HIR is not workable at v1; the duplicate on the simplest shape is documented as a known v1 limitation. 3. `lowerExpression` silently dropped `SpreadElement` arguments (`f(...args)` lowered to a CallExpression with the spread missing). Extracted `lowerCallArguments` that unwraps the spread's `argument` so the operand identity is still threaded through — losing only the spread shape (not modeled in v1) but keeping setState/alias propagation working through spread args. 4. `lowerStatement` only handled VariableDeclaration, ExpressionStatement, ReturnStatement, BlockStatement, IfStatement, and the function- declaration shapes. `for`, `while`, `do-while`, `for-of`, `for-in`, `switch`, `try`, `throw`, `labeled` all silently fell through, so a useEffect (or any hook call) inside any of these blocks was invisible to validators. Added recursive descent for all of them — control flow collapses into the surrounding block (no real CFG terminals yet) but the bodies get lowered. 5. The hir-unit test for the article §1 example used the single- setter-call shape that the round-2 scoping fix now defers on; updated to the multi-statement-with-locals form to keep the coverage on the validator's unique path. All 490 tests pass; lint, typecheck, format clean. Co-authored-by: Aiden Bai <aidenybai@users.noreply.github.com>

Mirrors the structure of `babel-plugin-react-compiler/src/HIR/HIR.ts` but heavily simplified for our needs: - operates on ESTree (oxlint plugin AST), not Babel - no SSA / phi nodes (uses a mutable name→Place table per-scope) - single block per function in v1 (no if/loop terminals modeled) - 13 instruction kinds (vs the compiler's 30+) What's preserved: - data model: HIRFunction → blocks → instructions → places - lvalue / value-place / operand-place shape so validators can `switch (instr.value.kind)` exactly like the upstream code - identifier IDs are stable per binding, so propagation analyses (e.g. setState flowing through a const) work the same way - source locations threaded through every Place - type-tag layer (StateValue / StateSetter / UseEffectHook / RefValue / EffectEvent / PropCallback / …) and the matching isXType() predicates the upstream validators use This commit is types-only; the lower / infer / validators / runner land in subsequent commits to keep each layer reviewable on its own. Co-authored-by: Aiden Bai <aidenybai@users.noreply.github.com>

Walks the ESTree of a component and emits a single-block HIR with SSA-flavored instructions. Mirrors the compiler's `BuildHIR.ts::lower` shape (each AST expression becomes one or more Instructions whose lvalue is a temporary Place; operands are Places that may load from local bindings). Key design points: - Shared id allocator across nested LoweringEnvironments so a captured outer binding keeps its IdentifierId when seen by an inner closure. This is what lets validators reason about 'this effect callback closes over <X>' by Place identity rather than by name matching. - Parent-chain bindings lookup so `setCount` declared in the component body resolves to the same Place inside a useEffect callback. - ArrayPattern destructuring for `const [v, sv] = useState(...)` lowers to two indexed PropertyLoad instructions whose lvalues pick up StateValue / StateSetter types in the inferTypes pass. Deliberate v1 simplifications: - control flow is flattened (if/while/for descend into their consequent/loop without producing terminal/branch instructions) - JSX collapses to a single Literal placeholder per element - effect annotations on Place all default to 'Read'; aliasing inference is out of scope for v1 Co-authored-by: Aiden Bai <aidenybai@users.noreply.github.com>

inferTypes runs after lowering and tags Identifiers with React semantic types (StateValue, StateSetter, UseEffectHook, RefValue, EffectEvent, …). Implements a pared-down version of the compiler's `inferTypes()` pass: - LoadGlobal of a known React hook name → tag the lvalue's type - CallExpression whose callee resolves to UseStateHook tags lvalue as Object (the [value, setter] pair); subsequent indexed PropertyLoad on it tags the destructured names as StateValue and StateSetter - PropertyLoad of `.current` on a RefValue → RefCurrent - LoadLocal / StoreLocal propagate the source identifier's type so `const fn = setState; fn(x)` is still seen as a setState call by the validators printHIR emits a textual dump of an HIRFunction, used by the unit test as a snapshot of the lower pass. Co-authored-by: Aiden Bai <aidenybai@users.noreply.github.com>

Two HIR-based validators, ported from the React Compiler: validateNoSetStateInEffects (~150 LOC, upstream is 347) Tracks setState propagation through LoadLocal, StoreLocal, FunctionExpression, and useEffectEvent wrappers — exactly the five instruction kinds the compiler's switch uses to seed its `setStateFunctions: Map<IdentifierId, Place>`. Reports when a useEffect's callback resolves (transitively) to a setState binding. v1 omissions vs upstream: - ref-derived setState exception (control-dominator analysis) - aliasing-effect tracking on operands validateNoDerivedComputationsInEffects (~120 LOC, upstream is 229) Three lookup tables that mirror the upstream code: candidateDependencies: lvalue → ArrayExpression instruction effectFunctions: lvalue → FunctionExpression instruction localAliases: lvalue → underlying source IdentifierId For `useEffect(arg0, arg1)` resolves arg0 to a tracked function and arg1 to a tracked deps array; reports when the inner function captures only those deps + setStates (= pure derivation, should move to render). Both validators are pure functions of HIRFunction → finding[]; the runner / oxlint Rule shell is in the next commit. Co-authored-by: Aiden Bai <aidenybai@users.noreply.github.com>

…in runtime Two thin oxlint Rules (`hir-no-set-state-in-effect`, `hir-no-derived-computations-in-effects`) that: 1. detect a component-shaped function (uppercase FunctionDeclaration or const Foo = () => ...) 2. lower it to HIR and run inferTypes once 3. forward validator findings via context.report() Findings retain the React Compiler's diagnostic precision (Place identity, type tags, source loc) but surface through the oxlint plugin contract — no compiler runtime, no Babel, no second toolchain. Registered in plugin/index.ts and oxlint-config.ts (severity: warn) with category 'State & Effects' in run-oxlint.ts. Help text mentions 'Detected via HIR data flow analysis' so users can distinguish these findings from the AST-walker rules. The two HIR rules deliberately share IDs prefixed `hir-` so future ports stay namespaced. Co-authored-by: Aiden Bai <aidenybai@users.noreply.github.com>

Two test files: hir-port.test.ts — end-to-end through oxlint flag cases: setState-direct, setState-aliased (SSA propagation), article §1 fullName derived-computation no-flag cases: setState inside subscription callback (sub-handler), useEffect that reads non-dep prop (genuine sync) hir-unit.test.ts — direct calls into lower / infer / validators Bypasses oxlint to assert the HIR shape and validator findings on parsed source. Useful during development of the lower pass; logs a printHIR dump on failure so the IR is visible in CI output. Adds @typescript-eslint/parser as a dev dep of the react-doctor package (used by hir-unit.test.ts to parse source into ESTree). Co-authored-by: Aiden Bai <aidenybai@users.noreply.github.com>

The HIR unit test (`hir-unit.test.ts`) parses source via @typescript-eslint/parser. Earlier in the lower-pass debugging session a stray `pnpm add` planted the dep at the workspace root too — that root entry was redundant with react-doctor's own devDep and added needless monorepo surface. Drop it; the lockfile follows. Co-authored-by: Aiden Bai <aidenybai@users.noreply.github.com>

Add `originNode` to Place so diagnostics can anchor on the offending source expression instead of the surrounding component. Mirrors how the React Compiler threads `loc` through every Place, but uses an actual node reference (not a separate location-to-node table) since we already have the ESTree nodes in hand. The lower pass now sets `originNode` for: - Identifier reads (LoadLocal / LoadGlobal) - Literal / TemplateLiteral - MemberExpression - CallExpression / MethodCall - ArrowFunctionExpression / FunctionExpression - ArrayExpression / ObjectExpression - BinaryExpression / LogicalExpression / UnaryExpression - ConditionalExpression - JSXElement / JSXFragment - VariableDeclarator (Identifier and ArrayPattern locals) - destructured prop entries - regular function parameters Synthetic temporaries (e.g. method-call property places that don't exist as a single AST node) get `originNode: null` and the runner falls back to the component declaration. Co-authored-by: Aiden Bai <aidenybai@users.noreply.github.com>

The validator's `findTopLevelSetStateCall` now returns BOTH the original setter Place AND the call-site Place (the synthetic temp whose `originNode` points at the offending CallExpression in source). A parallel `innerCallSites` map propagates that call site through `LoadLocal`/`StoreLocal`/`useEffectEvent` so a setState wrapped in `useEffectEvent` still surfaces at the wrapper-call site, not at the setter declaration site. `SetStateInEffectFinding` gains a `callSitePlace` field; the runner uses it in preference to `setterPlace` when reporting. Co-authored-by: Aiden Bai <aidenybai@users.noreply.github.com>

…site Two wiring improvements on top of the bare port: 1. Per-component HIR cache (WeakMap keyed by the original ESTree node). When multiple HIR rules visit the same component during a single oxlint pass, lowering + inferTypes runs once instead of once-per-rule. The WeakMap lets the cache GC alongside the AST. 2. Diagnostics anchor on the offending source expression. Each finding's Place carries an `originNode` (set during the lower pass). The runner uses it as the report node, falling back to the component declaration only for synthetic places that don't correspond to a single source node. New regression test asserts the line number of the diagnostic for a setState-in-effect violation: must be the line of the `setCount(1)` call (line 6 in the fixture), not the line of the surrounding `export const Counter = () => {}` declaration (line 3). Co-authored-by: Aiden Bai <aidenybai@users.noreply.github.com>

`Place.originNode` was typed `unknown | null`, which forced two `as` casts in the runner (one for the WeakMap key, one when reading back the originNode for context.report). Importing `EsTreeNode` into hir/types.ts (no cycle — plugin/types.ts has no HIR imports) lets us type the field as `EsTreeNode | null` directly. Both casts gone; AGENTS.md 'Do not type cast unless absolutely necessary' is honored. Also adds the missing `isUseStateHookType` predicate. The type `UseStateHook` was already in the `ReactType` union but had no sibling helper alongside `isUseEffectHookType` / `isUseRefType` etc. Co-authored-by: Aiden Bai <aidenybai@users.noreply.github.com>

…ls) in validateEffect The previous validateEffect walked every LoadLocal in the inner function body and treated each one as a 'capture'. That conflated: - reads of variables captured from outer scope (real captures) - reads of variables LOCAL to the inner function (e.g. a temporary bound inside the effect body) Mismatch in the second category caused inner-scope locals to be flagged as 'captures-non-dep', which bailed the validator early and silently dropped a true derived-computation finding. Reproduced by: useEffect(() => { const combined = firstName + ' ' + lastName; // ← inner local setFullName(combined); }, [firstName, lastName]); // pure derivation Fix: use the FunctionExpression instruction's `capturedPlaces` field (populated by lower.ts's `collectCapturedPlaces`). That set is exactly the outer bindings the inner reads — the 1:1 analog of the upstream compiler's `effectFunction.context`. The `effectFunctions` lookup table now stores both the HIRFunction and its capture list; the validator iterates only the captures. Co-authored-by: Aiden Bai <aidenybai@users.noreply.github.com>

…rEnv parameter Two cleanups in lower.ts: 1. `lowerExpression` now also handles nested FunctionDeclaration. Previously a `function helper() {}` inside a component body fell through to the Unsupported case AND its name wasn't bound in the enclosing env, so a subsequent `helper()` call would LoadGlobal the name instead of resolving to the inner function. We lower the FunctionDeclaration like an Arrow/FunctionExpression and tie its name to the lvalue Place when entering the binding. 2. `collectCapturedPlaces` had a dead `outerEnv` parameter the comment apologized for with `void outerEnv`. The function only ever needed the inner HIRFunction (Place identifier ids are already shared via the root-env id allocator, so an outer-env argument was never load-bearing). Drop it. Test cleanup: `hir-unit.test.ts` no longer uses `as any` to coerce the parser output; it does a single explicit `as unknown as EsTreeNode` with a HACK comment explaining why the parser's Program.body[0] is structurally compatible with our EsTreeNode interface. Co-authored-by: Aiden Bai <aidenybai@users.noreply.github.com>

Five issues found on a fresh deep review of the HIR port (PR #164): 1. inferTypes conflated useState/useMemo/useContext returns under ReactType "Object". The PropertyLoad index-0/index-1 → StateValue/ StateSetter branch keyed off "Object", so a destructure like `const [n, runIt] = useMemo(...)` would tag `runIt` as a StateSetter and the validators would treat `runIt()` as a setState call. Fix: add a distinct `StateTuple` ReactType, gate the PropertyLoad branch on it. Regression test added. 2. The HIR rules duplicated `noDerivedStateEffect` on the canonical single-setter-call shape (article §1 fullName example), producing two diagnostics on the same line. Fix: `hir-no-derived-computations-in-effects` now defers when the effect body has no intermediate local bindings (StoreLocal in the inner HIR), preserving the multi-statement-with-locals path the AST walker can't see through. `hir-no-set-state-in-effect` stays unscoped because distinguishing direct vs aliased setter calls reliably from the HIR is not workable at v1; the duplicate on the simplest shape is documented as a known v1 limitation. 3. `lowerExpression` silently dropped `SpreadElement` arguments (`f(...args)` lowered to a CallExpression with the spread missing). Extracted `lowerCallArguments` that unwraps the spread's `argument` so the operand identity is still threaded through — losing only the spread shape (not modeled in v1) but keeping setState/alias propagation working through spread args. 4. `lowerStatement` only handled VariableDeclaration, ExpressionStatement, ReturnStatement, BlockStatement, IfStatement, and the function- declaration shapes. `for`, `while`, `do-while`, `for-of`, `for-in`, `switch`, `try`, `throw`, `labeled` all silently fell through, so a useEffect (or any hook call) inside any of these blocks was invisible to validators. Added recursive descent for all of them — control flow collapses into the surrounding block (no real CFG terminals yet) but the bodies get lowered. 5. The hir-unit test for the article §1 example used the single- setter-call shape that the round-2 scoping fix now defers on; updated to the multi-statement-with-locals form to keep the coverage on the validator's unique path. All 490 tests pass; lint, typecheck, format clean. Co-authored-by: Aiden Bai <aidenybai@users.noreply.github.com>

Two follow-up cleanups on the round-5 HIR review: 1. `TryStatement` lowering didn't bind the catch parameter (`} catch (error) {`), so a reference to `error` inside the handler body fell through to LoadGlobal — wrong for any future validator that traces identifier reads through try/catch. Bind the param's Identifier in the surrounding env (no real block scoping in v1; minor over-scope is acceptable). 2. Dead loop at the top of `inferTypes` that walked `fn.destructuredProps` and did nothing on every entry — removed. The intent (tag function-typed props) is already covered by lower.ts's `on[A-Z]` heuristic. All 490 tests pass; lint, typecheck, format clean. Co-authored-by: Aiden Bai <aidenybai@users.noreply.github.com>

Cleanup pass following round-5 review. No behavior changes — all 490 tests pass, lint/typecheck/format clean. Removed: - types.ts: unused predicates (isStateValueType, isUseStateHookType, isUseRefType, isRefValueType, isEffectEventType, isPropCallbackType), unused 'prop' Identifier.origin variant, unused EffectKind union members (Mutate / Capture / Store / Freeze). - print.ts: entire 80-line file (printHIR was only referenced by hir-unit.test.ts's debug logging, which is also removed). - validate-no-derived-computations-in-effects.ts: dead 'reason' field on findings — only one value was ever consumed by the runner; collapsed validator to return null on the other paths. - hir-unit.test.ts: console.log + JSON.stringify debug output that spammed CI on every run; replaced with proper expect()s. - 200+ lines of doc-style comments across types.ts / lower.ts / infer-types.ts / runner.ts / validators / hir-port.test.ts that re-explained what the code says or repeated information from the PR description. Kept (with concise // HACK: prefix): - StateTuple discriminator (real semantic distinction) - SpreadElement unwrap (real ESTree wart) - Catch-param binding (real correctness fix) - Multi-statement-with-locals defer (real overlap mitigation) - Inner-call-site Place tracking (real diagnostic-anchor concern) Co-authored-by: Aiden Bai <aidenybai@users.noreply.github.com>

vercel Bot deployed to Preview May 8, 2026 02:40 View deployment

vercel Bot deployed to Preview May 8, 2026 02:56 View deployment

vercel Bot deployed to Preview May 8, 2026 05:21 View deployment

vercel Bot deployed to Preview May 8, 2026 05:33 View deployment

vercel Bot deployed to Preview May 8, 2026 05:45 View deployment

cursor Bot force-pushed the cursor/hir-port-7144 branch from c101b26 to 29c79d0 Compare May 8, 2026 09:03

vercel Bot deployed to Preview May 8, 2026 09:03 View deployment

cursor Bot force-pushed the cursor/hir-port-7144 branch from 29c79d0 to e6c2397 Compare May 8, 2026 09:18

vercel Bot deployed to Preview May 8, 2026 09:18 View deployment

cursoragent and others added 16 commits May 8, 2026 09:23

cursor Bot force-pushed the cursor/hir-port-7144 branch from e6c2397 to e6211a0 Compare May 8, 2026 09:25

vercel Bot deployed to Preview May 8, 2026 09:25 View deployment

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat: port the React Compiler's HIR + validators into react-doctor#164

feat: port the React Compiler's HIR + validators into react-doctor#164
aidenybai wants to merge 16 commits intomainfrom
cursor/hir-port-7144

aidenybai commented May 8, 2026 •

edited by cursor Bot

Loading

Uh oh!

vercel Bot commented May 8, 2026 •

edited

Loading

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Conversation

aidenybai commented May 8, 2026 • edited by cursor Bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

What got ported

Round 5 — deep code review fixes

Validation

Known v1 limitations (documented in code)

Architecture summary

Uh oh!

vercel Bot commented May 8, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

aidenybai commented May 8, 2026 •

edited by cursor Bot

Loading

vercel Bot commented May 8, 2026 •

edited

Loading