Fix workspace tab timeline flicker

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<typeof useMountedTabSet> }): 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"]);
+  });
+});

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<string>;
 }
 
-function createInitialMountedTabIds(input: UseMountedTabSetInput): Set<string> {
+interface DeriveMountedTabLruInput {
-  if (!input.activeTabId || !input.allTabIds.includes(input.activeTabId)) {
+  activeTabId: string | null;
-    return new Set<string>();
+  availableTabIds: Set<string>;
-  }
+  cap: number;
-  return new Set<string>([input.activeTabId]);
+  previousLru: string[];
 }
 
-function setsEqual(left: Set<string>, right: Set<string>): boolean {
+function createInitialMountedTabLru(input: UseMountedTabSetInput): string[] {
-  if (left.size !== right.size) {
+  if (!input.activeTabId || !input.allTabIds.includes(input.activeTabId)) {
-    return false;
+    return [];
   }
-  for (const value of left) {
+  return [input.activeTabId];
-    if (!right.has(value)) {
+}
-      return false;
+
+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 committedLruRef = useRef(createInitialMountedTabLru(input));
-  const lruRef = useRef(activeTabId && allTabIds.includes(activeTabId) ? [activeTabId] : []);
+  const mountedTabLru = useMemo(
+    () =>
+      deriveMountedTabLru({
+        activeTabId,
+        availableTabIds,
+        cap,
+        previousLru: committedLruRef.current,
+      }),
+    [activeTabId, availableTabIds, cap],
+  );
+  const mountedTabIds = useMemo(() => new Set<string>(mountedTabLru), [mountedTabLru]);
 
   useLayoutEffect(() => {
-    const nextLru = lruRef.current.filter((tabId) => availableTabIds.has(tabId));
+    committedLruRef.current = mountedTabLru;
-    if (activeTabId && availableTabIds.has(activeTabId)) {
+  }, [mountedTabLru]);
-      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]);
 
   return { mountedTabIds };
 }