[compiler] Fix TypeError from hoisting nullable closure property deps

[everettbu]

Mirror of facebook/react#35887
Original author: fresh3nough

---

Summary

Fixes #35762

The compiler's hoistable property load analysis promotes non-null proofs from inner functions (JSX attribute callbacks, hook callbacks, direct calls, returned functions) to the outer render scope. However, these proofs are only valid at invocation time, not at the point where cache keys are evaluated during render.

When a closure accesses obj.value inside an event handler, the inner function proves obj is non-null. The compiler incorrectly promotes this proof to the outer scope, allowing obj.value to be hoisted as a cache dependency ($[0] !== obj.value). If obj is null/undefined at render time, this throws a TypeError before the null guard can run.

Fix

In collectNonNullsInBlocks (CollectHoistablePropertyLoads.ts), when promoting hoistable entries from assumed-invoked inner functions to the outer scope, only promote entries whose root identifier is already proven non-null in the outer scope. This prevents unsafe property access hoisting while preserving the optimization for cases where the root is known to be non-null (e.g., component props object, variables proven non-null by prior instructions).

The effect is that dependencies are truncated to the root identifier (e.g., $[0] !== user instead of $[0] !== user.name), which is always safe for reference comparison and still correct for memoization.

How did you test this change?

• Added a new fixture nullable-closure-property-dep that reproduces the issue from #35762
• Updated 35 existing fixture snapshots (all consistent: property access deps in closures truncated to root when root isn't proven non-null)
• All 43 Jest tests pass across 15 test suites
• Prettier and lint pass

Steps to reproduce the original bug

const MyComponent = ({user}) => {
  const handleClick = () => {
    console.log(user.name);
  };

  if (!user) return null;

  return <button onClick={handleClick}>Click</button>;
};

// Calling <MyComponent user={null} /> throws:
// TypeError: Cannot read properties of null (reading 'name')

Before fix: Compiler generates $[0] !== user.name as cache key, which throws when user is null.

After fix: Compiler generates $[0] !== user as cache key, which safely compares references.

[greptile-apps]

Greptile Summary

Fixes a TypeError that occurs when the React Compiler incorrectly hoists property access from inner closures (e.g. event handlers, hook callbacks, JSX attribute callbacks) as cache keys in the outer render scope. The bug manifests when a closure accesses obj.value but obj can be null/undefined at render time — the compiler would generate $[0] !== obj.value as a cache dependency check, throwing before any null guard could run.

The fix adds a single guard in collectNonNullsInBlocks (CollectHoistablePropertyLoads.ts): when promoting non-null proofs from assumed-invoked inner functions to the outer scope, it now requires the root identifier to already be proven non-null in the outer scope. This correctly distinguishes between "proven non-null at invocation time" (inside the closure) and "proven non-null at render time" (where cache keys are evaluated).

• The core change is a +12 line guard in CollectHoistablePropertyLoads.ts that gates inner-to-outer non-null proof promotion on the root identifier already being in the outer scope's assumedNonNullObjects set
• A new regression test (nullable-closure-property-dep) reproduces the exact scenario from the linked issue
• 35 existing fixture snapshots are updated consistently: property access deps in closures are truncated to the root identifier when the root isn't proven non-null in the outer scope (e.g. obj.value → obj, a.b.c → a)
• The optimization is preserved when the root IS known non-null (e.g. repro-invariant shows data.a.d → data.a because data from component props is proven non-null via a direct property load in render scope)
• The trade-off is slightly less granular memoization (comparing by root reference instead of deep property), but this is always safe for correctness

Confidence Score: 5/5

• This PR is safe to merge — it fixes a correctness bug with a minimal, well-scoped change that is conservative by design.
• The fix is a small, targeted guard (12 lines) in a single function that makes the compiler more conservative (less hoisting), which can only improve correctness at the cost of slightly coarser memoization. The logic is sound: inner function non-null proofs are only valid at invocation time, not at render-time cache key evaluation. All 35 updated snapshots are consistent with the expected behavior. A new regression test directly covers the reported bug. The repro-invariant fixture confirms the optimization is preserved when it is safe (root proven non-null in outer scope).
• No files require special attention.

Important Files Changed

Filename	Overview
compiler/packages/babel-plugin-react-compiler/src/HIR/CollectHoistablePropertyLoads.ts	Core fix: when promoting inner function non-null proofs to the outer scope, now requires the root identifier to already be proven non-null in the outer scope. Well-scoped, minimal change with clear correctness reasoning.
compiler/packages/babel-plugin-react-compiler/src/tests/fixtures/compiler/nullable-closure-property-dep.js	New regression test that reproduces the original bug: closure accessing user.name where user can be null at render time.
compiler/packages/babel-plugin-react-compiler/src/tests/fixtures/compiler/nullable-closure-property-dep.expect.md	Expected output showing compiler correctly generates $[0] !== user (not $[0] !== user.name) as cache key, preventing TypeError when user is null.
compiler/packages/babel-plugin-react-compiler/src/tests/fixtures/compiler/repro-context-var-reassign-no-scope.expect.md	Snapshot updated: cache key truncated from users.length to users, and memo scopes merged resulting in reduced cache slot count (8 to 7).
compiler/packages/babel-plugin-react-compiler/src/tests/fixtures/compiler/propagate-scope-deps-hir-fork/repro-invariant.expect.md	Snapshot updated: cache key truncated from data.a.d to data.a — partial truncation since data (from component props) IS proven non-null, but data.a is not.

_{Last reviewed commit: 7763e66}

[greptile-apps]

_{38 files reviewed, no comments}

_{Edit Code Review Agent Settings | Greptile}

[everettbu]

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