Skip to content

refactor(web): the command palette caches its derived lookups without "use memo" - #21116

Merged
juliusmarminge merged 1 commit into
mainfrom
refactor/command-palette-items
Oct 18, 2026
Merged

juliusmarminge merged 1 commit into
mainfrom
refactor/command-palette-items

Conversation

@juliusmarminge

Copy link
Copy Markdown
Member

The command palette's five derived-value helpers (theme cards, the thread-search content-match map, environment lookups, project lookups, sorted project groups) kept "use memo" behind oxlint-disable for t3code/no-manual-memo. They were the last such suppressions in apps/web.

Why they needed it

Each helper took its source value as an argument and called no hook. In infer mode the React Compiler only compiles a use* function that actually calls a hook, so without the directive these were treated as plain functions and cached nothing, with no diagnostic. Their call sites in the 2,500-line dialog weren't cached either, because the results flow into item arrays that are built across many interleaved hooks.

The change

Each helper now reads its own source hook and derives from it:

  • useThemeCards() reads useCustomThemes and useEnvironmentThemeDefinitions.
  • useThreadSearchWithContentMatches(ids, query) wraps useThreadSearch and returns the search together with its content-match map.
  • useEnvironmentLookups() reads useEnvironments.
  • useProjectLookups() reads useProjects.
  • useSortedProjectGroups(environmentLabelById) reads projects, threads, project order, client settings and the primary environment id itself.

The compiler then gives each derived value a cache scope keyed on its store snapshot. The directives, the suppressions and the comment block are gone. docs/internals/web-ui.md gains one sentence about this trap: a hook-free use* helper silently gets no cache.

I didn't move all item building into a pure module. That would have relocated about 2,000 lines of render code to buy back caching for five values, and this change gets the same caching without it.

Compile proof

Compiled with oxc-transform-react using the options from reactCompilerCoverage.test.ts (0 diagnostics):

function useThemeCards() {                    // $ = _c(3)
	if ($[0] !== customThemes || $[1] !== environmentThemes) {
function useThreadSearchWithContentMatches(environmentIds, query) {   // _c(5)
	if ($[0] !== threadSearch.matches) {
function useEnvironmentLookups() {            // _c(7)
	if ($[0] !== environments) {              // environmentLabelById
	if ($[2] !== environments) {              // projectEnvironmentLocationById
function useProjectLookups() {                // _c(10)
	if ($[0] !== projects) {  /* projectCwdById */
	if ($[2] !== projects) {  /* projectByKey */
	if ($[4] !== projects) {  /* projectTitleById */
function useSortedProjectGroups(environmentLabelById) {   // _c(14)
	if ($[0] !== clientSettings.sidebarProjectGroupingMode || $[1] !== clientSettings.sidebarProjectGroupingOverrides || $[2] !== clientSettings.sidebarProjectSortOrder || $[3] !== environmentLabelById || $[4] !== primaryEnvironmentId || $[5] !== projectOrder || $[6] !== projects || $[7] !== threads) {

Renders

Flow: open the palette (Ctrl+K), type 10 characters, clear them, open Change theme, close. The counter is rc-init.js (counts only fibers rendered in each commit) on a dev server without --watch, with the same migrate-dev-db snapshot for both. Before is 8f026485df (origin/main), after is this branch.

commits total renders OpenCommandPaletteDialog
before, runs 166 / 167 / 165 4850 / 5020 / 4850 61 / 61 / 61
before, median 166 4850 61
after, runs 172 / 165 / 158 / 158 / 165 5275 / 4850 / 4898 / 4171 / 4850 63 / 61 / 59 / 60 / 61
after, median 165 4850 61

The modal run is identical on both sides (165 commits, 4850 renders, 61 dialog renders, with the same per-component breakdown). The spread comes from keystroke timing in the driver: useDeferredValue batches differently depending on how keystrokes land. I ran two extra "after" runs to rule out a regression. Render counts don't change, because render counts are not where this pays off: the change keeps the lookup Maps, theme cards and sorted groups from being rebuilt on each of the dialog's ~61 renders.

Checks

  • grep '"use memo"\|no-manual-memo' finds nothing in CommandPalette.tsx, and there's no oxlint-disable … no-manual-memo anywhere in apps/web/src.
  • vp lint src/components/CommandPalette.tsx is clean.
  • vp test run src/reactCompilerCoverage.test.ts -t CommandPalette passes.
  • CommandPalette.logic.test.ts passes (33 tests).
  • tsc-rs --noEmit passes for apps/web.

Claude Opus 5.5 via Claude Code.

🤖 Generated with Claude Code

… "use memo"

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@github-actions github-actions Bot added vouch:trusted PR author is trusted by repo permissions or the VOUCHED list. size:M 30-99 changed lines (additions + deletions). labels Oct 18, 2026
@juliusmarminge juliusmarminge added the macroscope-review Opt PRs made by unvouched contributors in for Macroscope review. Vouched contributors auto-reviews label Oct 18, 2026
@macroscopeapp

macroscopeapp Bot commented Oct 18, 2026

Copy link
Copy Markdown
Contributor

Approvability

Verdict: Approved at dac256b

Macroscope's review found this PR approvable — This is a focused compiler-aware refactor that preserves command-palette behavior while retaining caching for existing derived lookups without lint suppressions. It changes no product defaults, schemas, workflows, or sensitive code, and the documentation update is non-runtime.

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

@github-actions

Copy link
Copy Markdown
Contributor

Thread transfer impact

⚠️ The latest CI run did not produce a thread transfer result for dac256b.

This comment will update automatically after the next completed run.

@coderabbitai

coderabbitai Bot commented Oct 18, 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: Team
  • Run ID: bffb9729-e1a8-4c6c-8fdc-a4731ae91f9f
📥 Commits

Reviewing files that changed from the base of the PR and between 8f02648 and dac256b.

📒 Files selected for processing: 2
  • apps/web/src/components/CommandPalette.tsx
  • docs/internals/web-ui.md

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

Command Palette helpers now read their source data through hooks. The dialog calls these helpers without passing hook results. The web UI guidance describes the React Compiler behavior for helpers that call hooks.

Changes: Command Palette Hook Refactor

Layer / File(s) Summary
Hook-owned data and derived values
apps/web/src/components/CommandPalette.tsx, docs/internals/web-ui.md
Theme, thread-search, project-lookup, and sorted-project-group helpers read source data through hooks. The thread-search helper returns search results and user and assistant content matches. The guidance describes how the React Compiler handles helpers that call hooks.
Dialog calls to hook helpers
apps/web/src/components/CommandPalette.tsx
The dialog removes separate data reads and calls the helpers without passing their source values.

Priority: ⬇️ Low

Estimated code review effort: 2 (Simple) | ~10 minutes

Change: Refactor

Suggested reviewers: t3dotgg

Merge Risk: ⚪ Minimal · up to dac25

No actionable merge-blocking risk is identified; the change is mergeable after normal checks.

Pre-merge checks: Passed 3 · Failed 1

Failed Failed checks: 1 warning

Check name Status Explanation Resolution
Description check Warning The description thoroughly explains the problem, implementation, compiler evidence, render comparison, and verification. It does not provide the required Scope and approval information or explain why … Add a Scope and approval section with the related issue or maintainer approval. If this focused refactor needs no prior issue, explain why it qualifies for that exemption. State whether the UI has visual changes; if it does, add the require…
Passed Passed checks: 3 passed
Check name Status Explanation
Title check Passed The title clearly identifies the command palette refactor and the removal of the "use memo" requirement.
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.
Full details: Description check

Explanation

The description thoroughly explains the problem, implementation, compiler evidence, render comparison, and verification. It does not provide the required Scope and approval information or explain why prior approval was unnecessary.

Resolution

Add a Scope and approval section with the related issue or maintainer approval. If this focused refactor needs no prior issue, explain why it qualifies for that exemption. State whether the UI has visual changes; if it does, add the required before/after screenshots and any interaction recording.

  • Fix all pre-merge checks with AI
✨ Finishing Touches: 💡 1
🛠️ Fix failing CI checks: 💡
  • Commit to this branch
  • Create a new PR
📝 Generate docstrings
  • Commit to this branch
  • Create a new PR
🧪 Generate unit tests (beta)
  • Commit to this branch
  • 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.

@juliusmarminge
juliusmarminge merged commit 2e16cf2 into main Oct 18, 2026
47 of 49 checks passed
@juliusmarminge
juliusmarminge deleted the refactor/command-palette-items branch October 18, 2026 07:06
github-actions Bot added a commit to omarcresp/t3code-flake that referenced this pull request Oct 18, 2026
## What's Changed
* perf(mobile): Metro runs the React Compiler, with a coverage ratchet by @juliusmarminge in pingdotgg/t3code#20967
* perf(mobile): usage screens compile with the React Compiler by @juliusmarminge in pingdotgg/t3code#20969
* perf(mobile): Connect onboarding compiles with the React Compiler by @juliusmarminge in pingdotgg/t3code#20970
* perf(mobile): device preview compiles with the React Compiler by @juliusmarminge in pingdotgg/t3code#20971
* fix(android): foldable sidebar and layout fixes by @t3dotgg in pingdotgg/t3code#21071
* test(mobile): measure renders on a device by @juliusmarminge in pingdotgg/t3code#21001
* fix(web): Working threads can be dragged into chat as references again by @t3dotgg in pingdotgg/t3code#19697
* refactor(web): the command palette caches its derived lookups without "use memo" by @juliusmarminge in pingdotgg/t3code#21116
* feat(mobile): link, unlink and watch a thread's pull requests from its sheet by @flamboh in pingdotgg/t3code#20394
* test(web): the right panel Escape test advances fake frames until focus returns by @juliusmarminge in pingdotgg/t3code#21118
* test(web): ChatMarkdown thread find waits for highlights to settle by @juliusmarminge in pingdotgg/t3code#21121


**Full Changelog**: pingdotgg/t3code@v0.1.1-nightly.20261018.3285...v0.1.1-nightly.20261018.3302

Upstream release: https://github.com/pingdotgg/t3code/releases/tag/v0.1.1-nightly.20261018.3302
github-actions Bot added a commit to davidvanderklay/t3code-flake that referenced this pull request Oct 18, 2026
## What's Changed
* perf(mobile): Metro runs the React Compiler, with a coverage ratchet by @juliusmarminge in pingdotgg/t3code#20967
* perf(mobile): usage screens compile with the React Compiler by @juliusmarminge in pingdotgg/t3code#20969
* perf(mobile): Connect onboarding compiles with the React Compiler by @juliusmarminge in pingdotgg/t3code#20970
* perf(mobile): device preview compiles with the React Compiler by @juliusmarminge in pingdotgg/t3code#20971
* fix(android): foldable sidebar and layout fixes by @t3dotgg in pingdotgg/t3code#21071
* test(mobile): measure renders on a device by @juliusmarminge in pingdotgg/t3code#21001
* fix(web): Working threads can be dragged into chat as references again by @t3dotgg in pingdotgg/t3code#19697
* refactor(web): the command palette caches its derived lookups without "use memo" by @juliusmarminge in pingdotgg/t3code#21116
* feat(mobile): link, unlink and watch a thread's pull requests from its sheet by @flamboh in pingdotgg/t3code#20394
* test(web): the right panel Escape test advances fake frames until focus returns by @juliusmarminge in pingdotgg/t3code#21118
* test(web): ChatMarkdown thread find waits for highlights to settle by @juliusmarminge in pingdotgg/t3code#21121


**Full Changelog**: pingdotgg/t3code@v0.1.1-nightly.20261018.3285...v0.1.1-nightly.20261018.3302

Upstream release: https://github.com/pingdotgg/t3code/releases/tag/v0.1.1-nightly.20261018.3302
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

macroscope-review Opt PRs made by unvouched contributors in for Macroscope review. Vouched contributors auto-reviews size:M 30-99 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.

1 participant