perf(web): coalesce overlapping file tree refreshes

apps/web/src/components/files/useDirectoryEntries.browser.test.tsx

@@ -1,66 +1,216 @@
 import { RegistryContext } from "@effect/atom-react";
-import { EnvironmentId } from "@t3tools/contracts";
 import * as Cause from "effect/Cause";
+import * as Effect from "effect/Effect";
 import { AsyncResult, Atom } from "effect/reactivity";
-import { describe, expect, it, vi } from "vite-plus/test";
-
 import { appAtomRegistry } from "~/rpc/atomRegistry";
+import { EnvironmentId, type ProjectEntry } from "@t3tools/contracts";
+import { useEffect } from "react";
 import { act, render } from "../../../test/render";
-import { useDirectoryEntries } from "./useDirectoryEntries";
+import { expect, it, vi } from "vite-plus/test";
 
-const mocks = vi.hoisted(() => ({
-  reachable: null as unknown as Atom.Writable<boolean>,
-  connectedSession: null as unknown as Atom.Writable<number>,
+const { listEntries, mocks } = vi.hoisted(() => ({
+  listEntries: vi.fn(),
+  mocks: {
+    reconnect: false,
+    reachable: null as unknown as Atom.Writable<boolean>,
+    connectedSession: null as unknown as Atom.Writable<number>,
+  },
 }));
-
 vi.mock("~/connection/catalog", () => ({
-  // Derived like the real one, which only follows the connection once something computes it.
   environmentCatalog: {
     sessionGenerationAtom: () => Atom.make((get) => get(mocks.connectedSession)),
   },
 }));
 vi.mock("~/state/projects", () => ({
   projectEnvironment: {
-    listEntries: () =>
-      Atom.make((get) =>
-        get(mocks.reachable)
-          ? AsyncResult.success({
-              entries: [{ path: "README.md", kind: "file" }],
-              truncated: false,
-            })
-          : AsyncResult.failure(Cause.fail(new Error("Environment is reconnecting."))),
-      ),
+    listEntries: (request: unknown) =>
+      mocks.reconnect
+        ? Atom.make((get) =>
+            get(mocks.reachable)
+              ? AsyncResult.success({
+                  entries: [{ path: "README.md", kind: "file" }],
+                  truncated: false,
+                })
+              : AsyncResult.failure(Cause.fail(new Error("Environment is reconnecting."))),
+          )
+        : Atom.make(Effect.promise(() => listEntries(request))),
   },
 }));
 
+import { useDirectoryEntries } from "./useDirectoryEntries";
+
 function setup() {
+  mocks.reconnect = false;
   mocks.reachable = Atom.make(false);
   mocks.connectedSession = Atom.make(1);
+  let view!: ReturnType<typeof useDirectoryEntries>;
+  const calls: Array<{ path: string; resolve: (entries: readonly ProjectEntry[]) => void }> = [];
+  let activeQueries = 0;
+  let peakQueries = 0;
+  function HookProbe() {
+    const result = useDirectoryEntries(EnvironmentId.make("files-test"), "/workspace");
+    useEffect(() => {
+      view = result;
+    });
+    return null;
+  }
+
+  async function settle(index: number, entries: readonly ProjectEntry[] = []) {
+    await act(async () => calls[index]!.resolve(entries));
+  }
+
+  listEntries.mockReset().mockImplementation((request) => {
+    activeQueries++;
+    peakQueries = Math.max(peakQueries, activeQueries);
+    return new Promise((resolve) => {
+      calls.push({
+        path: request.input.directoryPath,
+        resolve: (entries) => {
+          activeQueries--;
+          resolve({ entries });
+        },
+      });
+    });
+  });
+  return {
+    Probe: () => (
+      <RegistryContext.Provider value={appAtomRegistry}>
+        <HookProbe />
+      </RegistryContext.Provider>
+    ),
+    settle,
+    calls,
+    get view() {
+      return view;
+    },
+    get activeQueries() {
+      return activeQueries;
+    },
+    resetPeakQueries() {
+      peakQueries = activeQueries;
+    },
+    get peakQueries() {
+      return peakQueries;
+    },
+  };
 }
 
-function Probe() {
+it("coalesces 20 overlapping refreshes", async () => {
+  const state = setup();
+  // The hook probe does not navigate.
+  await using _screen = await render(<state.Probe />, { router: false });
+  await state.settle(0);
+  await act(async () => {
+    for (let index = 0; index < 20; index++) state.view.refresh();
+  });
+  for (let index = 1; index < state.calls.length; index++) await state.settle(index);
+  expect(state.calls).toHaveLength(3); // Initial load, active refresh, one final rescan.
+  expect(state.view.isPending).toBe(false);
+});
+
+it("rescans mutations during both the initial listing and the follow-up", async () => {
+  const state = setup();
+  // The hook probe does not navigate.
+  await using _screen = await render(<state.Probe />, { router: false });
+  await act(async () => state.view.refresh());
+  await state.settle(0);
+  expect(state.calls).toHaveLength(2);
+  await act(async () => {
+    state.view.refresh();
+    state.view.refresh();
+  });
+  await state.settle(1);
+  expect(state.calls).toHaveLength(3);
+  const entry = { path: "latest.txt", kind: "file" } as const;
+  await state.settle(2, [entry]);
+  expect(state.view.entries).toEqual([entry]);
+  expect(state.view.isPending).toBe(false);
+});
+
+it("shares the follow-up promise and does not resolve callers before the final listing", async () => {
+  const state = setup();
+  // The hook probe does not navigate.
+  await using _screen = await render(<state.Probe />, { router: false });
+  let completed = false;
+  const first = state.view.load("", true);
+  const second = state.view.load("", true);
+  expect(second).toBe(first);
+  void first.then(() => {
+    completed = true;
+  });
+  await state.settle(0);
+  expect(completed).toBe(false);
+  await state.settle(1);
+  expect(completed).toBe(true);
+});
+
+it("retains the four-query concurrency bound across directories and refreshes", async () => {
+  const folders = Array.from({ length: 8 }, (_, index) => ({
+    path: `folder-${index}`,
+    kind: "directory" as const,
+  }));
+  const state = setup();
+  // The hook probe does not navigate.
+  await using _screen = await render(<state.Probe />, { router: false });
+  await state.settle(0, folders);
+  await act(async () => {
+    for (const folder of folders) void state.view.load(folder.path);
+  });
+  for (let index = 1; index < state.calls.length; index++) {
+    await state.settle(index);
+  }
+  expect(state.activeQueries).toBe(0);
+  const refreshStart = state.calls.length;
+  state.resetPeakQueries();
+  await act(async () => state.view.refresh());
+  expect(state.peakQueries).toBe(4);
+  expect(state.activeQueries).toBe(4);
+  await act(async () => {
+    for (let index = 0; index < 20; index++) state.view.refresh();
+  });
+  for (let index = refreshStart; index < state.calls.length; index++) {
+    await state.settle(index, state.calls[index]!.path === "" ? folders : []);
+    expect(state.activeQueries).toBeLessThanOrEqual(4);
+  }
+  expect(state.peakQueries).toBe(4);
+  expect(new Set(state.calls.map((call) => call.path)).size).toBe(9);
+  for (const path of ["", ...folders.map((folder) => folder.path)]) {
+    expect(state.calls.filter((call) => call.path === path).length).toBeLessThanOrEqual(3);
+  }
+  expect(state.view.isPending).toBe(false);
+});
+
+it("does not start a queued follow-up after unmount", async () => {
+  const state = setup();
+  // The hook probe does not navigate.
+  await using screen = await render(<state.Probe />, { router: false });
+  await act(async () => state.view.refresh());
+  await screen.unmount();
+  await state.settle(0);
+  expect(state.calls).toHaveLength(1);
+});
+
+function ReconnectProbe() {
   const directory = useDirectoryEntries(EnvironmentId.make("env-files"), "/repo");
   return (
     <output>{directory.error ?? directory.entries.map((entry) => entry.path).join(",")}</output>
   );
 }
 
-describe("useDirectoryEntries", () => {
-  it("reads a folder that failed while unreachable again once a new session is up", async () => {
-    setup();
-    await using screen = await render(
-      <RegistryContext.Provider value={appAtomRegistry}>
-        <Probe />
-      </RegistryContext.Provider>,
-      // A bare hook probe; nothing here navigates.
-      { router: false },
-    );
-    await expect.element(screen.getByText("Environment is reconnecting.")).toBeInTheDocument();
-
-    await act(() => {
-      appAtomRegistry.set(mocks.reachable, true);
-      appAtomRegistry.set(mocks.connectedSession, 2);
-    });
-    await expect.element(screen.getByText("README.md")).toBeInTheDocument();
+it("reads a folder that failed while unreachable again once a new session is up", async () => {
+  setup();
+  mocks.reconnect = true;
+  await using screen = await render(
+    <RegistryContext.Provider value={appAtomRegistry}>
+      <ReconnectProbe />
+    </RegistryContext.Provider>,
+    // The hook probe does not navigate.
+    { router: false },
+  );
+  await expect.element(screen.getByText("Environment is reconnecting.")).toBeInTheDocument();
+  await act(() => {
+    appAtomRegistry.set(mocks.reachable, true);
+    appAtomRegistry.set(mocks.connectedSession, 2);
   });
+  await expect.element(screen.getByText("README.md")).toBeInTheDocument();
 });

apps/web/src/components/files/useDirectoryEntries.ts

@@ -16,6 +16,7 @@ export function useDirectoryEntries(environmentId: EnvironmentId, cwd: string) {
     () => new Set([""]),
   );
   const requests = useRef(new Map<string, Promise<void>>());
+  const followUpRefreshes = useRef(new Map<string, Promise<void>>());
   const loaded = useRef(new Set<string>());
   const requested = useRef(new Set<string>());
   const active = useRef(true);
@@ -86,8 +87,18 @@ export function useDirectoryEntries(environmentId: EnvironmentId, cwd: string) {
 
   const load = function loadDirectory(directoryPath: string, refresh = false): Promise<void> {
     const existing = requests.current.get(directoryPath);
-    if (existing)
-      return refresh ? existing.then(() => loadDirectory(directoryPath, true)) : existing;
+    if (existing) {
+      if (!refresh) return existing;
+      let followUp = followUpRefreshes.current.get(directoryPath);
+      if (!followUp) {
+        followUp = existing.then(() => {
+          followUpRefreshes.current.delete(directoryPath);
+          if (active.current) return loadDirectory(directoryPath, true);
+        });
+        followUpRefreshes.current.set(directoryPath, followUp);
+      }
+      return followUp;
+    }
     if (!refresh && loaded.current.has(directoryPath)) return Promise.resolve();
     setPending((count) => count + 1);
     if (!refresh) setLoadingDirectories((previous) => new Set(previous).add(directoryPath));
@@ -128,15 +139,10 @@ export function useDirectoryEntries(environmentId: EnvironmentId, cwd: string) {
 
   const refresh = () => {
     const paths = [...requested.current].filter((path) => reachableDirectories.has(path));
-    const workerCount = Math.min(4, paths.length);
-    const worker = async () => {
-      while (active.current) {
-        const path = paths.shift();
-        if (path === undefined) return;
-        await load(path, true);
-      }
-    };
-    for (let index = 0; index < workerCount; index++) void worker();
+    for (const path of paths) {
+      if (!active.current) break;
+      void load(path, true);
+    }
   };
 
   return {