Uh oh!
There was an error while loading. Please reload this page.
improvement(sidebar): memoize workflow/folder rows for faster tab navigation - #5428
Conversation
…igation Switching between workspace tabs re-rendered every workflow and folder row in the sidebar because the rows (and the shared export hooks they call) subscribed to useParams, which re-renders on every navigation. - Wrap WorkflowItem and FolderItem in React.memo (the only un-memoized leaf rows; every sibling row was already memoized). - Decouple the rows from useParams: thread workspaceId as a stable prop from WorkflowList (matching the FileList convention), and expose the live active workflowId through a stable activeWorkflowIdRef on SidebarListContext, read only in delete callbacks — never during render. - Refactor the three shared export hooks (used only by these two rows) to take workspaceId as a param instead of calling useParams internally. - Stabilize handleWorkflowClick's identity via refs so the shared list context no longer changes identity on navigation. - Lazy-init the drag-drop siblings Map ref. On a tab switch only the two rows whose active state flips now re-render.
The latest updates on your projects. Learn more about Vercel for GitHub. |
PR SummaryLow Risk Overview
On tab navigation, only rows whose active highlight changes should re-render, not the full workflow/folder list. Reviewed by Cursor Bugbot for commit ca6f169. Configure here. |
Uh oh!
There was an error while loading. Please reload this page.
Greptile SummaryThis PR eliminates full-list re-renders when switching workspace tabs by wrapping
Confidence Score: 5/5Safe to merge — the optimization is semantically equivalent to the old behavior, all route-hook subscriptions have been removed from the leaf rows, and the previously flagged stale-closure on workspaceId in both renderWorkflowItem and renderFolderSection dependency arrays has been corrected. Every useParams call in the leaf rows and their export hooks has been replaced with either a stable prop or a ref read at action time. The handleWorkflowClick callback now carries only stable Zustand store actions in its deps. The workspaceId stale-closure risk flagged in the prior review round is fixed in both renderWorkflowItem and renderFolderSection. The lazy-init pattern for siblingsCacheRef is a straightforward and safe change. No behavioral regressions were identified. No files require special attention. Important Files Changed
Flowchart%%{init: {'theme': 'neutral'}}%%
flowchart TD
A[Tab Navigation\nworkflowId changes] --> B[WorkflowList re-renders]
B --> C[activeWorkflowIdRef.current updated\nno new object identity]
B --> D[isWorkflowActive recomputed\ndepends on workflowId]
B --> E[listContextValue stays stable\nref identity unchanged]
D --> F[renderWorkflowItem recreated]
F --> G{props changed?}
G -->|active flipped: 2 rows| H[WorkflowItem / FolderItem\nRe-renders]
G -->|props identical: N-2 rows| I[React.memo blocks re-render]
E --> J[Context consumers\nno re-render from context]
subgraph Delete / Export callbacks
K[User clicks Delete/Export]
K --> L[activeWorkflowIdRef.current\nread at call time]
L --> M[Correct live workflowId]
end
%%{init: {'theme': 'base', 'themeVariables': {"darkMode": true, "background": "#0d1117", "primaryColor": "#21262d", "primaryTextColor": "#e6edf3", "primaryBorderColor": "#8b949e", "lineColor": "#8b949e", "textColor": "#e6edf3", "edgeLabelBackground": "#161b22", "actorBkg": "#21262d", "actorBorder": "#8b949e", "actorTextColor": "#e6edf3", "actorLineColor": "#8b949e", "signalColor": "#8b949e", "signalTextColor": "#e6edf3", "noteBkgColor": "#373320", "noteBorderColor": "#d4a72c", "noteTextColor": "#f0e6c0", "labelBoxBkgColor": "#21262d", "labelBoxBorderColor": "#8b949e", "labelTextColor": "#e6edf3", "loopTextColor": "#e6edf3", "activationBkgColor": "#30363d", "activationBorderColor": "#8b949e"}}}%%
flowchart TD
A[Tab Navigation\nworkflowId changes] --> B[WorkflowList re-renders]
B --> C[activeWorkflowIdRef.current updated\nno new object identity]
B --> D[isWorkflowActive recomputed\ndepends on workflowId]
B --> E[listContextValue stays stable\nref identity unchanged]
D --> F[renderWorkflowItem recreated]
F --> G{props changed?}
G -->|active flipped: 2 rows| H[WorkflowItem / FolderItem\nRe-renders]
G -->|props identical: N-2 rows| I[React.memo blocks re-render]
E --> J[Context consumers\nno re-render from context]
subgraph Delete / Export callbacks
K[User clicks Delete/Export]
K --> L[activeWorkflowIdRef.current\nread at call time]
L --> M[Correct live workflowId]
end
Reviews (3): Last reviewed commit: "fix(sidebar): add workspaceId to render-..." | Re-trigger Greptile |
renderWorkflowItem/renderFolderSection now pass workspaceId into the rows, so they must list it as a dependency — otherwise a workspace switch that doesn't also change workflowId would leave the callbacks closing over a stale workspaceId (wrong-workspace deletes/exports).
waleedlatif1
commented
Jul 5, 2026
waleedlatif1
commented
Jul 5, 2026
@cursor review |
There was a problem hiding this comment.
✅ Bugbot reviewed your changes and found no new issues!
Comment @cursor review or bugbot run to trigger another review on this PR
Reviewed by Cursor Bugbot for commit ca6f169. Configure here.
waleedlatif1
commented
Jul 5, 2026
waleedlatif1
commented
Jul 5, 2026
@cursor review |
There was a problem hiding this comment.
✅ Bugbot reviewed your changes and found no new issues!
Comment @cursor review or bugbot run to trigger another review on this PR
Reviewed by Cursor Bugbot for commit ca6f169. Configure here.
Summary
useParams, which re-renders on every navigation.React.memoalone can't stop that.WorkflowItemandFolderIteminReact.memo. They were the only un-memoized leaf rows; every sibling row (SidebarNavItem,SidebarChatItem,FileFolderNodeItem) was already memoized.useParams: threadworkspaceIdas a stable prop fromWorkflowList(matching theFileListconvention), and expose the live activeworkflowIdthrough a stableactiveWorkflowIdRefonSidebarListContext, read only inside delete callbacks — never during render.useExportWorkflow/useExportSelection/useExportFolder, used only by these two rows) to takeworkspaceIdas a param instead of callinguseParamsinternally.handleWorkflowClick's identity via refs so the shared list context no longer changes identity on navigation.Mapref.Note: this optimizes tab switches. Clicking a specific workflow still fires
selectOnly(), which mutates the selection Set the rows subscribe to, so those re-render regardless — pre-existing behavior, untouched here.Type of Change
Testing
tsc0 errors,biomecleanfork-file-tree.test.ts— confirmed identical on cleanorigin/staging)activeWorkflowIdRef.currentis semantically equivalent to the oldparams.workflowIdat delete time;memois now genuinely effective (every route-hook subscription removed, all 7 context slots + props stable across tab nav); all export-hook call sites covered. Plus a 4-angle simplify pass (reuse/simplification/efficiency/altitude) — clean.Checklist