Sign up
View stargazersSign up

perf(mobile): settings screens compile with the React Compiler#21119

merged2
main
perf/mobile-compile-1
opened12d agobyJJulius Marminge
7 files changed
View on GitHubTwo-way sync enabledPR last imported 10d ago

Six settings screens were skipped by the React Compiler: About, Environments, Environment detail, Notifications, Project overview and Provider accounts. All six used try/finally or try without catch, so they shipped unmemoized.

Each screen now does its cleanup in .finally() on the promise. Where the awaited call is an atom command, the cleanup simply runs after the await, because atom commands settle into a result and never reject. The Notifications live-activity disable body moved into a named async function so the in-flight ref resets through .finally(). With the compiler covering these files, their manual useCallback/useMemo are gone. KNOWN_BAILOUTS: 84 → 78.

Verified on device (Pixel 9 API 36 emulator, dev client built from main, isolated seeded server):

  • Environments: list renders, opened the connected environment's detail.
  • Environment detail: Check for updates ran to "You are up to date". Refresh providers showed "Provider status refreshed."
  • Project overview: picked a project from the settings filter, renamed it (title updated on save), then renamed it back.
  • Provider accounts, About and Notifications render.
  • No red box, and no useMemoCache or React errors in the Metro log.

Renders while opening each screen from Home (measure-renders --route, median of 3, same base and device):

RouteBeforeAfter
/settings/environments12681268
/settings/notifications720620
/settings/about886886
/settings/provider-accounts10451045

No measurable change on three of the screens: these are mount-and-settle windows, and the components barely re-render after mount. Notifications drops by one wrapper re-render pass (720 → 620 in 2 of 3 runs).

Review follow-up

  • Settings: .finally/.catch chains replaced. Commands (provider sign-in, project rename, cloud refresh, live-activity writers) are awaited, with the reset right after. runAppUpdateCheck, the environment action runner and findEnvironmentUpdate go through settlePromise. The provider-accounts .catch(() => null) is gone, because commands never reject.
  • On device: environment detail → Check for updates and Refresh providers both complete and reset ("Provider status refreshed."). With the server stopped, the rows go disabled and no spinner is left running.

Provider refresh succeeded After stopping the server: rows disabled, no stuck spinner

Device: Pixel 9 API 36 emulator on nucbox-1, client rebuilt from perf/mobile-compile-8 (the old one lacked the T3FoldPosture native view), Metro and an isolated seeded server on cups.

Claude Opus 5.5 via Claude Code.

🤖 Generated with Claude Code

Checks

20 successful · 12 skipped
All checks passed
4a7e5dc
MmacroscopeappBot12d ago
Synced from GitHub

Approvability

Verdict: Needs Human Approval

Macroscope's review found this PR requires human approval — This is a focused React Compiler compatibility refactor, but it changes existing provider sign-in and authenticated relay-token settings flows. Those authentication-sensitive production changes require human review under the approval criteria.

No code changes detected at 4a7e5dc. Prior analysis still applies.

You can add or adjust custom eligibility rules. Learn more.

CcoderabbitaiBot12d ago
Synced from GitHub

Review in Change Stack →

Important

Review skipped

Review was skipped as selected files did not have any reviewable changes.

⚙️ Run configuration
  • Configuration used: Path: .coderabbit.config.ts
  • Review profile: CHILL
  • Plan: Team
  • Run ID: 9eba21fe-87a3-448e-8f5f-abd615f79f20
📥 Commits

Reviewing files that changed from the base of the PR and between 1d083f137157e1d68f8f0362ed6d84f81e02aeb7 and 4a7e5dcb861dc1c0cadae36047ae31c8139a46cb.

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

  • 🔍 Trigger review

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration
  • Configuration used: Path: .coderabbit.config.ts
  • Review profile: CHILL
  • Plan: Team
  • Run ID: da103372-4725-47d0-a7f0-e5eb0a3147de
📥 Commits

Reviewing files that changed from the base of the PR and between 9ccf6d967d9f6fa55db5794c0b1ecca66fda0ccb and f64ca20173e42ad02795c22fbc04cea82787c707.

📒 Files selected for processing: 4
  • apps/mobile/src/features/settings/SettingsAboutRouteScreen.tsx
  • apps/mobile/src/features/settings/SettingsEnvironmentDetailRouteScreen.tsx
  • apps/mobile/src/features/settings/SettingsNotificationsRouteScreen.tsx
  • apps/mobile/src/features/settings/SettingsProviderAccountsRouteScreen.tsx

Included review availability: This review used your included allowance. Your plan provides up to 10 included reviews per hour; 3 remain after this review.

📝 Walkthrough

Several mobile settings screens replace memoized callbacks with ordinary functions. The changes also alter asynchronous error and state handling. The React Compiler coverage test removes six settings screens from its known bailout list.

Changes: Mobile settings updates

Layer / File(s)Summary
Settings callbacks and compiler coverage
apps/mobile/src/features/settings/SettingsAboutRouteScreen.tsx, apps/mobile/src/features/settings/SettingsEnvironmentsRouteScreen.tsx, apps/mobile/src/reactCompilerCoverage.test.ts		About and environments handlers become ordinary functions. Cloud refresh clears pending and refreshing state after a successful refresh. The coverage test removes six screens from KNOWN_BAILOUTS.
Notification and Live Activity handlers
apps/mobile/src/features/settings/SettingsNotificationsRouteScreen.tsx		The screen computes saved connections on each render and declares handlers as ordinary functions. It extracts Live Activity disabling into a separate function. The handler awaits operations before resetting its in-flight flag.
Settings operation outcomes
apps/mobile/src/features/settings/SettingsEnvironmentDetailRouteScreen.tsx, apps/mobile/src/features/settings/SettingsProjectOverviewRouteScreen.tsx, apps/mobile/src/features/settings/SettingsProviderAccountsRouteScreen.tsx		Environment-detail operations use settlePromise to process results. Project-name saving and provider-account commands change when pending state resets and how rejected promises are handled.

Priority: ⬇️ Low

Estimated code review effort: 3 (Moderate) | ~20 minutes

Change: Refactor

Merge Risk: ⚪ Minimal · up to f64ca

The settings changes appear ready to merge after normal checks; no blocking behavior was identified.

Pre-merge checks: Passed 3 · Failed 1

Failed Failed checks: 1 warning

Check nameStatusExplanationResolution
Description checkWarningThe description clearly explains the problem, implementation, and verification results, including device testing, render measurements, screenshots, and the agent used. It does not provide the required…Add the required Scope and approval section. Link the triaged issue or approval discussion with the maintainer approval comment, or explain why this focused change qualifies for the no-approval exception.
Passed Passed checks: 3 passed
Check nameStatusExplanation
Linked Issues checkPassedCheck skipped because no linked issues were found for this pull request.
Out of Scope Changes checkPassedCheck skipped because no linked issues were found for this pull request.
Title checkPassedThe title clearly and concisely identifies the main change: enabling React Compiler compilation for the mobile settings screens.
Full details: Description check

Explanation

The description clearly explains the problem, implementation, and verification results, including device testing, render measurements, screenshots, and the agent used. It does not provide the required Scope and approval information, such as a triaged issue, maintainer approval, or justification for proceeding without one.

Resolution

Add the required Scope and approval section. Link the triaged issue or approval discussion with the maintainer approval comment, or explain why this focused change qualifies for the no-approval exception.

✨ Finishing Touches
📝 Generate docstrings
  • Commit to this branch
  • Create a new PR
🧪 Generate unit tests (beta)
  • Commit to this branch
  • Create a new PR
  • Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

Comment @coderabbitai help to get the list of available commands.

Ggithub-actionsBot12d ago
Synced from GitHub

Thread transfer impact

⚠️ The latest CI run did not produce a thread transfer result for 1d083f1.

This comment will update automatically after the next completed run.

Added 2 commits

  • refactor(mobile): settings screens read settled results instead of promise chains4a7e5dc
  • perf(mobile): settings screens compile with the React Compiler643f042