Improve typing and sidebar performance in large workspaces - #516
Conversation
Greptile SummaryThis PR addresses typing lag and sidebar sluggishness in large workspaces through a coherent set of targeted optimizations. Model writes from editors are debounced (400 ms) via a new
Confidence Score: 4/5Safe to merge; all flush-before-send paths are correctly wired and the debounce logic is sound. The core debounce + flush contract is correctly implemented: coalesced patches are always flushed synchronously before any backend operation, and the beforeunload handler guards against window-close data loss. The sidebar virtualization and collapsed-row filtering follow an established pattern. The two open items — a stateSavers map that accumulates dead closures on request deletion, and a scrollMargin offset that is not refreshed when the sidebar layout shifts — are real but narrow: the memory overhead is tiny and scrollToIndex is only invoked when keyboard-navigating to an off-screen row. Files Needing Attention: packages/ui/src/components/tree/TreeItemList.tsx (stale scrollMargin) and apps/yaak-client/components/core/Editor/Editor.tsx (stateSavers cleanup).
|
| Filename | Overview |
|---|---|
| crates/yaak-models/guest-js/store.ts | Adds patchModelDebounced with per-model coalescing, flushAllPendingPatches, and wires into existing flushAllModelWrites; logic is sound and flush ordering is correct |
| packages/common-lib/debounce.ts | Adds flush() method and fixes cancel() to clear lastArgs; implementation is correct |
| apps/yaak-client/components/core/Editor/Editor.tsx | Adds per-stateKey debounced state saving and flushes on unmount; the module-level stateSavers map accumulates dead entries for deleted requests and is never pruned |
| packages/ui/src/components/tree/TreeItemList.tsx | Introduces VirtualTreeItemList using @tanstack/react-virtual; scrollMargin offset is measured once on mount and not refreshed if the container layout changes later |
| packages/ui/src/components/tree/Tree.tsx | Filters hidden/collapsed items out of the render list, virtualizes via TreeItemList, and updates tryFocus to scroll virtualized rows into range before focusing |
| apps/yaak-client/components/Sidebar.tsx | Wraps export with memo to prevent route-navigation re-renders from churning DndContext; straightforward single-line change |
| apps/yaak-client/hooks/useAllRequests.ts | Adds allRequestIdsAtom and allRequestUrlsAtom as identity-stable selectAtom derivations; equality functions are correct |
| apps/yaak-client/hooks/useParentFolders.ts | Keys useMemo on folderId instead of the full model object so URL keystrokes no longer invalidate the memo; correct and clean refactor |
Sequence Diagram
sequenceDiagram
participant User
participant Editor
participant patchModelDebounced
participant debounceTimer as Debounce 400ms
participant pendingPatches as pendingPatches Map
participant flushAllModelWrites
participant Backend
User->>Editor: keystroke
Editor->>patchModelDebounced: patch
patchModelDebounced->>pendingPatches: merge patch into pending entry
patchModelDebounced->>debounceTimer: reset timer
User->>Editor: keystroke
Editor->>patchModelDebounced: patch
patchModelDebounced->>pendingPatches: merge patch coalesced
patchModelDebounced->>debounceTimer: reset timer
alt 400ms idle timer fires
debounceTimer->>pendingPatches: writePendingPatch key
pendingPatches->>Backend: patchModelById invoke models_upsert
else User clicks Send or Connect
User->>flushAllModelWrites: send clicked
flushAllModelWrites->>pendingPatches: flushAllPendingPatches sync
pendingPatches->>Backend: patchModelById invoke models_upsert
flushAllModelWrites->>flushAllModelWrites: await Promise.allSettled
flushAllModelWrites->>Backend: invoke cmd_send_http_request
end
Comments Outside Diff (2)
-
apps/yaak-client/components/core/Editor/Editor.tsx, line 357-360 (link)stateSaversaccumulates entries and never cleans upflushCachedEditorStatecallsflush()on unmount but never removes the entry from the module-levelstateSaversmap. Each uniquestateKeyever mounted adds a permanent closure to the map. For a typical workflow — importing a large OpenAPI spec and then pruning requests — this means hundreds of dead debounce closures accumulate for the lifetime of the window. CallingstateSavers.delete(stateKey)insideflushCachedEditorState, after the flush, would keep the map bounded to currently-mounted editors. -
apps/yaak-client/components/HttpRequestPane.tsx, line 93-95 (link)Identical
requestUrlOptionsAtomduplicated across two modulesHttpRequestPane.tsxandWebsocketRequestPane.tsxeach define their own module-levelrequestUrlOptionsAtomwith identical bodies. Since atoms are module singletons, two separate atom instances are created, meaning two independent subscriptions toallRequestUrlsAtomand two copies of the resulting array. Moving the shared atom touseAllRequests.tswould eliminate the duplication and reduce downstream subscriptions to one.Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!
Reviews (1): Last reviewed commit: "Merge branch 'main' into worktree-perf" | Re-trigger Greptile
| useLayoutEffect(() => { | ||
| const list = listRef.current; | ||
| const scroller = getScrollElement(); | ||
| if (list == null || scroller == null) return; | ||
| const offset = | ||
| list.getBoundingClientRect().top - scroller.getBoundingClientRect().top + scroller.scrollTop; | ||
| setScrollMargin(offset); | ||
| }, [getScrollElement]); |
There was a problem hiding this comment.
scrollMargin computed only once and can go stale
The useLayoutEffect that measures the list-to-scroller offset only reruns when getScrollElement changes, which never happens (it's a stable useCallback with no deps). If the sidebar header grows or shrinks after mount, the cached offset will be wrong and scrollToIndex will land on a different row than intended. A ResizeObserver on the scroll container — or adding nodes.length as a second dependency — would keep this accurate.
Performance work targeting large workspaces (thousands of requests, eg. a big OpenAPI/Postman import).
Typing lag
patchModelDebounced, 400ms). Sends, duplicates, and WS/gRPC connects flush pending patches first. gRPC message stays un-debounced since its send path reads from the frontend store.useParentFoldersonfolderIdso keystrokes no longer rebuild every editor's language/autocomplete extensions.Sidebar
@tanstack/react-virtual(existing pattern from the event viewer). Only visible rows mount, which also shrinks dnd-kit registration/measuring and the per-row drag monitor fan-out.Sidebarso route navigations don't re-render the subtree.Request switching
Follow-ups (not in this PR): lazy-mount editors for inactive request-pane tabs (~64 CodeMirror mounts per switch), batch
model_writeevents after import, SQLite indexes onworkspace_id/folder_id.