[CP staging] Fix FlashList max-update-depth crashes from OXC React Compiler bailouts#96168
Conversation
…n nested closures OXC's React Compiler silently fails to compile a component/hook (no build warning) when a generic type expression referencing the function's own type parameters (e.g. `as TableHandle<DataType, ColumnKey, FilterKey>`, `satisfies ActiveSorting<ColumnKey>`) appears inside a nested closure passed to a hook. Babel's compiler handles the identical code fine, so this regressed after #93980 swapped OXC in for the web build. Extract the offending expression into a top-level helper function in each affected file (Table.tsx, sorting.ts, filtering.ts, useOnyx.ts) so OXC compiles and memoizes them again. Verified with `react-compiler-compliance-check check-changed`. Co-authored-by: Cursor <cursoragent@cursor.com>
Codecov Report✅ Changes either increased or maintained existing code coverage, great job!
|
06007ef ("Fix Account tab crash from OXC memoizing SVG elements") fixed a real bug (React Compiler under OXC was caching rendered <svg /> elements instead of component references) but, in doing so, replaced the useMemo-wrapped icon map construction in useMemoizedLazyIllustrations and useMemoizedLazyExpensifyIcons with a plain unmemoized loop. That meant every render produced a brand-new icons object, even when nothing relevant had changed. Table row components (e.g. WorkspaceDistanceRatesTableRow, rendered per-row inside FlashList) call useMemoizedLazyExpensifyIcons on every render, so the unstable return value fed into FlashList's own re-render/measurement cycle, eventually tripping React's "Maximum update depth exceeded" limit on pages like Workspace > Distance Rates. Found via git bisect across ~1300 commits between the last production release and main, isolating this exact commit as the regression. Wrap the icon map construction back in useMemo (keeping the resolveIconComponent fix intact) so the returned object is stable again when its inputs don't change. Verified: 3 consecutive clean reproductions on the Distance Rates page with no console errors, and confirmed the fix is present in the served bundle. Co-authored-by: Cursor <cursoragent@cursor.com>
roryabraham
changed the title
…iles identically Table.tsx, sorting.ts, filtering.ts, and useOnyx.ts no longer silently bail out of React Compiler auto-memoization on either compiler, and CI now guards against future regressions. Drop the useMemo/useCallback/ genericMemo workarounds so the compiler is the single source of truth for memoization here, verified by inspecting the compiled output on both compilers and Playwright-testing the sorting/filtering behavior. Co-authored-by: Cursor <cursoragent@cursor.com>
OXC bails on const TName / TName[number] inside hook bodies, so move logic into non-generic impls and stabilize inline importFn via useState. Co-authored-by: Cursor <cursoragent@cursor.com>
roryabraham
changed the title
OXC React Compiler skips memoization for files with type params
("Unsupported declaration type for hoisting"), which caused web
infinite update loops. Split generic hooks/components into non-
generic Impl + thin typed wrappers and drop genericMemo.
Co-authored-by: Cursor <cursoragent@cursor.com>
OXC hoisting sweep landedApplied the non-generic Audit
Playwright retest
Cleanup tracking#96209 (Monthly) — unwind Impl workarounds when oxc ships a hoisting fix. |
OXC only discovers module-scope components, so nested with* HOCs got zero web memoization. Hoist Impls, dual-compile Button/AnimatedFlatList, and delete unused genericMemo. Co-authored-by: Cursor <cursoragent@cursor.com>
Fix easy lint nits; widen no-unsafe-type-assertion baseline for unavoidable casts from generic/unknown Impl splits. Co-authored-by: Cursor <cursoragent@cursor.com>
ReturnType of the unknown-payload Impl widened SharedValue and broke ActionSheetAwareScrollView. useMemoizedLazyAsset intentionally keeps the first importFn; update the unit test to match. Co-authored-by: Cursor <cursoragent@cursor.com>
Hoist PlatformNavigatorImpl to module scope and replace dynamic hook options with getCustomState + Effects so OXC can auto-memoize. Co-authored-by: Cursor <cursoragent@cursor.com>
roryabraham
changed the title
|
Hey! I see that you made changes to our Form component. Make sure to update the docs in FORMS.md accordingly. Cheers! |
resolveIconComponent was replacing pre-rendered elements with empty PlaceholderIcon (including host <svg>), which blanked illustrations like the location permission modal. Wrap those elements as components and drop the useMemo fusion that reintroduced the Account-tab crash. Co-authored-by: Cursor <cursoragent@cursor.com>
|
Codex Review: Didn't find any major issues. You're on a roll. Reviewed commit: ℹ️ About Codex in GitHubCodex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback". |
|
AI analysis: Regression Findings (with reproduction steps)Fix: "FlashList max-update-depth crashes from OXC React Compiler bailouts" ================================================================ 1. [HIGH] Stale icon when
|
Regression scan — no blocking regressions found ✅I reviewed all 66 changed files across six parallel passes (HOC wrappers, report/policy HOCs, PlatformStack navigators, hooks, generic-component splits, list-item headers / SubStepForms / MFA). Every group came back as a clean, mechanical refactor — logic, gate conditions, hook order, effect dependency arrays, ref forwarding, and prop precedence are preserved. The one thing this PR is entirely load-bearing on — both Babel and OXC actually auto-memoizing these files — is confirmed green by CI, so the dropped manual memoization is reliably replaced. CI evidence for the load-bearing assumption: React Compiler Compliance ✅, typecheck ✅, ESLint ✅, all Jest/Bun tests ✅, and both baseline-perf + branch-perf ✅. (Only Two items are worth flagging — neither is a bug in this PR, but both are latent footguns worth a comment/awareness: 1.
|
| keyExtractor, | ||
| measuredHeightOfSuggestionRows, | ||
| }: ExternalProps<TSuggestion>) { | ||
| }: ExternalProps<unknown>) { |
There was a problem hiding this comment.
NAB: Can we avoid unknown typing?
There was a problem hiding this comment.
I've created a follow-up to clean this up
|
I dug adversarially into every "conditional" finding from the analysis above against the current tip ( The prior findings, resolved#1 — Stale illustration on #2 — Split-button dropdown ref churn (claimed MED): benign, popover still anchors correctly. The anchor element is unchanged. #3 ( What I'd actually verify (the deltas code review can't fully prove)I'm not claiming these break — CI + the traces above say they don't. But the PR removes manual memoization and leans entirely on the compiler, and that only fails at runtime, so these are the precise smokes worth 5 minutes:
Bottom line: green CI (React Compiler Compliance, typecheck, all tests, baseline+branch perf), plus each flagged finding traced to "safe" against the current tip. I found nothing reproducible that should block this. |
jasperhuangg
changed the title
|
🚧 jasperhuangg has triggered a test Expensify/App build. You can view the workflow run here. |
|
🧪🧪 Use the links below to test this adhoc build on Android, iOS, and Web. Happy testing! 🧪🧪
|
|
✋ This PR was not deployed to staging yet because QA is ongoing. It will be automatically deployed to staging after the next production release. |
…c-closure-bailout Fix FlashList max-update-depth crashes from OXC React Compiler bailouts (cherry picked from commit 5952b09) (cherry-picked to staging by jasperhuangg) Co-authored-by: Cursor <cursoragent@cursor.com>
…454712293-1 🍒 Cherry pick PR #96168 to staging 🍒
|
🚀 Cherry-picked to staging by https://github.com/jasperhuangg in version: 9.4.34-13 🚀
|
Help site review — no docs changes required ✅I reviewed all 66 changed files against the help site under Why: This PR is a purely internal React Compiler (OXC) compliance refactor — splitting generic components into The help site articles document product behavior and UI, none of which this PR alters, so there is nothing to add, remove, or correct. If a future revision of this PR introduces any user-visible change, re-tag me and I'll draft the corresponding docs update. @roryabraham — no help site PR was created because no documentation changes are required for this refactor. Let me know if you'd like me to reconsider any specific article. |
|
🚀 Cherry-picked to staging by https://github.com/jasperhuangg in version: 9.4.34-14 🚀
|
No help site changes required — no draft PR created ✅I reviewed all 66 changed files in this PR against the help site content under Why: This is a purely internal refactor to satisfy the OXC React Compiler — splitting generic components into Help site articles document product behavior and user-facing flows. Since nothing a user sees or does changes here (aside from screens no longer crashing), there is nothing in If any of the affected screens (e.g. Workspace → Distance Rates, LHN, workspace tables) had known-outdated documentation independent of this PR, that would be a separate docs task — but this changeset itself doesn't create a docs need. @roryabraham — no linked help site PR was created because no docs changes are required. If you believe a specific user-facing behavior did change and should be documented, reply with the detail and I'll draft the update. |
|
🚀 Deployed to production by https://github.com/roryabraham in version: 9.4.34-14 🚀
|
|
🚀 Cherry-picked to staging by https://github.com/jasperhuangg in version: 9.4.34-14 🚀
|
Help site review — no docs changes required ✅I reviewed all 66 changed files in this PR against This is a purely internal refactor to satisfy the OXC React Compiler — splitting generic components into If a reviewer believes any user-visible behavior does change here (e.g. a screen that now renders or navigates differently), reply with |
|
🚀 Deployed to production by https://github.com/roryabraham in version: 9.4.34-14 🚀
Bundle Size Analysis (Sentry): |
|
🚀 Cherry-picked to staging by https://github.com/jasperhuangg in version: 9.4.35-1 🚀
|
No help site changes required ✅I reviewed all 66 changed files in this PR and confirmed no updates to Why: This is a purely internal refactor to fix FlashList "Maximum update depth exceeded" crashes caused by OXC React Compiler bailouts. Every change is in The help articles under If you'd expected a specific feature's behavior to be user-visible here (e.g. a screen that now looks or behaves differently to end users), let me know which one and I'll re-scope the docs check against that. |
|
🚀 Deployed to production by https://github.com/jasperhuangg in version: 9.4.35-6 🚀
|
5 participants
Explanation of Change
Web FlashList pages were hitting "Maximum update depth exceeded" because OXC's React Compiler often fails to memoize files that Babel still does. Unstable props then churn FlashList rows into an infinite update loop.
The Distance Rates crash was isolated (via bisect) to unstable icon maps from
useLazyAssetafter an SVG-memoization fix dropped auseMemo. Fixing that alone was not enough for the wider class of OXC bailouts, so this PR also restores dual Babel/OXC auto-memoization for the patterns we hit:Unsupported declaration type for hoisting). Split into a non-generic*Impl+ thin typed wrapper, then drop compensatory manual memo.*Impls (and for platform stack navigators, replace dynamicuseCustomState/useCustomEffectswithgetCustomState+ anEffectscomponent slot).Leftovers that still diverge (RNGH ref false positives, intentional
useSidebarOrderedReportsprovider bailout) are tracked in #96209 for cleanup once upstream compilers improve. That issue also tracks unwinding the Impl workarounds when OXC fixes generic hoisting.Fixed Issues
$ #96161
$ #96207
(probably #96204 too - I had one chance to test it on dev and it worked, but there's no obvious repeatable repro)
Tests
Offline tests
N/A — restores memoization / compiler eligibility; no new offline-specific behavior.
QA Steps
Same as tests.
PR Author Checklist
### Fixed Issuessection aboveTestssectionOffline stepssectionQA stepssectioncanBeMissingparam foruseOnyxtoggleReportand notonIconClick)src/languages/*files and using the translation methodSTYLE.md) were followedAvatar, I verified the components usingAvatarare working as expected)StyleUtils.getBackgroundAndBorderStyle(theme.componentBG))npm run compress-svg)Avataris modified, I verified thatAvataris working as expected in all cases)Designlabel and/or tagged@Expensify/designso the design team can review the changes.ScrollViewcomponent to make it scrollable when more elements are added to the page.mainbranch was merged into this PR after a review, I tested again and verified the outcome was still expected according to theTeststeps.Screenshots