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 1/2] 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: [{}], +}; From 6d3e7081377d532c18938f92c96401ad642163fe Mon Sep 17 00:00:00 2001 From: arsyadal <116419335+arsyadal@users.noreply.github.com> Date: Wed, 15 Apr 2026 20:59:56 +0700 Subject: [PATCH 2/2] Fix DevTools profiler flamegraph HOC guard --- .../src/__tests__/profilingCharts-test.js | 30 +++++++++++++++++++ .../views/Profiler/FlamegraphChartBuilder.js | 2 +- 2 files changed, 31 insertions(+), 1 deletion(-) diff --git a/packages/react-devtools-shared/src/__tests__/profilingCharts-test.js b/packages/react-devtools-shared/src/__tests__/profilingCharts-test.js index da20511a650e..28bb0bd35673 100644 --- a/packages/react-devtools-shared/src/__tests__/profilingCharts-test.js +++ b/packages/react-devtools-shared/src/__tests__/profilingCharts-test.js @@ -205,6 +205,36 @@ describe('profiling charts', () => { ] `); }); + + // @reactVersion >= 16.9 + it('should tolerate missing hocDisplayNames in profiling snapshots', () => { + const Parent = (_: {}) => { + Scheduler.unstable_advanceTime(1); + return ; + }; + + const Child = () => { + Scheduler.unstable_advanceTime(1); + return null; + }; + + utils.act(() => store.profilerStore.startProfiling()); + utils.act(() => render()); + utils.act(() => store.profilerStore.stopProfiling()); + + const rootID = store.roots[0]; + const profilingData = store.profilerStore.profilingData; + expect(profilingData).not.toBeNull(); + + const dataForRoot = ((profilingData: any): any).dataForRoots.get(rootID); + expect(dataForRoot).toBeDefined(); + + dataForRoot.snapshots.forEach(snapshotNode => { + snapshotNode.hocDisplayNames = (undefined: any); + }); + + expect(() => getFlamegraphChartData(rootID, 0)).not.toThrow(); + }); }); describe('ranked chart', () => { diff --git a/packages/react-devtools-shared/src/devtools/views/Profiler/FlamegraphChartBuilder.js b/packages/react-devtools-shared/src/devtools/views/Profiler/FlamegraphChartBuilder.js index 53f9f4965779..63b2250e339e 100644 --- a/packages/react-devtools-shared/src/devtools/views/Profiler/FlamegraphChartBuilder.js +++ b/packages/react-devtools-shared/src/devtools/views/Profiler/FlamegraphChartBuilder.js @@ -94,7 +94,7 @@ export function getChartData({ let maybeBadge = ''; const maybeForgetBadge = compiledWithForget ? '✨ ' : ''; - if (hocDisplayNames !== null && hocDisplayNames.length > 0) { + if (hocDisplayNames != null && hocDisplayNames.length > 0) { maybeBadge = ` (${hocDisplayNames[0]})`; }