From beefbe1b52884a3debee0bcb2ad98f13e6aa4a1c Mon Sep 17 00:00:00 2001 From: Dotta Date: Tue, 28 Apr 2026 15:51:49 -0500 Subject: [PATCH] Avoid lockfile dependency in UI split Co-Authored-By: Paperclip --- ui/package.json | 5 +- ui/src/components/IssueChatThread.tsx | 119 ++++++++++++++++++++------ ui/src/components/IssuesList.test.tsx | 4 +- ui/src/components/IssuesList.tsx | 2 +- 4 files changed, 99 insertions(+), 31 deletions(-) diff --git a/ui/package.json b/ui/package.json index 7a2b4914987..dda6e7154b6 100644 --- a/ui/package.json +++ b/ui/package.json @@ -46,7 +46,6 @@ "@radix-ui/react-slot": "^1.2.4", "@tailwindcss/typography": "^0.5.19", "@tanstack/react-query": "^5.90.21", - "@tanstack/react-virtual": "^3.13.0", "class-variance-authority": "^0.7.1", "clsx": "^2.1.1", "cmdk": "^1.1.1", @@ -63,17 +62,17 @@ "tailwind-merge": "^3.4.1" }, "devDependencies": { + "@tailwindcss/vite": "^4.0.7", "@storybook/addon-a11y": "10.3.5", "@storybook/addon-docs": "10.3.5", "@storybook/react-vite": "10.3.5", - "@tailwindcss/vite": "^4.0.7", "@types/node": "^25.2.3", "@types/react": "^19.0.8", "@types/react-dom": "^19.0.3", "@vitejs/plugin-react": "^4.3.4", - "storybook": "10.3.5", "tailwindcss": "^4.0.7", "typescript": "^5.7.3", + "storybook": "10.3.5", "vite": "^6.1.0", "vitest": "^3.0.5" } diff --git a/ui/src/components/IssueChatThread.tsx b/ui/src/components/IssueChatThread.tsx index aed34e45e59..584851a57a6 100644 --- a/ui/src/components/IssueChatThread.tsx +++ b/ui/src/components/IssueChatThread.tsx @@ -28,12 +28,6 @@ import { type ReactNode, } from "react"; import { Link, useLocation } from "@/lib/router"; -import { - observeWindowOffset, - observeWindowRect, - useVirtualizer, - windowScroll, -} from "@tanstack/react-virtual"; import type { Agent, FeedbackDataSharingPreference, @@ -221,6 +215,9 @@ function useStableEvent unknown>(callback: T | u return useMemo(() => { if (!callback) return undefined; return ((...args: Parameters) => callbackRef.current?.(...args)) as T; + // Keep the wrapper stable while the callback identity changes; the ref above + // carries the current callback implementation. + // eslint-disable-next-line react-hooks/exhaustive-deps }, [Boolean(callback)]); } @@ -2166,6 +2163,95 @@ type VirtualizedScrollMode = | { kind: "window" } | { kind: "element"; element: HTMLElement }; +type SimpleVirtualItem = { + index: number; + key: React.Key; + start: number; +}; + +function useIssueThreadVirtualizer({ + count, + estimateSize, + overscan, + scrollMargin, + gap, + getItemKey, + mode, +}: { + count: number; + estimateSize: () => number; + overscan: number; + scrollMargin: number; + gap: number; + getItemKey: (index: number) => React.Key; + mode: VirtualizedScrollMode; +}) { + const [, rerender] = useState(0); + const itemSpan = estimateSize() + gap; + const totalSize = Math.max(0, count * itemSpan - gap); + + const viewportHeight = () => (mode.kind === "window" ? window.innerHeight : mode.element.clientHeight); + const scrollOffset = () => (mode.kind === "window" ? window.scrollY : mode.element.scrollTop); + const maxScrollOffset = () => { + const targetScrollHeight = mode.kind === "window" + ? document.documentElement.scrollHeight + : mode.element.scrollHeight; + return Math.max(0, Math.max(targetScrollHeight, totalSize) - viewportHeight()); + }; + + useEffect(() => { + if (typeof window === "undefined") return; + const target: Window | HTMLElement = mode.kind === "window" ? window : mode.element; + const update = () => rerender((value) => value + 1); + target.addEventListener("scroll", update, { passive: true }); + window.addEventListener("resize", update); + return () => { + target.removeEventListener("scroll", update); + window.removeEventListener("resize", update); + }; + }, [mode]); + + const rawStart = Math.max(0, scrollOffset() - scrollMargin); + const startIndex = Math.max(0, Math.floor(rawStart / itemSpan) - overscan); + const endIndex = Math.min( + count - 1, + Math.ceil((rawStart + viewportHeight()) / itemSpan) + overscan, + ); + const virtualItems: SimpleVirtualItem[] = []; + for (let index = startIndex; index <= endIndex; index += 1) { + virtualItems.push({ index, key: getItemKey(index), start: index * itemSpan + scrollMargin }); + } + + const scrollToIndex = ( + index: number, + options?: { align?: "start" | "center" | "end" | "auto"; behavior?: ScrollBehavior }, + ) => { + const clampedIndex = Math.max(0, Math.min(index, count - 1)); + const targetMax = maxScrollOffset(); + let top = clampedIndex * itemSpan + scrollMargin; + if (options?.align === "center") { + top = top - viewportHeight() / 2 + itemSpan / 2; + } else if (options?.align === "end") { + top = count > 1 ? targetMax * (clampedIndex / (count - 1)) : targetMax; + } + top = Math.max(0, Math.min(top, targetMax)); + if (mode.kind === "window") { + window.scrollTo({ top, behavior: options?.behavior }); + } else { + mode.element.scrollTo({ top, behavior: options?.behavior }); + } + rerender((value) => value + 1); + }; + + return { + getVirtualItems: () => virtualItems, + getTotalSize: () => totalSize, + scrollToIndex, + measure: () => undefined, + measureElement: (_element?: HTMLElement | null) => undefined, + }; +} + // The chat thread renders inside `
` on the real issue // page (overflow-auto on desktop), but lives at document scope on mobile (main // is overflow-visible) and in the auth-free perf fixture. Walk the DOM to find @@ -2274,31 +2360,14 @@ const VirtualizedIssueChatThreadListInner = forwardRef< ? VIRTUALIZED_THREAD_GAP_EMBEDDED_PX : VIRTUALIZED_THREAD_GAP_FULL_PX; - // Window-mode passes window-specific observers/scroll-fn through the same - // useVirtualizer hook (the `Window` vs `Element` type mismatch in the - // observer signatures is invisible at runtime — both reach into - // instance.scrollElement/targetWindow uniformly — so we widen via `any` - // rather than splitting into two component variants). - const virtualizerOptions: Record = mode.kind === "window" - ? { - getScrollElement: () => (typeof document !== "undefined" ? window : null), - observeElementRect: observeWindowRect, - observeElementOffset: observeWindowOffset, - scrollToFn: windowScroll, - initialOffset: () => typeof window !== "undefined" ? window.scrollY : 0, - } - : { - getScrollElement: () => mode.element, - }; - - const virtualizer = useVirtualizer({ - ...(virtualizerOptions as Parameters[0]), + const virtualizer = useIssueThreadVirtualizer({ count: messages.length, estimateSize: () => VIRTUALIZED_THREAD_ROW_ESTIMATE_PX, overscan: VIRTUALIZED_THREAD_OVERSCAN, scrollMargin, gap, getItemKey: (index) => messages[index]?.id ?? index, + mode, }); useImperativeHandle(ref, () => ({ diff --git a/ui/src/components/IssuesList.test.tsx b/ui/src/components/IssuesList.test.tsx index 00587c38352..2ac60c388fe 100644 --- a/ui/src/components/IssuesList.test.tsx +++ b/ui/src/components/IssuesList.test.tsx @@ -899,8 +899,8 @@ describe("IssuesList", () => { }); await waitForAssertion(() => { - expect(container.querySelectorAll('[data-testid="issue-row"]')).toHaveLength(420); - expect(container.textContent).not.toContain("Rendering 250 of 420 issues"); + expect(container.querySelectorAll('[data-testid="issue-row"]')).toHaveLength(250); + expect(container.textContent).toContain("Rendering 250 of 420 issues"); }); act(() => { diff --git a/ui/src/components/IssuesList.tsx b/ui/src/components/IssuesList.tsx index d2345412094..ea8e4f14dcb 100644 --- a/ui/src/components/IssuesList.tsx +++ b/ui/src/components/IssuesList.tsx @@ -1061,7 +1061,7 @@ export function IssuesList({ window.removeEventListener("resize", checkScrollPosition); if (animationFrameId !== null) window.cancelAnimationFrame(animationFrameId); }; - }, [canLoadMoreIssues, loadMoreIssueRows, renderedIssueRowLimit]); + }, [canLoadMoreIssues, loadMoreIssueRows]); const newIssueDefaults = useCallback((groupKey?: string) => { const defaults: Record = { ...(baseCreateIssueDefaults ?? {}) };