From 42990590a5d3fd1155215300c39910ad3e296b80 Mon Sep 17 00:00:00 2001
From: arsyadal <116419335+arsyadal@users.noreply.github.com>
Date: Fri, 20 Mar 2026 22:02:08 +0700
Subject: [PATCH] Fix: preserve optional chaining when inner fn in hook
callback is conditionally called
---
.../src/HIR/CollectHoistablePropertyLoads.ts | 27 +++-
...m-conditional-inner-fn-in-effect.expect.md | 132 ++++++++++++++++++
...ved-from-conditional-inner-fn-in-effect.js | 42 ++++++
3 files changed, 200 insertions(+), 1 deletion(-)
create mode 100644 compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/bug-optional-chain-preserved-from-conditional-inner-fn-in-effect.expect.md
create mode 100644 compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/bug-optional-chain-preserved-from-conditional-inner-fn-in-effect.js
diff --git a/compiler/packages/babel-plugin-react-compiler/src/HIR/CollectHoistablePropertyLoads.ts b/compiler/packages/babel-plugin-react-compiler/src/HIR/CollectHoistablePropertyLoads.ts
index c47a41145157..0fd2f521a3e9 100644
--- a/compiler/packages/babel-plugin-react-compiler/src/HIR/CollectHoistablePropertyLoads.ts
+++ b/compiler/packages/babel-plugin-react-compiler/src/HIR/CollectHoistablePropertyLoads.ts
@@ -429,7 +429,32 @@ function collectNonNullsInBlocks(
}
if (instr.value.kind === 'FunctionExpression') {
const innerFn = instr.value.loweredFunc;
- if (context.assumedInvokedFns.has(innerFn)) {
+ /**
+ * Only propagate non-null assumptions from inner functions when we are
+ * at the top-level function scope (nestedFnImmutableContext === null).
+ *
+ * When already inside a nested function (e.g. a useEffect callback),
+ * we must NOT further recurse into its inner functions. Otherwise,
+ * property accesses inside a conditionally-called inner function (e.g.
+ * `if (x) inner()`) incorrectly mark `x` as non-null in the parent
+ * function's entry block, causing optional chains (`x?.foo`) to be
+ * unsafely removed.
+ *
+ * Example of the bug this prevents:
+ * useEffect(() => {
+ * async function log() { console.log(x.os) } // non-optional
+ * if (x) log(); // conditional call
+ * }, [x]);
+ * // Compiler must NOT infer x is non-null from log()'s body
+ *
+ * Functions defined directly in the component scope (depth 0) are
+ * still processed correctly, since nestedFnImmutableContext is null
+ * at that level.
+ */
+ if (
+ context.nestedFnImmutableContext === null &&
+ context.assumedInvokedFns.has(innerFn)
+ ) {
const innerHoistableMap = collectHoistablePropertyLoadsImpl(
innerFn.func,
{
diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/bug-optional-chain-preserved-from-conditional-inner-fn-in-effect.expect.md b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/bug-optional-chain-preserved-from-conditional-inner-fn-in-effect.expect.md
new file mode 100644
index 000000000000..e3955bc9fbcb
--- /dev/null
+++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/bug-optional-chain-preserved-from-conditional-inner-fn-in-effect.expect.md
@@ -0,0 +1,132 @@
+
+## Input
+
+```javascript
+// @validatePreserveExistingMemoizationGuarantees:false
+import {useState, useEffect} from 'react';
+
+/**
+ * Regression test for: React Compiler removes optional chaining due to
+ * incorrect non-null assumption from a conditionally-called inner function
+ * inside useEffect.
+ *
+ * The compiler was incorrectly assuming `currentDevice` is always non-null
+ * because `currentDevice.os` is accessed inside `log()`. However, `log()` is
+ * only called when `currentDevice` is truthy — so this access does NOT prove
+ * non-nullability in the outer scope.
+ *
+ * The compiled output must preserve `currentDevice?.type` and
+ * `currentDevice?.os` as optional chains.
+ */
+export default function Scanner() {
+ const [devices, setDevices] = useState([]);
+ const [currentDeviceIndex, setCurrentDeviceIndex] = useState(0);
+
+ const currentDevice = devices[currentDeviceIndex];
+
+ useEffect(() => {
+ async function log() {
+ console.log(currentDevice.os);
+ }
+ if (currentDevice) log();
+ }, [currentDevice]);
+
+ return (
+
+ device type:{' '}
+ {currentDevice?.type ||
+ (currentDevice?.os?.match(/android|ios/i) ? 'mobile' : 'desktop')}
+
+ );
+}
+
+export const FIXTURE_ENTRYPOINT = {
+ fn: Scanner,
+ params: [{}],
+};
+
+```
+
+## Code
+
+```javascript
+import { c as _c } from "react/compiler-runtime"; // @validatePreserveExistingMemoizationGuarantees:false
+import { useState, useEffect } from "react";
+
+/**
+ * Regression test for: React Compiler removes optional chaining due to
+ * incorrect non-null assumption from a conditionally-called inner function
+ * inside useEffect.
+ *
+ * The compiler was incorrectly assuming `currentDevice` is always non-null
+ * because `currentDevice.os` is accessed inside `log()`. However, `log()` is
+ * only called when `currentDevice` is truthy — so this access does NOT prove
+ * non-nullability in the outer scope.
+ *
+ * The compiled output must preserve `currentDevice?.type` and
+ * `currentDevice?.os` as optional chains.
+ */
+export default function Scanner() {
+ const $ = _c(9);
+ let t0;
+ if ($[0] === Symbol.for("react.memo_cache_sentinel")) {
+ t0 = [];
+ $[0] = t0;
+ } else {
+ t0 = $[0];
+ }
+ const [devices] = useState(t0);
+ const [currentDeviceIndex] = useState(0);
+
+ const currentDevice = devices[currentDeviceIndex];
+ let t1;
+ let t2;
+ if ($[1] !== currentDevice) {
+ t1 = () => {
+ const log = async function log() {
+ console.log(currentDevice.os);
+ };
+ if (currentDevice) {
+ log();
+ }
+ };
+ t2 = [currentDevice];
+ $[1] = currentDevice;
+ $[2] = t1;
+ $[3] = t2;
+ } else {
+ t1 = $[2];
+ t2 = $[3];
+ }
+ useEffect(t1, t2);
+ let t3;
+ if ($[4] !== currentDevice?.os || $[5] !== currentDevice?.type) {
+ t3 =
+ currentDevice?.type ||
+ (currentDevice?.os?.match(/android|ios/i) ? "mobile" : "desktop");
+ $[4] = currentDevice?.os;
+ $[5] = currentDevice?.type;
+ $[6] = t3;
+ } else {
+ t3 = $[6];
+ }
+ let t4;
+ if ($[7] !== t3) {
+ t4 = device type: {t3}
;
+ $[7] = t3;
+ $[8] = t4;
+ } else {
+ t4 = $[8];
+ }
+ return t4;
+}
+
+export const FIXTURE_ENTRYPOINT = {
+ fn: Scanner,
+ params: [{}],
+};
+
+```
+
+### Eval output
+(kind: ok) device type: desktop
\ No newline at end of file
diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/bug-optional-chain-preserved-from-conditional-inner-fn-in-effect.js b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/bug-optional-chain-preserved-from-conditional-inner-fn-in-effect.js
new file mode 100644
index 000000000000..fa5ef548733b
--- /dev/null
+++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/bug-optional-chain-preserved-from-conditional-inner-fn-in-effect.js
@@ -0,0 +1,42 @@
+// @validatePreserveExistingMemoizationGuarantees:false
+import {useState, useEffect} from 'react';
+
+/**
+ * Regression test for: React Compiler removes optional chaining due to
+ * incorrect non-null assumption from a conditionally-called inner function
+ * inside useEffect.
+ *
+ * The compiler was incorrectly assuming `currentDevice` is always non-null
+ * because `currentDevice.os` is accessed inside `log()`. However, `log()` is
+ * only called when `currentDevice` is truthy — so this access does NOT prove
+ * non-nullability in the outer scope.
+ *
+ * The compiled output must preserve `currentDevice?.type` and
+ * `currentDevice?.os` as optional chains.
+ */
+export default function Scanner() {
+ const [devices, setDevices] = useState([]);
+ const [currentDeviceIndex, setCurrentDeviceIndex] = useState(0);
+
+ const currentDevice = devices[currentDeviceIndex];
+
+ useEffect(() => {
+ async function log() {
+ console.log(currentDevice.os);
+ }
+ if (currentDevice) log();
+ }, [currentDevice]);
+
+ return (
+
+ device type:{' '}
+ {currentDevice?.type ||
+ (currentDevice?.os?.match(/android|ios/i) ? 'mobile' : 'desktop')}
+
+ );
+}
+
+export const FIXTURE_ENTRYPOINT = {
+ fn: Scanner,
+ params: [{}],
+};