Repository navigation
perf(mobile): pause relative-time clocks while backgrounded - #9886
Conversation
Co-authored-by: Orca <help@stably.ai>
📝 WalkthroughWalkthroughUpdated 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches📝 Generate docstrings
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
🧹 Nitpick comments (3)
mobile/src/hooks/use-now.ts (1)
4-25: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winDocument the non-obvious lifecycle constraints.
mobile/src/hooks/use-now.ts#L4-L25: Add a briefWhy:comment explaining why AppState ownership is shared and ref-counted.mobile/src/hooks/use-now.test.ts#L5-L21: Explain why the AppState mock must usevi.hoisted.mobile/app/h/[hostId]/accounts.tsx#L44-L53: Explain why reset-label updates are enabled only while this route is focused.As per coding guidelines, “For non-obvious TypeScript code, add a concise comment explaining why the constraint exists, not how the code works.”
Source: Coding guidelines
mobile/src/hooks/use-now.test.ts (2)
1-3: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAvoid adding deprecated
react-test-renderercoverage.React 19 deprecates this renderer, and the test suppresses its warning. Port this to the repository’s supported testing-library setup and remove the suppression.
Also applies to: 50-56
95-100: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winTest the multi-caller ref-count contract.
This only proves cleanup for one caller. Mount two harnesses, unmount one and assert
removewas not called; then unmount the last and assert it was called once.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: a46ac347-96d9-4f02-899b-3fac39a2ad1c
📒 Files selected for processing (3)
mobile/app/h/[hostId]/accounts.tsxmobile/src/hooks/use-now.test.tsmobile/src/hooks/use-now.ts
Co-authored-by: Orca <help@stably.ai>
What
The mobile
useNowhook drives relative-time labels ("2m ago") by re-rendering on a fixed interval — the host list every 30s, Accounts every 60s. It kept ticking while the app was backgrounded and for retained-but-covered routes, forcing re-renders (e.g. a whole host-list render) that nobody could see.Fix: make
useNowAppState-aware — the interval runs only while the app isactive, and it ticks once immediately on returning to active so labels are correct on resume. A single shared, ref-countedAppStatelistener (viauseSyncExternalStore) serves alluseNowcallers rather than one listener per hook instance. An optionalenabledflag lets a routed screen (Accounts) also pause while unfocused via its existing focus hook.ELI5
Your phone kept re-drawing "X minutes ago" timestamps every 30–60 seconds even while the app was in your pocket. Now those clocks pause while the app is backgrounded and instantly refresh when you reopen it.
Proof of zero regression
useNowinstance unmounts (ref-counted).use-now.test.ts3/3 (active updates, background suspension, immediate foreground refresh, focus disable/re-enable, listener cleanup) and full mobile suite green.typecheck/oxlint/oxfmt --checkclean.Proof of perf improvement
Stops the 30s/60s forced re-renders (including whole host-list renders) while the app is backgrounded or the route is covered — low per-tick cost but broad and steady, and fully parked in the background.
Made with Orca 🐋