fix(desktop): stop silently routing electronTrpc font-settings query through host-service#3394
Conversation
…client @trpc/react-query falls back to a module-level singleton React context when createTRPCReact is called without an explicit context. In the v2 workspace, nesting <workspaceTrpc.Provider> overrides that singleton and silently routes electronTrpc.*.useQuery calls through the workspace HTTP link, producing "No procedure found on path settings.getFontSettings" against host-service. Switch the three v2 render paths (useTerminalAppearance, WorkspaceDiff, and v1 CodeEditor which is reused in v2's FilePane renderers) to use electronTrpcClient.settings.getFontSettings.query() wrapped in a plain @tanstack/react-query useQuery, matching the rest of v2.
|
Caution Review failedThe pull request is closed. ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (3)
📝 WalkthroughWalkthroughReplaced three usages of Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Greptile SummaryThis PR fixes a silent routing bug in v2 workspace panes: because Key points:
Confidence Score: 4/5Safe to merge — the core bug fix is correct — but the cache-key mismatch means font-setting changes won't propagate immediately to the patched components; this should be addressed soon. The PR correctly identifies and fixes the singleton React context collision that caused 404 errors in v2 workspace panes. The imperative-client approach is consistent with how the rest of v2 already calls All three changed files share the same cache-key mismatch; FontSettingSection.tsx (unchanged) will also need a follow-up to invalidate the new query key after mutations. Important Files Changed
Sequence DiagramsequenceDiagram
participant App as App (React tree)
participant WTRPC as workspaceTrpc.Provider (v2 layout)
participant Hook as useTerminalAppearance / WorkspaceDiff / CodeEditor
participant Client as electronTrpcClient (IPC proxy)
participant IPC as Electron IPC
participant Main as Main process (settings router)
Note over App,WTRPC: Before this PR
App->>WTRPC: renders workspaceTrpc.Provider
Note over WTRPC: Overwrites @trpc/react-query module-level singleton context
Hook->>WTRPC: electronTrpc.settings.getFontSettings.useQuery()
WTRPC-->>Hook: routes through host-service HTTP link
Hook--xMain: TRPCError: No procedure found on path settings.getFontSettings
Note over App,Main: After this PR
App->>WTRPC: renders workspaceTrpc.Provider
Hook->>Client: useQuery -> electronTrpcClient.settings.getFontSettings.query()
Client->>IPC: IPC link (bypasses React context entirely)
IPC->>Main: settings.getFontSettings
Main-->>IPC: FontSettings data
IPC-->>Client: resolved
Client-->>Hook: fontSettings
Reviews (1): Last reviewed commit: "fix(desktop): route font-settings query ..." | Re-trigger Greptile |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| const { data: fontSettings } = useQuery({ | ||
| queryKey: ["electron", "settings", "getFontSettings"], | ||
| queryFn: () => electronTrpcClient.settings.getFontSettings.query(), | ||
| staleTime: 30_000, | ||
| }); |
There was a problem hiding this comment.
The new queryKey: ["electron", "settings", "getFontSettings"] is a plain React Query key that does not match the internal key that @trpc/react-query generates for electronTrpc.settings.getFontSettings.useQuery(). tRPC generates a nested key structure like [["settings", "getFontSettings"], { input: undefined, type: "query" }].
FontSettingSection.tsx calls utils.settings.getFontSettings.invalidate() in its mutation's onSettled callback. That call targets only the tRPC-managed cache entry — it will not reach the vanilla useQuery entries in this hook, WorkspaceDiff, or CodeEditor. After a user saves new font settings, these three components will continue displaying the old values until the 30 s staleTime expires and a natural refetch fires (e.g. window focus or remount).
The same issue exists in WorkspaceDiff.tsx and CodeEditor.tsx.
One way to close the gap without restructuring provider wiring: add a matching queryClient.invalidateQueries call in FontSettingSection's onSettled alongside the existing utils call:
queryClient.invalidateQueries({ queryKey: ["electron", "settings", "getFontSettings"] });Alternatively, use getQueryKey (exported from @trpc/react-query) to derive the exact tRPC-generated key and supply it to these vanilla useQuery calls, so that utils.settings.getFontSettings.invalidate() covers them automatically.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
🧹 Nitpick comments (1)
apps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspace/$workspaceId/hooks/usePaneRegistry/components/DiffPane/components/WorkspaceDiff/WorkspaceDiff.tsx (1)
54-58: The hardcodedqueryKeyhere is intentional—see the preceding comment explaining that imperativeelectronTrpcClientis required to avoid silent routing through the host-service HTTP link whenworkspaceTrpc.Providernests the default@trpc/react-querycontext (a module-level singleton). This pattern is used consistently across three files (WorkspaceDiff.tsx,useTerminalAppearance.ts,CodeEditor.tsx).The trade-off is that cache invalidations from
utils.settings.getFontSettings.invalidate()inFontSettingSection.tsxwon't reach these hardcoded-key queries. The 30-secondstaleTimemitigates staleness risk, but if faster consistency is needed, consider centralizing font-settings queries to a single cache-key strategy (though this would require first resolving the Provider nesting constraint).🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspace/`$workspaceId/hooks/usePaneRegistry/components/DiffPane/components/WorkspaceDiff/WorkspaceDiff.tsx around lines 54 - 58, The query in WorkspaceDiff.tsx (and the matching queries in useTerminalAppearance.ts and CodeEditor.tsx) uses a hardcoded queryKey which prevents cache invalidation calls from utils.settings.getFontSettings.invalidate() from reaching them; fix by introducing and importing a single shared constant (e.g., FONT_SETTINGS_QUERY_KEY) used by all three query usages and by the invalidate call in FontSettingSection.tsx so invalidations target the same cache key (alternatively, if Provider nesting cannot be changed yet, reduce staleTime or call invalidate via the same imperative electronTrpcClient path but prefer the shared query-key constant for consistency).
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In
`@apps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspace/`$workspaceId/hooks/usePaneRegistry/components/DiffPane/components/WorkspaceDiff/WorkspaceDiff.tsx:
- Around line 54-58: The query in WorkspaceDiff.tsx (and the matching queries in
useTerminalAppearance.ts and CodeEditor.tsx) uses a hardcoded queryKey which
prevents cache invalidation calls from
utils.settings.getFontSettings.invalidate() from reaching them; fix by
introducing and importing a single shared constant (e.g.,
FONT_SETTINGS_QUERY_KEY) used by all three query usages and by the invalidate
call in FontSettingSection.tsx so invalidations target the same cache key
(alternatively, if Provider nesting cannot be changed yet, reduce staleTime or
call invalidate via the same imperative electronTrpcClient path but prefer the
shared query-key constant for consistency).
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: ce32f4ea-d74c-49e3-8c19-3c354b694e07
📒 Files selected for processing (3)
apps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspace/$workspaceId/hooks/usePaneRegistry/components/DiffPane/components/WorkspaceDiff/WorkspaceDiff.tsxapps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspace/$workspaceId/hooks/usePaneRegistry/components/TerminalPane/hooks/useTerminalAppearance/useTerminalAppearance.tsapps/desktop/src/renderer/screens/main/components/WorkspaceView/components/CodeEditor/CodeEditor.tsx
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
There was a problem hiding this comment.
1 issue found across 3 files
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="apps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspace/$workspaceId/hooks/usePaneRegistry/components/DiffPane/components/WorkspaceDiff/WorkspaceDiff.tsx">
<violation number="1" location="apps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspace/$workspaceId/hooks/usePaneRegistry/components/DiffPane/components/WorkspaceDiff/WorkspaceDiff.tsx:55">
P2: Cache invalidation is broken for these vanilla `useQuery` calls. The `queryKey` here (`["electron", "settings", "getFontSettings"]`) doesn't match the tRPC-generated key structure (`[["settings", "getFontSettings"], { input: undefined, type: "query" }]`), so when `FontSettingSection` calls `utils.settings.getFontSettings.invalidate()` after saving, it won't invalidate these entries. Users will see stale font settings for up to 30 s (the `staleTime`) after changing them.
Two options:
1. Use `getQueryKey` from `@trpc/react-query` to derive the exact tRPC key and pass it here, so the existing `invalidate()` covers these entries automatically.
2. Add a matching `queryClient.invalidateQueries({ queryKey: ["electron", "settings", "getFontSettings"] })` in `FontSettingSection`'s `onSettled` callback alongside the existing `utils` call.
The same issue applies to `WorkspaceDiff.tsx` and `CodeEditor.tsx`.</violation>
</file>
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
| // singleton — nesting workspaceTrpc.Provider overrides it and silently | ||
| // routes electronTrpc hooks through the host-service HTTP link. | ||
| const { data: fontSettings } = useQuery({ | ||
| queryKey: ["electron", "settings", "getFontSettings"], |
There was a problem hiding this comment.
P2: Cache invalidation is broken for these vanilla useQuery calls. The queryKey here (["electron", "settings", "getFontSettings"]) doesn't match the tRPC-generated key structure ([["settings", "getFontSettings"], { input: undefined, type: "query" }]), so when FontSettingSection calls utils.settings.getFontSettings.invalidate() after saving, it won't invalidate these entries. Users will see stale font settings for up to 30 s (the staleTime) after changing them.
Two options:
- Use
getQueryKeyfrom@trpc/react-queryto derive the exact tRPC key and pass it here, so the existinginvalidate()covers these entries automatically. - Add a matching
queryClient.invalidateQueries({ queryKey: ["electron", "settings", "getFontSettings"] })inFontSettingSection'sonSettledcallback alongside the existingutilscall.
The same issue applies to WorkspaceDiff.tsx and CodeEditor.tsx.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At apps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspace/$workspaceId/hooks/usePaneRegistry/components/DiffPane/components/WorkspaceDiff/WorkspaceDiff.tsx, line 55:
<comment>Cache invalidation is broken for these vanilla `useQuery` calls. The `queryKey` here (`["electron", "settings", "getFontSettings"]`) doesn't match the tRPC-generated key structure (`[["settings", "getFontSettings"], { input: undefined, type: "query" }]`), so when `FontSettingSection` calls `utils.settings.getFontSettings.invalidate()` after saving, it won't invalidate these entries. Users will see stale font settings for up to 30 s (the `staleTime`) after changing them.
Two options:
1. Use `getQueryKey` from `@trpc/react-query` to derive the exact tRPC key and pass it here, so the existing `invalidate()` covers these entries automatically.
2. Add a matching `queryClient.invalidateQueries({ queryKey: ["electron", "settings", "getFontSettings"] })` in `FontSettingSection`'s `onSettled` callback alongside the existing `utils` call.
The same issue applies to `WorkspaceDiff.tsx` and `CodeEditor.tsx`.</comment>
<file context>
@@ -47,10 +47,15 @@ export const WorkspaceDiff = memo(function WorkspaceDiff({
+ // singleton — nesting workspaceTrpc.Provider overrides it and silently
+ // routes electronTrpc hooks through the host-service HTTP link.
+ const { data: fontSettings } = useQuery({
+ queryKey: ["electron", "settings", "getFontSettings"],
+ queryFn: () => electronTrpcClient.settings.getFontSettings.query(),
+ staleTime: 30_000,
</file context>
🧹 Preview Cleanup CompleteThe following preview resources have been cleaned up:
Thank you for your contribution! 🎉 |
…through host-service (superset-sh#3394) * fix(desktop): route font-settings query via imperative electron tRPC client @trpc/react-query falls back to a module-level singleton React context when createTRPCReact is called without an explicit context. In the v2 workspace, nesting <workspaceTrpc.Provider> overrides that singleton and silently routes electronTrpc.*.useQuery calls through the workspace HTTP link, producing "No procedure found on path settings.getFontSettings" against host-service. Switch the three v2 render paths (useTerminalAppearance, WorkspaceDiff, and v1 CodeEditor which is reused in v2's FilePane renderers) to use electronTrpcClient.settings.getFontSettings.query() wrapped in a plain @tanstack/react-query useQuery, matching the rest of v2. * chore(desktop): drop explanatory comments from font-settings workaround
1 participant
Summary
@trpc/react-queryuses a module-level singleton React context whencreateTRPCReact()is called without an explicitcontext. BothelectronTrpc(apps/desktop/src/renderer/lib/electron-trpc.ts) andworkspaceTrpc(packages/workspace-client/src/workspace-trpc.ts) do this, so nesting<workspaceTrpc.Provider>inside the v2 workspace layout overrides that singleton — and everyelectronTrpc.*.useQuery(...)call rendered under it silently routes through the workspace HTTP link instead of the Electron IPC link.The visible symptom was a recurring
TRPCError: No procedure found on path "settings.getFontSettings"coming fromhost-service.jswhenever v2 workspace panes mounted (Terminal, WorkspaceDiff, FilePane code/markdown renderers).Rather than introduce a new React context and re-test v2's provider wiring, this PR matches the pattern v2 already uses everywhere else — the imperative
electronTrpcClientproxy — and wraps the query in a plain@tanstack/react-queryuseQuery. That bypasses trpc-react-query's shared context entirely.Files touched:
useTerminalAppearance— v2 terminal font/theme hookWorkspaceDiff— v2 diff paneCodeEditor(v1, reused in v2'sFilePanecode/markdown renderers)Terminal.tsxandLightDiffViewer.tsxalso use the React hook form, but neither is rendered inside v2'sWorkspaceTrpcProvider, so they're left alone.Test plan
bun run typecheck(desktop) passesbun run lint:fixcleanbun test apps/desktop— same baseline asmain(1 pre-existing fail intrpc-storage/useOrderedSections, unrelated)settings.getFontSettings404s in logs and that editor/terminal font settings still applySummary by cubic
Fix v2 panes routing font settings queries through the workspace HTTP link. Switch to
electronTrpcClientwrapped in@tanstack/react-queryuseQueryso font settings go through Electron IPC and apply correctly in Terminal, Diff, and CodeEditor.electronTrpc.settings.getFontSettings.useQuerywithuseQuery({ queryKey: ["electron","settings","getFontSettings"], queryFn: () => electronTrpcClient.settings.getFontSettings.query(), staleTime: 30_000 }).useTerminalAppearance,WorkspaceDiff, andCodeEditor; removed inline comments from this workaround.@trpc/react-queryshared context bleed from nestedworkspaceTrpc.Provider, preventing host-service 404s/TRPCErroronsettings.getFontSettings.Written for commit 03f3c3d. Summary will update on new commits.
Summary by CodeRabbit