fix(desktop): optimize terminal rendering to prevent re-render loops#670
fix(desktop): optimize terminal rendering to prevent re-render loops#670
Conversation
- Wrap Terminal and TabPane components in React.memo to prevent cascading re-renders - Use granular Zustand selectors in Terminal to avoid subscribing to the full pane object - Break circular dependency where Terminal updates cwd → store notifies → Terminal re-renders - Use stable callback pattern for tRPC subscription to prevent re-initialization This fixes dropped frames and slowness when typing in the terminal UI. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
📝 WalkthroughWalkthroughReact.memo memoization is applied to TabPane and Terminal components for render optimization. The TabView pane derivation logic is refactored to filter panes directly from a global store instead of deriving them from layout structure, decoupling pane visibility from layout-based extraction. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 1 | ❌ 2❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (1 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
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 |
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
apps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/TabView/index.tsx (1)
47-55: Subscribing to all panes may still cause unnecessary re-renders.The selector
useTabsStore((s) => s.panes)subscribes to the entire panes object. Any pane update (e.g., cwd changes from any terminal) will produce a newallPanesreference and trigger a re-render ofTabView, even if the update is for a pane in a different tab.Consider using a more granular selector that only returns panes for this specific tab, or using Zustand's
shallowequality check:Option 1: Use shallow comparison
+import { shallow } from "zustand/shallow"; + // Get all panes belonging to this tab -const allPanes = useTabsStore((s) => s.panes); -const tabPanes = useMemo( - () => - Object.fromEntries( - Object.entries(allPanes).filter(([_, pane]) => pane.tabId === tab.id), - ), - [allPanes, tab.id], -); +const tabPanes = useTabsStore( + (s) => + Object.fromEntries( + Object.entries(s.panes).filter(([_, pane]) => pane.tabId === tab.id), + ), + shallow, +);This ensures
TabViewonly re-renders when its own tab's panes actually change (by shallow-comparing the derived object's entries).
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
apps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/TabView/TabPane.tsxapps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/TabView/index.tsxapps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/Terminal/Terminal.tsx
🧰 Additional context used
📓 Path-based instructions (6)
apps/desktop/**/*.{ts,tsx}
📄 CodeRabbit inference engine (apps/desktop/AGENTS.md)
apps/desktop/**/*.{ts,tsx}: For Electron interprocess communication, ALWAYS use tRPC as defined insrc/lib/trpc
Use alias as defined intsconfig.jsonwhen possible
Prefer zustand for state management if it makes sense. Do not use effect unless absolutely necessary.
For tRPC subscriptions with trpc-electron, ALWAYS use the observable pattern from@trpc/server/observableinstead of async generators, as the library explicitly checksisObservable(result)and throws an error otherwise
Files:
apps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/TabView/TabPane.tsxapps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/TabView/index.tsxapps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/Terminal/Terminal.tsx
**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
**/*.{ts,tsx}: Use object parameters for functions with 2+ parameters instead of positional arguments
Functions with 2+ parameters should accept a single params object with named properties for self-documentation and extensibility
Use prefixed console logging with context pattern: [domain/operation] message
Extract magic numbers and hardcoded values to named constants at module top
Use lookup objects/maps instead of repeated if (type === ...) conditionals
Avoid usinganytype - maintain type safety in TypeScript code
Never swallow errors silently - at minimum log them with context
Import from concrete files directly when possible - avoid barrel file abuse that creates circular dependencies
Avoid deep nesting (4+ levels) - use early returns, extract functions, and invert conditions
Use named properties in options objects instead of boolean parameters to avoid boolean blindness
Files:
apps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/TabView/TabPane.tsxapps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/TabView/index.tsxapps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/Terminal/Terminal.tsx
apps/desktop/src/renderer/**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
Never import Node.js modules (fs, path, os, net) in renderer process or shared code - they are externalized for browser compatibility
Files:
apps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/TabView/TabPane.tsxapps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/TabView/index.tsxapps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/Terminal/Terminal.tsx
**/*.tsx
📄 CodeRabbit inference engine (AGENTS.md)
One component per file - do not create multi-component files
Files:
apps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/TabView/TabPane.tsxapps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/TabView/index.tsxapps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/Terminal/Terminal.tsx
apps/**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
Use Drizzle ORM for all database operations - never use raw SQL
Files:
apps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/TabView/TabPane.tsxapps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/TabView/index.tsxapps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/Terminal/Terminal.tsx
**/*.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (AGENTS.md)
Use Biome for formatting and linting - run at root level with
bun run lint:fixorbiome check --write
Files:
apps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/TabView/TabPane.tsxapps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/TabView/index.tsxapps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/Terminal/Terminal.tsx
🧠 Learnings (1)
📚 Learning: 2026-01-02T06:50:28.671Z
Learnt from: CR
Repo: superset-sh/superset PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-01-02T06:50:28.671Z
Learning: Applies to apps/*/src/components/{ui,ai-elements,react-flow}/*.tsx : Use kebab-case single files for shadcn/ui components (e.g., button.tsx, base-node.tsx) in src/components/ui/, src/components/ai-elements, and src/components/react-flow/
Applied to files:
apps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/TabView/TabPane.tsx
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: Build
🔇 Additional comments (4)
apps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/TabView/TabPane.tsx (1)
44-58: Good use ofmemo()for render optimization.The memoization combined with granular Zustand selectors (lines 60-61) should help reduce unnecessary re-renders when unrelated store state changes.
However, note that callback props like
onMoveToTabandonMoveToNewTabare created inline in the parent'srenderPanecallback, which creates new references on each render and may partially defeat memo. Consider wrapping those callbacks withuseCallbackin the parent if you observe re-renders persisting.apps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/Terminal/Terminal.tsx (3)
30-41: Excellent use of granular selectors to break the re-render loop.The component now subscribes only to specific pane properties (
initialCommands,initialCwd,tabId) that don't change after creation, rather than the entire pane object. This prevents the circular dependency wherecwdupdates would trigger re-renders.
57-59: Good defensive handling for potentially undefinedparentTabId.The fallback to empty string ensures no runtime error, and the conditional check on line 291 prevents calling
setFocusedPanewith undefined. This handles edge cases where the pane might not exist in the store during mount/unmount transitions.
246-286: Core fix: Stable callback pattern prevents subscription re-initialization.This pattern correctly solves the re-render loop:
stableOnDatahas a stable identity (empty depsuseCallback)handleStreamDataRef.currentis updated each render to capture fresh closures- The subscription sees the same callback reference, preventing re-initialization
This breaks the cycle: Terminal updates cwd → store notifies → Terminal re-renders → but subscription callback identity unchanged → no subscription re-init.
Summary
Problem
When typing in the terminal, there were dropped frames and noticeable slowness. Investigation revealed:
paneobject from Zustand storepane.cwdviaupdatePaneCwdwhen parsing OSC-7 sequencesTest plan
🤖 Generated with Claude Code
Summary by CodeRabbit
Release Notes
✏️ Tip: You can customize this high-level summary in your review settings.