From 3d8ef237e3f4e9da738d28cd6fb4dd7fc08a848f Mon Sep 17 00:00:00 2001 From: Mohamed Boudra Date: Thu, 4 Jun 2026 19:10:33 +0700 Subject: [PATCH] Fix workspace tab timeline flicker --- .../workspace/use-mounted-tab-set.test.ts | 52 +++++++++++++ .../screens/workspace/use-mounted-tab-set.ts | 75 ++++++++++--------- 2 files changed, 92 insertions(+), 35 deletions(-) create mode 100644 packages/app/src/screens/workspace/use-mounted-tab-set.test.ts diff --git a/packages/app/src/screens/workspace/use-mounted-tab-set.test.ts b/packages/app/src/screens/workspace/use-mounted-tab-set.test.ts new file mode 100644 index 000000000..1b0a3df45 --- /dev/null +++ b/packages/app/src/screens/workspace/use-mounted-tab-set.test.ts @@ -0,0 +1,52 @@ +// @vitest-environment jsdom + +import { renderHook } from "@testing-library/react"; +import { describe, expect, it } from "vitest"; +import { useMountedTabSet } from "./use-mounted-tab-set"; + +function mountedIds(result: { current: ReturnType }): string[] { + return Array.from(result.current.mountedTabIds); +} + +describe("useMountedTabSet", () => { + it("includes a newly active tab in the same render", () => { + let renderCount = 0; + const { result, rerender } = renderHook( + ({ activeTabId }) => { + renderCount += 1; + return useMountedTabSet({ + activeTabId, + allTabIds: ["first", "second"], + cap: 3, + }); + }, + { initialProps: { activeTabId: "first" } }, + ); + + expect(mountedIds(result)).toEqual(["first"]); + expect(renderCount).toBe(1); + + rerender({ activeTabId: "second" }); + + expect(mountedIds(result)).toEqual(["second", "first"]); + expect(renderCount).toBe(2); + }); + + it("preserves the cap while synchronously adding the active tab", () => { + const { result, rerender } = renderHook( + ({ activeTabId }) => + useMountedTabSet({ + activeTabId, + allTabIds: ["first", "second", "third"], + cap: 2, + }), + { initialProps: { activeTabId: "first" } }, + ); + + rerender({ activeTabId: "second" }); + expect(mountedIds(result)).toEqual(["second", "first"]); + + rerender({ activeTabId: "third" }); + expect(mountedIds(result)).toEqual(["third", "second"]); + }); +}); diff --git a/packages/app/src/screens/workspace/use-mounted-tab-set.ts b/packages/app/src/screens/workspace/use-mounted-tab-set.ts index d23c3dbec..60256b4a6 100644 --- a/packages/app/src/screens/workspace/use-mounted-tab-set.ts +++ b/packages/app/src/screens/workspace/use-mounted-tab-set.ts @@ -1,4 +1,4 @@ -import { useLayoutEffect, useMemo, useRef, useState } from "react"; +import { useLayoutEffect, useMemo, useRef } from "react"; interface UseMountedTabSetInput { activeTabId: string | null; @@ -10,23 +10,36 @@ interface UseMountedTabSetResult { mountedTabIds: Set; } -function createInitialMountedTabIds(input: UseMountedTabSetInput): Set { - if (!input.activeTabId || !input.allTabIds.includes(input.activeTabId)) { - return new Set(); - } - return new Set([input.activeTabId]); +interface DeriveMountedTabLruInput { + activeTabId: string | null; + availableTabIds: Set; + cap: number; + previousLru: string[]; } -function setsEqual(left: Set, right: Set): boolean { - if (left.size !== right.size) { - return false; +function createInitialMountedTabLru(input: UseMountedTabSetInput): string[] { + if (!input.activeTabId || !input.allTabIds.includes(input.activeTabId)) { + return []; } - for (const value of left) { - if (!right.has(value)) { - return false; + return [input.activeTabId]; +} + +function deriveMountedTabLru(input: DeriveMountedTabLruInput): string[] { + const { activeTabId, availableTabIds, cap, previousLru } = input; + const maxSize = Math.max(1, cap); + + const next: string[] = []; + if (activeTabId && availableTabIds.has(activeTabId)) { + next.push(activeTabId); + } + + for (const tabId of previousLru) { + if (next.length >= maxSize) break; + if (tabId !== activeTabId && availableTabIds.has(tabId)) { + next.push(tabId); } } - return true; + return next; } export function useMountedTabSet(input: UseMountedTabSetInput): UseMountedTabSetResult { @@ -36,30 +49,22 @@ export function useMountedTabSet(input: UseMountedTabSetInput): UseMountedTabSet void allTabIdsKey; return new Set(allTabIds); }, [allTabIds, allTabIdsKey]); - const [mountedTabIds, setMountedTabIds] = useState(() => createInitialMountedTabIds(input)); - const lruRef = useRef(activeTabId && allTabIds.includes(activeTabId) ? [activeTabId] : []); + const committedLruRef = useRef(createInitialMountedTabLru(input)); + const mountedTabLru = useMemo( + () => + deriveMountedTabLru({ + activeTabId, + availableTabIds, + cap, + previousLru: committedLruRef.current, + }), + [activeTabId, availableTabIds, cap], + ); + const mountedTabIds = useMemo(() => new Set(mountedTabLru), [mountedTabLru]); useLayoutEffect(() => { - const nextLru = lruRef.current.filter((tabId) => availableTabIds.has(tabId)); - if (activeTabId && availableTabIds.has(activeTabId)) { - const existingIndex = nextLru.indexOf(activeTabId); - if (existingIndex >= 0) { - nextLru.splice(existingIndex, 1); - } - nextLru.unshift(activeTabId); - } - if (nextLru.length > cap) { - nextLru.length = cap; - } - - lruRef.current = nextLru; - setMountedTabIds((previousMountedTabIds) => { - const nextMountedTabIds = new Set(nextLru); - return setsEqual(previousMountedTabIds, nextMountedTabIds) - ? previousMountedTabIds - : nextMountedTabIds; - }); - }, [activeTabId, availableTabIds, cap]); + committedLruRef.current = mountedTabLru; + }, [mountedTabLru]); return { mountedTabIds }; }