feat(code): migrate renderer to TanStack Router#2469
Open
adamleithp wants to merge 4 commits into
Open
Conversation
Collapses the three-PR stack (#2455 file-based routing, #2456 architectural cleanup, #2457 navigationStore → router-derived facade) into a single change, rebased on main and stabilized. No intermediate layer was independently green (the base failed typecheck/unit and conflicted with main), so the stack was providing false review granularity; one PR is one green CI surface and one review. ## Migration - File-based routing via the TanStack Router Vite plugin (autoCodeSplitting), hash history for Electron's loadFile, cold-boot last-route restore. - Delete navigationStore + settingsDialogStore. URL is the source of truth; remaining transient state lives in small stores (settingsPageStore, taskInputPrefillStore). - Settings becomes a full-page route; SettingsDialog kept for the AI-consent gate via an imperative openSettingsDialog(). ## Stabilization (defects found shaking the stack out) - Break the import cycle (router → routeTree → __root → hooks → navigationBridge → router) that broke code-split chunks via TDZ. Added routerRef (leaf module); navigationBridge reads it and degrades to a no-op when the router isn't mounted (early boot / tests) rather than throwing. - Memoize useAppView on route primitives + stable prefill ref. It previously returned a fresh object every render, turning SidebarMenu's [view] effect into an infinite markViewed loop that starved the UI thread. - Task-detail loader is synchronous + cache-only so it can never hang the router on optimistic/cloud-pending tasks; the cold-deep-link fetch + spinner live in the component. openTask seeds the detail cache. ## Per-route loading Router context (queryClient), createRootRouteWithContext, defaultPendingMs: 0 + defaultPendingComponent (RoutePending) — navigation commits instantly with a per-route pending slot ready to become skeletons. ## Merge resolution (vs main) - AiApprovalScreen: kept main's in-app approve mutation, applied the settingsDialogStore → openSettingsDialog migration on top. - authStore: kept main's org-switching (orgProjectsMap/switchOrg/current*), migrated its new switchOrg + logout off the deleted stores. - AiApprovalScreen.test: hoist spies (vi.hoisted), mock openSettingsDialog. ## Tooling biome ignores the generated routeTree.gen.ts (lint/format/assist); marked linguist-generated. Typecheck, biome, and all 1594 renderer unit tests pass. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
This was referenced Jun 2, 2026
Contributor
Prompt To Fix All With AIFix the following 3 code review issues. Work through them one at a time, proposing concise fixes.
---
### Issue 1 of 3
apps/code/src/renderer/navigationBridge.ts:48-50
The `COMMAND_CENTER_VIEWED` analytics event was tracked in the old `navigationStore.navigateToCommandCenter` action but is now silently dropped. After this PR, `COMMAND_CENTER_VIEWED` is never fired anywhere in the codebase — `navigationBridge.navigateToCommandCenter` does a bare router navigate, and the route component adds no tracking call. The `track` import and the constant are both still present in the analytics types, so no compiler error catches this.
```suggestion
export function navigateToCommandCenter(): void {
void getRouterOrNull()?.navigate({ to: "/command-center" });
track(ANALYTICS_EVENTS.COMMAND_CENTER_VIEWED);
}
```
### Issue 2 of 3
apps/code/src/renderer/hooks/useAppView.ts:48-55
**Stale `data` field in the task-detail branch** — `getCachedTask(taskId)` is called inside `deriveFromMatches`, which runs inside a `useMemo` that only re-computes when route primitives change (`routeId`, `taskId`, etc.). If the React Query cache for that task is updated by a poll, mutation, or subscription while the user stays on the same task-detail route, the memo won't re-fire and `view.data` will silently carry stale task data. Any consumer reading `view.data` for live content (rather than using `useTasks()` / `useQuery()` directly) will render out-of-date information. The PR description acknowledges deleting `useAppView` in a P0 follow-up; in the meantime callers should prefer `view.taskId` and their own query hooks over `view.data`.
### Issue 3 of 3
apps/code/src/renderer/hooks/useAppView.ts:22-25
**Duplicate `TaskInputReportAssociation` type** — the same interface is declared here and again in `taskInputPrefillStore.ts`. The two definitions are structurally identical today but can drift. Consider re-exporting the single canonical type from `taskInputPrefillStore.ts` (or `types.ts`) and importing it here instead.
Reviews (1): Last reviewed commit: "feat(code): migrate renderer to TanStack..." | Re-trigger Greptile |
Switching settings categories pushed a history entry each time, so closeSettings' single history.back() walked back through previously-visited categories (environments → general → app) instead of leaving settings. Make in-settings category switches replace the history entry; opening settings still pushes one entry, so "Back to app" returns directly to the route you came from. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…a, dedup type - Restore COMMAND_CENTER_VIEWED analytics: the pre-router navigationStore emitted it on navigate; navigationBridge.navigateToCommandCenter now does too (TASK_VIEWED parity is already covered by openTask). - Remove the memoized `AppView.data` snapshot, which could go stale while the user stayed on a task (the memo only recomputes on route-primitive changes). HeaderRow now reads the live task via useTasks() keyed on view.taskId, and useSidebarData uses view.taskId — both equivalent cache coverage but live. - Dedup TaskInputReportAssociation: import the canonical type from taskInputPrefillStore instead of redeclaring it in useAppView. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Contributor
Author
|
Addressed the review comments in
typecheck, biome, and all 1594 unit tests green. |
handleTaskClick looked the task up in taskMap (the full-list query) and silently did nothing on a miss. Sidebar rows come from the summaries path, which can include tasks the list query doesn't carry. Fall back to navigating by id — the task-detail route resolves the task from its own query. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Consolidates the TanStack Router migration stack — #2455 (file-based routing), #2456 (architectural cleanup), #2457 (navigationStore → router-derived facade) — into one PR, rebased on
mainand stabilized.Why one PR instead of the stack
No layer was independently shippable: the base (#2455) failed typecheck + unit and conflicted with
main, and #2457 rewrites much of what #2455/#2456 introduced. The stack gave false review granularity and 3× the CI/rebase/conflict work. Collapsing makes the broken intermediate-state failures vanish (≈2/3 of the red checks) and leaves one green CI surface + one review. Supersedes #2455, #2456, #2457.What's in it
Migration
autoCodeSplitting), hash history for ElectronloadFile, cold-boot last-route restore.navigationStore+settingsDialogStore; URL is the source of truth. Transient state moves to small stores (settingsPageStore,taskInputPrefillStore).SettingsDialogretained for the AI-consent gate via imperativeopenSettingsDialog().Stabilization (defects found shaking the stack out)
router → routeTree → __root → hooks → navigationBridge → routerbroke code-split chunks (TDZ). AddedrouterRef(leaf module); the bridge degrades to a no-op when the router isn't mounted instead of throwing.useAppViewreturned a fresh object every render →SidebarMenu's[view]effect looped (markViewed~50×/2s), starving the UI thread. Now memoized on route primitives + stable prefill ref.openTaskseeds the detail cache.Per-route loading UX
context: { queryClient },createRootRouteWithContext,defaultPendingMs: 0+defaultPendingComponent(RoutePending). Navigation commits instantly with a per-route pending slot ready for skeletons.Merge resolution vs main
AiApprovalScreen: kept main's in-app approve mutation + applied the store migration.authStore: kept main's org-switching (orgProjectsMap/switchOrg/current*), migratedswitchOrg+logoutoff the deleted stores.routeTree.gen.ts; markedlinguist-generated.Verification
typecheckbiome check(whole repo)Follow-ups (separate PRs, post-merge)
useAppView→ primitiveuseRouterStateselectors + per-routeuseParams(removes the render-storm class permanently).navigationBridge→ typed<Link>/useNavigate()(removes the import cycle structurally;routerRefcan go).defaultErrorComponent+defaultNotFoundComponent(no error boundaries today).queryOptionsloader pattern + per-route skeletons across the other data routes.validateSearch) to retiresettingsPageStore/taskInputPrefillStore; auth asbeforeLoadguards; tunedefaultPendingMs.🤖 Generated with Claude Code
🧪 Manual testing checklist
Routing now drives app boot, auth gating, and every screen. Check each area on a fully reloaded build (Cmd+Shift+R, or restart
pnpm dev:code) — HMR can't safely apply these graph changes.App boot & route restore
/code(new-task input), no flash of the wrong screen/settings/environments,/skills)markViewed/ IPC-rate spam in the terminal at idle (render-storm regression)Onboarding & auth gates (App.tsx gates the RouterProvider)
/codeAuth lifecycle (authStore merge with main's org-switching)
New task flow (loader / storm hotspot)
/code/tasks/pending/$key) shows then transitions to the real taskNavigation between routes
Per-route loading
Settings (full-page route)
/settingsredirects to/settings/general/codeon deep-link entryDeep links (warm + cold)
new,plan, andissueactions land on task input with prefill/folders/$folderIdOther routes
/folders/$folderId) loadsRegressions to watch