Synced from GitHub
fix(mobile): branch picker no longer renders twice per update#22132
Note
🤖 Claude Opus 5.5 on behalf of Oliver
On mobile, the branch picker rendered twice for every update. After the React Compiler migration #21137, usePaginatedBranches built a new Atom.make pages atom on every render, and the compiler didn't cache it. useAtomValue then subscribed to a fresh atom each time, which forced a second render, and refresh changed identity every render.
The fix keys the pages atom through an Atom.family on the target and its cursors, so the same branch list always gets the same atom, with no useMemo. Initial cursors use null instead of undefined so the key survives JSON.stringify.
Proof:
- In a jsdom harness running the compiled hook under a real
AtomRegistry, 5 parent updates caused 10 renders onmainand 5 here, andrefreshchanged identity 11 times onmainand 0 here. - Ran
vp test run apps/mobile/src/reactCompilerCoverage.test.ts,queries.test.tsandqueries.filesystem.test.ts, plus mobiletypecheck. - No regression test: the double render only appears in compiled output, and the mobile test runner doesn't apply the React Compiler, so a render test passes on
maintoo. ThecompilerCachedCallshelper from #22913 doesn't fit either, because the fix stops creating the atom during render instead of caching it. - Not checked: a real device or simulator. There is no visible UI change, so no screenshots.
Claude Opus 5.5 via Claude Code in T3 Code
Checks
21 successful · 11 skipped
Synced from GitHub
Approvability
Verdict: Approved at 5fea58d
Macroscope's review found this PR approvable — This is an isolated mobile performance refactor that stabilizes paginated branch-query atoms and removes redundant rendering without changing branch data, pagination semantics, or product defaults. The limited runtime impact and contained scope make it low risk.
You can add or adjust custom eligibility rules. Learn more.
Synced from GitHub
No actionable comments were generated in the recent review. 🎉
ℹ️ Recent review info
⚙️ Run configuration
- Configuration used: Path: .coderabbit.config.ts
- Review profile: CHILL
- Plan: Advanced
- Run ID:
9d634c4d-9282-4349-a819-c41a76ff673e
📥 Commits
Reviewing files that changed from the base of the PR and between e0f853c66682ad192bb865a72b4302035bc9c31e and 5fea58de7d5d28e89fbbbf2e8f163b8495c529e9.
📒 Files selected for processing: 1
apps/mobile/src/state/queries.ts
Included review availability: This review used your included allowance. Your plan provides up to 10 included reviews per hour; 6 remain after this review.
📝 Walkthrough
usePaginatedBranchesnow uses shared keyed atoms for branch-page fetching and aggregation. Initial cursors usenull. Refresh targets the first shared branch-page atom.Changes: Branch pagination
Layer / File(s) Summary Shared branch page atoms
apps/mobile/src/state/queries.tsThe pagination state uses nullfor initial cursors. Shared atoms fetch and aggregate branch pages by serialized target key. The hook uses the keyed atom family and refreshes the first shared page atom when the environment and working directory exist.Priority: ⬇️ Low
Estimated code review effort: 2 (Simple) | ~10 minutes
Change: Bug fix
Suggested reviewers:
juliusmarminge
Merge Risk: ⚪ Minimal · up to 5fea5
The pagination change is ready to merge after normal checks; no branch-loading or refresh regression was identified.
Security architecture risk: 🔵 Low · up to 5fea5
The shared state remains separated by environment, repository directory, search query, and page cursors. No introduced security defect was identified, and the existing request authority is preserved. Upstream access enforcement was not fully verified.
Retained concerns No architecture-level concerns identified.
Security review details
Security Blast Radius
- inferred — The demonstrated change affects branch-reference reads in the mobile new-task and scheduled-task pickers. Aggregate sharing is limited to identical environment, directory, query, and cursor keys; the inspected change adds no request authority or repository mutation. Upstream endpoint exposure and authorization remain unverified.
Trust Boundaries and Controls
- observed — Page requests continue through the existing environment-scoped runtime to vcsListRefs. The aggregate key preserves the same environment and request-selection fields used by the underlying page family. This supports unchanged client-side isolation, but does not establish that upstream access enforcement is sufficient.
Pre-merge checks: Passed 4
Check name Status Explanation Linked Issues check Passed Check skipped because no linked issues were found for this pull request. Out of Scope Changes check Passed Check skipped because no linked issues were found for this pull request. Title check Passed The title clearly identifies the branch picker rendering regression, which matches the pull request’s main change. Description check Passed The description clearly explains the rendering problem, the implementation change, measured results, tests, limitations, and agent attribution. It does not explicitly provide a Scope and approval sect… Full details: Description check
Explanation
The description clearly explains the rendering problem, the implementation change, measured results, tests, limitations, and agent attribution. It does not explicitly provide a Scope and approval section or state why the focused fix qualifies without prior approval, but the linked related PRs and detailed technical context make the description mostly complete.
✨ Finishing Touches
🧪 Generate unit tests (beta)
- 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.
Synced from GitHub
Added 1 commit
- fix(mobile): branch picker no longer renders twice per update3d915e4
Folded into #22134. The branch hook returning new loadNext, refresh and branch lists on every render is what kept the new-task context changing even with #22134's provider fix, so the two belong in one PR. The commit moved over unchanged.