Skip to content

feat(code): introduce TanStack Router file-based routing#2455

Closed
adamleithp wants to merge 1 commit into
mainfrom
feat/tanstack-router
Closed

feat(code): introduce TanStack Router file-based routing#2455
adamleithp wants to merge 1 commit into
mainfrom
feat/tanstack-router

Conversation

@adamleithp
Copy link
Copy Markdown
Contributor

Summary

  • Replaces the Zustand useNavigationStore view switch in MainLayout with TanStack Router (hash history for Electron file:// compatibility).
  • 10 file-based routes; task surfaces nest under /code/*, settings at /settings/$category, infra views (command-center, skills, mcp-servers, folders) at the root so future verticals have room.
  • navigationStore + settingsDialogStore kept alive as transitional shims that mirror every action to router.navigate(...). The ~35 nav-store and ~20 settings-dialog consumers keep working unchanged.
  • TanStack Router DevTools mounted in import.meta.env.DEV.

Route map

URL Component
/ redirect → /code
/code TaskInput
/code/tasks/$taskId TaskDetail
/code/tasks/pending/$key TaskPendingView
/code/inbox InboxView
/code/archived ArchivedTasksView
/settings redirect → /settings/general
/settings/$category opens SettingsDialog modal (see follow-up #2)
/command-center CommandCenterView
/skills SkillsView
/mcp-servers McpServersView
/folders/$folderId FolderSettingsView

Known gaps — follow-up PRs

Captured here so reviewers don't read this as the chosen end-state architecture.

PR 2 — small cleanup (~half day)

  • Check in routeTree.gen.ts (currently gitignored — fresh-clone typecheck fails without running pnpm dev/build first).
  • Fix TanStack Router DevTools tree-shaking — current conditional render keeps the devtools chunk in prod bundle (~50–80KB). Switch to React.lazy + conditional dynamic import.
  • Drop the as string cast in syncToRouter — guard already prevents the hazard, cast just hides the type narrowing.
  • Settings close() currently always navigates to /code, losing back context. Switch to router.history.back() (combined with the existing window.history.back() in the modal close, pick exactly one).

PR 3 — Settings: commit to page or modal (~1 day)

Today /settings/$category renders null and toggles a Zustand isOpen flag, which mounts a Radix Dialog from __root. The URL changes but the chrome (sidebar, header) still shows underneath — incoherent. Either:

  • Extract SettingsPanel from the dialog, render it from the route as a real page, use a pathless layout route to hide MainSidebar. OR
  • Drop the route entirely and keep settings as a modal trigger.

Also: registerBillingSubscriptions auto-opens plan-usage settings on limit hit — that's now a navigation, not a modal-over-current-view. Verify the UX hasn't regressed for users mid-task.

PR 4 — Delete the nav store (the big one)

This is the actual unlock. Today the store and router are dual sources of truth for navigation; syncToRouter keeps them aligned. Cost:

  • Every nav action runs twice (Zustand set, then router.navigate).
  • Two writers, no clear arbiter — the bug I had to fix mid-implementation (TaskDetailRoute fighting in-flight navigation) is a direct symptom.
  • Adding a new view requires updating three places (store action, syncToRouter switch, route component).

Plan:

  1. Introduce apps/code/src/renderer/navigationBridge.ts that imports router and exports syncToRouter. Both stores import the bridge, not the router directly. Breaks the current store → router → routeTree → __root → store circular import (currently works by ES module live binding luck — fragile).
  2. Port the ~35 useNavigationStore consumers to useParams/useNavigate/<Link>.
  3. Delete navigationStore.ts, useNavigationStore.ts, navigationStore.test.ts.

PR 5 — Polish

  • Route loaders for TaskDetail, Inbox, FolderSettings. defaultPreload: "intent" is enabled but unused.
  • Evaluate autoCodeSplitting: true — for an Electron app loading from local disk, splitting 12 tiny route chunks may net negative on parse cost. Measure first.
  • Wire menu Cmd+[ / Cmd+] back/forward to router.history.go(±1) once nav store's own history stack is gone.
  • Fix cold-boot flicker — users with a persisted task currently see TaskInput briefly while hydrateTask resolves; restore last URL synchronously at router creation.

Test plan

Automated (passing):

  • Renderer typecheck (pnpm exec tsc -p tsconfig.web.json --noEmit)
  • Vitest suite — 131 files / 1598 tests pass
  • Biome lint clean on all new/changed files
  • Renderer Vite build succeeds

Manual (please verify before merge — I haven't run the Electron app end-to-end):

  • App boots to /code (new task screen)
  • Click task in sidebar → URL hash becomes #/code/tasks/<id>, TaskDetail renders, sidebar highlight follows
  • Click another task → URL + content + highlight all swap correctly
  • Click Inbox / Archived / Command Center / Skills / MCP Servers → each navigates and renders
  • Click "New task" → returns to /code with TaskInput
  • Open Settings via gear icon → URL becomes #/settings/general, dialog opens
  • Switch categories within settings → URL hash updates
  • Close settings via X → dialog closes, returns to /code
  • Deep link posthog-code://...?task=... → opens correct task and URL
  • Quit + relaunch → restores last task (note: brief TaskInput flicker is a known gap, see PR 5)

🤖 Generated with Claude Code

Replaces the Zustand-driven view switch in MainLayout with TanStack Router
(hash history for Electron file:// compatibility). Routes nest under
`/code/*` for task surfaces, leaving room for future verticals at the root.

- 10 routes generated from `apps/code/src/renderer/routes/`:
  - `/` → redirect to `/code`
  - `/code` → TaskInput
  - `/code/tasks/$taskId` → TaskDetail
  - `/code/tasks/pending/$key` → TaskPendingView
  - `/code/inbox`, `/code/archived`
  - `/settings/$category` (+ `/settings` redirect to `general`)
  - `/command-center`, `/skills`, `/mcp-servers`
  - `/folders/$folderId`
- `__root.tsx` keeps existing chrome (HeaderRow, MainSidebar, SpaceSwitcher,
  modals, deep-link hooks) and renders `<Outlet/>` where the view-switch was.
- `navigationStore` and `settingsDialogStore` kept as transitional shims that
  mirror their actions to `router.navigate(...)` via a new `syncToRouter`
  helper, so the ~35 nav-store and ~20 settings-dialog consumers keep working
  without per-call migration.
- TanStack Router DevTools mounted in dev only.

Follow-up PRs noted in the PR description.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
@greptile-apps
Copy link
Copy Markdown
Contributor

greptile-apps Bot commented Jun 2, 2026

Comments Outside Diff (1)

  1. apps/code/src/renderer/features/settings/stores/settingsDialogStore.ts, line 55-59 (link)

    P1 The window.history.pushState({ settingsOpen: true }, "") call creates a history entry, but the immediately following router.navigate(...) pushes a second entry whose state is TanStack Router's internal state — not { settingsOpen: true }. By the time close() runs, window.history.state holds the router's state, so window.history.state?.settingsOpen is always falsy and the window.history.back() branch in close() is never reached. The settingsOpen placeholder entry is never consumed, so every open→close cycle permanently pollutes the back-button stack with a phantom entry.

    Prompt To Fix With AI
    This is a comment left during a code review.
    Path: apps/code/src/renderer/features/settings/stores/settingsDialogStore.ts
    Line: 55-59
    
    Comment:
    The `window.history.pushState({ settingsOpen: true }, "")` call creates a history entry, but the immediately following `router.navigate(...)` pushes a second entry whose state is TanStack Router's internal state — not `{ settingsOpen: true }`. By the time `close()` runs, `window.history.state` holds the router's state, so `window.history.state?.settingsOpen` is always falsy and the `window.history.back()` branch in `close()` is never reached. The `settingsOpen` placeholder entry is never consumed, so every open→close cycle permanently pollutes the back-button stack with a phantom entry.
    
    
    
    How can I resolve this? If you propose a fix, please make it concise.
Prompt To Fix All With AI
Fix the following 3 code review issues. Work through them one at a time, proposing concise fixes.

---

### Issue 1 of 3
apps/code/src/renderer/features/settings/stores/settingsDialogStore.ts:55-59
The `window.history.pushState({ settingsOpen: true }, "")` call creates a history entry, but the immediately following `router.navigate(...)` pushes a second entry whose state is TanStack Router's internal state — not `{ settingsOpen: true }`. By the time `close()` runs, `window.history.state` holds the router's state, so `window.history.state?.settingsOpen` is always falsy and the `window.history.back()` branch in `close()` is never reached. The `settingsOpen` placeholder entry is never consumed, so every open→close cycle permanently pollutes the back-button stack with a phantom entry.

```suggestion
    open: (category, contextOrAction) => {
      const isAction = typeof contextOrAction === "string";
```

### Issue 2 of 3
apps/code/src/renderer/features/settings/stores/settingsDialogStore.ts:73-78
Since the `window.history.pushState({ settingsOpen: true })` call in `open()` is now dead (see companion comment), the guard here reading `window.history.state?.settingsOpen` is always false and `window.history.back()` is never called. This dead branch can be removed, leaving just the `router.navigate({ to: "/code" })` path to handle the URL change on close.

```suggestion
    close: () => {
      const wasOpen = get().isOpen;
      set({
```

### Issue 3 of 3
apps/code/src/renderer/routes/__root.tsx:155-157
**DevTools bundle not tree-shaken in prod** — The top-level `import { TanStackRouterDevtools } from "@tanstack/react-router-devtools"` is unconditional, so the entire devtools chunk is included in the production bundle regardless of the `import.meta.env.DEV` guard on the JSX. The fix is to use `React.lazy` with a conditional dynamic import so the module is only ever loaded in development builds.

Reviews (1): Last reviewed commit: "feat(code): introduce TanStack Router fi..." | Re-trigger Greptile

Comment on lines 73 to 78
close: () => {
if (get().isOpen && window.history.state?.settingsOpen) {
const wasOpen = get().isOpen;
if (wasOpen && window.history.state?.settingsOpen) {
window.history.back();
}
set({
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 Since the window.history.pushState({ settingsOpen: true }) call in open() is now dead (see companion comment), the guard here reading window.history.state?.settingsOpen is always false and window.history.back() is never called. This dead branch can be removed, leaving just the router.navigate({ to: "/code" }) path to handle the URL change on close.

Suggested change
close: () => {
if (get().isOpen && window.history.state?.settingsOpen) {
const wasOpen = get().isOpen;
if (wasOpen && window.history.state?.settingsOpen) {
window.history.back();
}
set({
close: () => {
const wasOpen = get().isOpen;
set({
Prompt To Fix With AI
This is a comment left during a code review.
Path: apps/code/src/renderer/features/settings/stores/settingsDialogStore.ts
Line: 73-78

Comment:
Since the `window.history.pushState({ settingsOpen: true })` call in `open()` is now dead (see companion comment), the guard here reading `window.history.state?.settingsOpen` is always false and `window.history.back()` is never called. This dead branch can be removed, leaving just the `router.navigate({ to: "/code" })` path to handle the URL change on close.

```suggestion
    close: () => {
      const wasOpen = get().isOpen;
      set({
```

How can I resolve this? If you propose a fix, please make it concise.

Comment on lines 155 to +157
<HedgehogMode />
{import.meta.env.DEV && (
<TanStackRouterDevtools position="bottom-right" />
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 DevTools bundle not tree-shaken in prod — The top-level import { TanStackRouterDevtools } from "@tanstack/react-router-devtools" is unconditional, so the entire devtools chunk is included in the production bundle regardless of the import.meta.env.DEV guard on the JSX. The fix is to use React.lazy with a conditional dynamic import so the module is only ever loaded in development builds.

Prompt To Fix With AI
This is a comment left during a code review.
Path: apps/code/src/renderer/routes/__root.tsx
Line: 155-157

Comment:
**DevTools bundle not tree-shaken in prod** — The top-level `import { TanStackRouterDevtools } from "@tanstack/react-router-devtools"` is unconditional, so the entire devtools chunk is included in the production bundle regardless of the `import.meta.env.DEV` guard on the JSX. The fix is to use `React.lazy` with a conditional dynamic import so the module is only ever loaded in development builds.

How can I resolve this? If you propose a fix, please make it concise.

Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!

@adamleithp
Copy link
Copy Markdown
Contributor Author

Superseded by #2469 — the three-PR stack is collapsed into a single PR (rebased on main, conflicts resolved, CI green: typecheck + biome + 1594 unit tests). No intermediate layer was independently green, so the stack was collapsed rather than fixed three times. Closing in favor of #2469.

@adamleithp adamleithp closed this Jun 2, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant