Skip to content

perf(web): Settings loads only the General page on open - #17121

Closed
flamboh wants to merge 6 commits into
pingdotgg:mainfrom
flamboh:t3/perf-settings-load-less
Closed

flamboh wants to merge 6 commits into
pingdotgg:mainfrom
flamboh:t3/perf-settings-load-less

Conversation

@flamboh

@flamboh flamboh commented Oct 8, 2026 •

Copy link
Copy Markdown
Contributor

Note

🤖 Claude Opus 5.5 on behalf of Oliver

Problem

Opening Settings loaded far more code than the General page needs. General imported the whole SettingsPanels module, which pulls in Appearance's font previews and theme import. The settings sidebar loaded T3 Connect account UI even when T3 Connect isn't configured, only to render nothing. Every entry point also navigated through the /settings redirect before General, and nothing started loading until the click. In dev, each extra module is a separate request, so a cold open was 36 script requests.

Fix

  • Split SettingsPanels.tsx into GeneralSettingsPanel, AppearanceSettingsPanel, ArchivedThreadsPanel and a shared useSettingsRestore hook. Function bodies moved unchanged (verified with an AST comparison).
  • Only render the T3 Connect sign-in and avatar when public cloud config is present, and lazy-load the two custom account pages. Each shows a short "Loading…" line until its code arrives.
  • The sidebar, command palette and desktop menu go straight to /settings/general. The sidebar entry preloads the route on hover or focus, and the palette preloads it when the Settings action is highlighted. The /settings redirect still works.
  • Background Activity's advanced dialog builds its fields only when opened.
Open Settings from a thread Before → after Runs (before / after)
Dev, first open 3866.2 → 2206.1 ms 5 / 5
Dev, repeat open 1646.9 → 1387.8 ms 5 / 5
Production, first open 1005.2 → 699.4 ms 5 / 5
Production, repeat open 484.5 → 362.2 ms 5 / 5
Script requests, first open (dev / production) 36 → 15 / 20 → 10 5 / 5, same in every run

Timings are medians from headless Chromium with CPU throttled 4× while opening Settings, in alternating before/after batches (3 then 2 runs per side) on an isolated dev server and on the production build. Before is main at 9f48092. After is this PR's four performance commits, plus one commit each from #17119 and #17120 that only change the Licenses and Keybindings pages. The later loading-line commit only affects the signed-in account pages and wasn't measured. In a separate run with frozen source, initial app load bytes were unchanged (within a few kB).

Before and after

Opening Settings from a thread right after page load, going Back, then opening it again. In this take, the first open took 720 → 552 ms and the second 423 → 382 ms.

https://gh-file-drop-api-prod-galwoqjslzlnws6s.oliver-boorstein.workers.dev/f/6f19154c3e252dbb/pr17121-before-after.mp4

Production build, CPU throttled 4× during the interaction, headless Chromium. Before is main, after is this branch. These are single takes, so the timings are illustrative; the tables above are medians.

Validation

  • Web typecheck, targeted lint (no errors), 96 tests across SettingsPanels restore and logic, settings search and scope navigation.
  • Headless dev and production smoke across all settings pages, settings-search hash targets, theme import, command-palette navigation, the /settings redirect, and hover preloading.
  • After the loading-line commit: web typecheck and lint on T3ConnectAccountPages.tsx pass.
  • Not exercised: native Electron (CmdOrCtrl+, reaches the same route), signed-in T3 Connect account pages, including their loading line (checked by source, types and lint only).

Scope and approval

There is no prior issue or discussion for this. It's a focused performance improvement with no intended behavior change beyond the account pages' brief loading line. CONTRIBUTING lists performance improvements among the changes most likely to be accepted. It's part of a Settings performance series: #17119, #17120, #17121, #17122, #17123.

Note

The file split will conflict with open PRs that edit SettingsPanels.tsx. The moved code is unchanged, so those resolve by applying the same edit to the new file.

Implemented by GPT-6.1 Sol (Codex harness, delegated through T3 Code); split, verified and opened by Claude Opus 5.5 (Claude Code). Review follow-up by Claude Opus 5.5 (Claude Code in T3 Code).

@github-actions github-actions Bot added vouch:trusted PR author is trusted by repo permissions or the VOUCHED list. size:XXL 1,000+ changed lines (additions + deletions). labels Oct 8, 2026
@flamboh
flamboh marked this pull request as ready for review October 8, 2026 08:43
@macroscopeapp

macroscopeapp Bot commented Oct 8, 2026 •

Copy link
Copy Markdown
Contributor

Approvability

Verdict: Not approved

Macroscope's review found this PR not approvable — This PR mainly splits existing settings code into route-specific chunks and defers optional UI loading, but it also changes a Clerk authentication component. Authentication-directory changes require human review under the approval criteria.

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

@coderabbitai

coderabbitai Bot commented Oct 8, 2026 •

Copy link
Copy Markdown

Review in Change Stack →

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: 8ad447fa-402d-4a6d-aebd-773b48dad9cd

📥 Commits

Reviewing files that changed from the base of the PR and between 849233a and af19e87.


📒 Files selected for processing (3)
  • apps/web/src/components/AppSidebarLayout.tsx
  • apps/web/src/components/CommandPalette.tsx
  • apps/web/src/components/settings/GeneralSettingsPanel.tsx

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



📝 Walkthrough

Walkthrough

Settings panels and restore logic move into dedicated modules. Settings entry points navigate to and preload the general settings route. Cloud account controls render conditionally, and two account pages load lazily.

Changes

Settings UI

Layer / File(s) Summary
Appearance settings controls
apps/web/src/components/settings/AppearanceSettingsPanel.tsx, apps/web/src/components/settings/GeneralSettingsPanel.tsx, apps/web/src/routes/settings.appearance.tsx
A dedicated panel adds theme, interface, and typography controls, including font discovery and validation. The appearance route imports the panel from its dedicated module.
Archived threads panel
apps/web/src/components/settings/ArchivedThreadsPanel.tsx, apps/web/src/components/settings/GeneralSettingsPanel.tsx, apps/web/src/routes/settings.archived.tsx
A dedicated panel groups archived threads and supports unarchive and delete actions. It displays loading, error, and empty states. The archived route imports the panel from its dedicated module.
Settings restore hook
apps/web/src/components/settings/useSettingsRestore.ts, apps/web/src/components/settings/SettingsPanels.restore.test.tsx, apps/web/src/routes/settings.tsx
The extracted hook compares theme and scoped settings with defaults and restores them after confirmation. If a theme reset fails, it attempts to restore the prior theme state and stops before resetting scoped settings. The test and route import the hook from its dedicated module.
Settings entry points and account controls
apps/web/src/components/AppSidebarLayout.tsx, apps/web/src/components/CommandPalette.tsx, apps/web/src/components/sidebar/SidebarChrome.tsx, apps/web/src/components/settings/SettingsSidebarNav.tsx, apps/web/src/components/settings/GeneralSettingsPanel.tsx
The sidebar menu and command palette navigate to and preload /settings/general. The settings sidebar renders sign-in and avatar components only when cloud public configuration is available. The general settings panel retains the hosted-channel selector and reorganizes the background-activity dialog.

Account page loading

Layer / File(s) Summary
Lazy-load account pages
apps/web/src/components/clerk/T3ConnectAccountPages.tsx
The Mobile clients and T3 Connect account pages use dynamic imports and labeled Suspense fallbacks.

Priority: ➖ Normal

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

Change: Refactor

Suggested reviewers: juliusmarminge


Merge Risk | ⚪ Minimal · up to af19e

Merge Risk: ⚪ Minimal · up to af19e

Settings now opens directly on the General page and loads less code up front. Panels were moved into separate files, and account pages load on demand. No concrete regression was identified, so the change appears ready to merge.

Security Architecture Review

Security architecture risk: 🔵 Low · up to af19e

The changes preserve the examined sign-in and settings-write controls. No introduced security concern was identified, but deferred-loading and dialog lifecycle behavior were not fully verified at runtime.

Retained concerns
No architecture-level concerns identified.

Security review details

Security Blast Radius

  • observed — The existing scoped settings path can write to multiple connected environments according to the selected scope. Target planning and per-request authorization rechecks are unchanged by the dialog extraction; the PR does not add a broader write target or additional authority.

Security Findings and Attack Paths

  • inferred — The examined navigation changes do not introduce an attacker-selected destination or bypass the Settings authentication boundary. They select a fixed internal child route whose parent guard remains in place. This conclusion is limited to the inspected entry flows, not a complete security assessment.

Trust Boundaries and Controls

  • observed — The new sidebar cloud-config gate repeats the condition already enforced inside the account controls. Existing signed-in and loaded-session checks remain, and the account-opening hook still returns no opener for a signed-out session. The gate is a loading condition, not a replacement for identity checks.

Resilience and Maintainability Implications

  • observed — Scoped persistence waits for target results and reports partial failures rather than providing an atomic cross-environment rollback. Explicit concurrent-write serialization is not added by this PR. These are pre-existing persistence characteristics, not introduced concerns from the dialog refactor.

Pre-merge checks | Passed 4
✅ Passed checks (4 passed)
Check name Status Explanation
Title check Passed The title is concise, specific, and accurately identifies the main performance change: opening Settings loads only the General page.
Description check Passed The description covers the problem, implementation, scope and approval rationale, verification results, performance measurements, evidence, limitations, and agent attribution. It is detailed and align…
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.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create a new PR

  • Autopilot · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

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

@t3dotgg

t3dotgg commented Oct 11, 2026

Copy link
Copy Markdown
Member

Thanks for digging into this, Oliver. We're going to pass on this one: Settings already opens fast enough, and splitting the panels across new files and preload paths adds more to maintain than the gain is worth right now. #17120 is still on our list.

@t3dotgg t3dotgg closed this Oct 11, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

size:XXL 1,000+ changed lines (additions + deletions). vouch:trusted PR author is trusted by repo permissions or the VOUCHED list.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants