From 0cd9aa82e75c846ef0db14bbf452c766dd4dec68 Mon Sep 17 00:00:00 2001 From: Joey Shi Date: Fri, 10 Jul 2026 11:08:18 -0700 Subject: [PATCH] fix(evalboard): fix search bar clear issue --- .../_components/__tests__/search-box.test.tsx | 75 +++++++++++++++++++ evalboard/app/_components/search-box.tsx | 20 ++++- 2 files changed, 91 insertions(+), 4 deletions(-) create mode 100644 evalboard/app/_components/__tests__/search-box.test.tsx diff --git a/evalboard/app/_components/__tests__/search-box.test.tsx b/evalboard/app/_components/__tests__/search-box.test.tsx new file mode 100644 index 00000000..2b30f8ae --- /dev/null +++ b/evalboard/app/_components/__tests__/search-box.test.tsx @@ -0,0 +1,75 @@ +import { describe, expect, test, vi, beforeEach, afterEach } from "vitest"; +import { render, screen, act, fireEvent } from "@testing-library/react"; + +// vi.hoisted ensures these are initialized before vi.mock hoists its factory. +const { mockReplace, navState } = vi.hoisted(() => ({ + mockReplace: vi.fn(), + navState: { q: "" as string }, +})); + +vi.mock("next/navigation", () => ({ + useRouter: () => ({ replace: mockReplace }), + usePathname: () => "/", + useSearchParams: () => new URLSearchParams(navState.q ? `q=${navState.q}` : ""), +})); + +const { SearchBox } = await import("../search-box"); + +describe("SearchBox — typing-ahead race condition", () => { + beforeEach(() => { + vi.useFakeTimers(); + mockReplace.mockClear(); + navState.q = ""; + }); + afterEach(() => { + vi.useRealTimers(); + }); + + test("preserves in-progress input when a navigation resolves mid-typing", () => { + // Reproduces the race: user types "foo" → debounce fires → user types + // more → navigation for "foo" resolves → input must NOT reset to "foo". + const { rerender } = render(); + const input = screen.getByRole("textbox"); + + fireEvent.change(input, { target: { value: "foo" } }); + + // Debounce fires; typingAhead becomes false. + act(() => { vi.advanceTimersByTime(300); }); + expect(mockReplace).toHaveBeenCalledOnce(); + + // User types more before the navigation resolves. + fireEvent.change(input, { target: { value: "foobar" } }); + + // Navigation for "foo" resolves — URL now reports "foo". + navState.q = "foo"; + rerender(); + + // typingAhead is true, so the sync effect must NOT overwrite the input. + expect(input).toHaveValue("foobar"); + }); + + test("syncs from URL when the user is not typing (external navigation)", () => { + // Back/forward nav or a tag click should still update the input when the + // user hasn't typed anything since the last URL write. + const { rerender } = render(); + const input = screen.getByRole("textbox"); + + navState.q = "tag:alpha"; + rerender(); + + expect(input).toHaveValue("tag:alpha"); + }); + + test("clears the input when the URL is cleared externally", () => { + navState.q = "foo"; + const { rerender } = render(); + const input = screen.getByRole("textbox"); + + expect(input).toHaveValue("foo"); + + navState.q = ""; + rerender(); + + expect(input).toHaveValue(""); + }); +}); diff --git a/evalboard/app/_components/search-box.tsx b/evalboard/app/_components/search-box.tsx index 2a0e35f8..52b9563d 100644 --- a/evalboard/app/_components/search-box.tsx +++ b/evalboard/app/_components/search-box.tsx @@ -1,7 +1,7 @@ "use client"; import { usePathname, useRouter, useSearchParams } from "next/navigation"; -import { useEffect, useState } from "react"; +import { useEffect, useRef, useState } from "react"; const Q_DEBOUNCE_MS = 300; @@ -18,18 +18,28 @@ export function SearchBox({ const urlQ = searchParams.get("q") ?? ""; const [q, setQ] = useState(urlQ); + // True while the user has typed ahead of the last URL write. Prevents the + // URL-sync effect from overwriting in-progress input when a navigation + // triggered by the debounce resolves asynchronously. + const typingAhead = useRef(false); // Sync local state when the URL changes externally (back/forward, link - // clicks). The debounced write below early-returns when state and URL - // agree, so this can't loop. + // clicks). Skipped while the user is ahead of the URL to avoid clobbering + // in-progress input with a stale value from a just-resolved navigation. useEffect(() => { + if (typingAhead.current) return; setQ((prev) => (prev.trim() === urlQ ? prev : urlQ)); }, [urlQ]); useEffect(() => { const trimmed = q.trim(); - if (trimmed === urlQ) return; + if (trimmed === urlQ) { + typingAhead.current = false; + return; + } + typingAhead.current = true; const timer = setTimeout(() => { + typingAhead.current = false; // Read the live URL at fire time so a concurrent write (e.g. a // tag click that landed during the debounce) isn't clobbered. const params = new URLSearchParams(window.location.search); @@ -40,6 +50,8 @@ export function SearchBox({ scroll: false, }); }, Q_DEBOUNCE_MS); + // Don't clear typingAhead in cleanup — the timer was cancelled because + // the user typed another character, so they're still ahead of the URL. return () => clearTimeout(timer); }, [q, urlQ, pathname, router]);