diff --git a/apps/web/src/components/knowledge/LinkAutocomplete.tsx b/apps/web/src/components/knowledge/LinkAutocomplete.tsx index 55b03ab..7c75f3f 100644 --- a/apps/web/src/components/knowledge/LinkAutocomplete.tsx +++ b/apps/web/src/components/knowledge/LinkAutocomplete.tsx @@ -1,7 +1,7 @@ "use client"; import React, { useState, useEffect, useRef, useCallback } from "react"; -import { apiGet } from "@/lib/api/client"; +import { apiRequest } from "@/lib/api/client"; import type { KnowledgeEntryWithTags } from "@mosaic/shared"; interface LinkAutocompleteProps { @@ -51,11 +51,14 @@ export function LinkAutocomplete({ const [isLoading, setIsLoading] = useState(false); const dropdownRef = useRef(null); const searchTimeoutRef = useRef(null); + const abortControllerRef = useRef(null); /** - * Search for knowledge entries matching the query + * Search for knowledge entries matching the query. + * Accepts an AbortSignal to allow cancellation of in-flight requests, + * preventing stale results from overwriting newer ones. */ - const searchEntries = useCallback(async (query: string): Promise => { + const searchEntries = useCallback(async (query: string, signal: AbortSignal): Promise => { if (!query.trim()) { setResults([]); return; @@ -63,7 +66,7 @@ export function LinkAutocomplete({ setIsLoading(true); try { - const response = await apiGet<{ + const response = await apiRequest<{ data: KnowledgeEntryWithTags[]; meta: { total: number; @@ -71,7 +74,10 @@ export function LinkAutocomplete({ limit: number; totalPages: number; }; - }>(`/api/knowledge/search?q=${encodeURIComponent(query)}&limit=10`); + }>(`/api/knowledge/search?q=${encodeURIComponent(query)}&limit=10`, { + method: "GET", + signal, + }); const searchResults: SearchResult[] = response.data.map((entry) => ({ id: entry.id, @@ -83,15 +89,23 @@ export function LinkAutocomplete({ setResults(searchResults); setSelectedIndex(0); } catch (error) { + // Ignore aborted requests - a newer search has superseded this one + if (error instanceof DOMException && error.name === "AbortError") { + return; + } console.error("Failed to search entries:", error); setResults([]); } finally { - setIsLoading(false); + if (!signal.aborted) { + setIsLoading(false); + } } }, []); /** - * Debounced search - waits 300ms after user stops typing + * Debounced search - waits 300ms after user stops typing. + * Cancels any in-flight request via AbortController before firing a new one, + * preventing race conditions where older results overwrite newer ones. */ const debouncedSearch = useCallback( (query: string): void => { @@ -99,8 +113,16 @@ export function LinkAutocomplete({ clearTimeout(searchTimeoutRef.current); } + // Abort any in-flight request from a previous search + if (abortControllerRef.current) { + abortControllerRef.current.abort(); + } + searchTimeoutRef.current = setTimeout(() => { - void searchEntries(query); + // Create a new AbortController for this search request + const controller = new AbortController(); + abortControllerRef.current = controller; + void searchEntries(query, controller.signal); }, 300); }, [searchEntries] @@ -321,13 +343,16 @@ export function LinkAutocomplete({ }, [textareaRef, handleInput, handleKeyDown]); /** - * Cleanup timeout on unmount + * Cleanup timeout and abort in-flight requests on unmount */ useEffect(() => { return (): void => { if (searchTimeoutRef.current) { clearTimeout(searchTimeoutRef.current); } + if (abortControllerRef.current) { + abortControllerRef.current.abort(); + } }; }, []); diff --git a/apps/web/src/components/knowledge/__tests__/LinkAutocomplete.test.tsx b/apps/web/src/components/knowledge/__tests__/LinkAutocomplete.test.tsx index bccf0ad..5729c90 100644 --- a/apps/web/src/components/knowledge/__tests__/LinkAutocomplete.test.tsx +++ b/apps/web/src/components/knowledge/__tests__/LinkAutocomplete.test.tsx @@ -8,10 +8,10 @@ import * as apiClient from "@/lib/api/client"; // Mock the API client vi.mock("@/lib/api/client", () => ({ - apiGet: vi.fn(), + apiRequest: vi.fn(), })); -const mockApiGet = apiClient.apiGet as ReturnType; +const mockApiRequest = apiClient.apiRequest as ReturnType; describe("LinkAutocomplete", (): void => { let textareaRef: React.RefObject; @@ -29,7 +29,7 @@ describe("LinkAutocomplete", (): void => { // Reset mocks vi.clearAllMocks(); - mockApiGet.mockResolvedValue({ + mockApiRequest.mockResolvedValue({ data: [], meta: { total: 0, page: 1, limit: 10, totalPages: 0 }, }); @@ -67,6 +67,146 @@ describe("LinkAutocomplete", (): void => { }); }); + it("should pass an AbortSignal to apiRequest for cancellation", async (): Promise => { + vi.useFakeTimers(); + + mockApiRequest.mockResolvedValue({ + data: [], + meta: { total: 0, page: 1, limit: 10, totalPages: 0 }, + }); + + render(); + + const textarea = textareaRef.current; + if (!textarea) throw new Error("Textarea not found"); + + // Simulate typing [[abc + act(() => { + textarea.value = "[[abc"; + textarea.setSelectionRange(5, 5); + fireEvent.input(textarea); + }); + + // Advance past debounce to fire the search + await act(async () => { + await vi.advanceTimersByTimeAsync(300); + }); + + // Verify apiRequest was called with a signal + expect(mockApiRequest).toHaveBeenCalledTimes(1); + const callArgs = mockApiRequest.mock.calls[0] as [ + string, + { method: string; signal: AbortSignal }, + ]; + expect(callArgs[1]).toHaveProperty("signal"); + expect(callArgs[1].signal).toBeInstanceOf(AbortSignal); + + vi.useRealTimers(); + }); + + it("should abort previous in-flight request when a new search fires", async (): Promise => { + vi.useFakeTimers(); + + mockApiRequest.mockResolvedValue({ + data: [], + meta: { total: 0, page: 1, limit: 10, totalPages: 0 }, + }); + + render(); + + const textarea = textareaRef.current; + if (!textarea) throw new Error("Textarea not found"); + + // First search: type [[foo + act(() => { + textarea.value = "[[foo"; + textarea.setSelectionRange(5, 5); + fireEvent.input(textarea); + }); + + // Advance past debounce to fire the first search + await act(async () => { + await vi.advanceTimersByTimeAsync(300); + }); + + expect(mockApiRequest).toHaveBeenCalledTimes(1); + const firstCallArgs = mockApiRequest.mock.calls[0] as [ + string, + { method: string; signal: AbortSignal }, + ]; + const firstSignal = firstCallArgs[1].signal; + expect(firstSignal.aborted).toBe(false); + + // Second search: type [[foobar (user continues typing) + act(() => { + textarea.value = "[[foobar"; + textarea.setSelectionRange(8, 8); + fireEvent.input(textarea); + }); + + // The first signal should be aborted immediately when debouncedSearch fires again + // (abort happens before the timeout, in the debounce function itself) + expect(firstSignal.aborted).toBe(true); + + // Advance past debounce to fire the second search + await act(async () => { + await vi.advanceTimersByTimeAsync(300); + }); + + expect(mockApiRequest).toHaveBeenCalledTimes(2); + const secondCallArgs = mockApiRequest.mock.calls[1] as [ + string, + { method: string; signal: AbortSignal }, + ]; + const secondSignal = secondCallArgs[1].signal; + expect(secondSignal.aborted).toBe(false); + + vi.useRealTimers(); + }); + + it("should abort in-flight request on unmount", async (): Promise => { + vi.useFakeTimers(); + + mockApiRequest.mockResolvedValue({ + data: [], + meta: { total: 0, page: 1, limit: 10, totalPages: 0 }, + }); + + const { unmount } = render( + + ); + + const textarea = textareaRef.current; + if (!textarea) throw new Error("Textarea not found"); + + // Trigger a search + act(() => { + textarea.value = "[[test"; + textarea.setSelectionRange(6, 6); + fireEvent.input(textarea); + }); + + // Advance past debounce + await act(async () => { + await vi.advanceTimersByTimeAsync(300); + }); + + expect(mockApiRequest).toHaveBeenCalledTimes(1); + const callArgs = mockApiRequest.mock.calls[0] as [ + string, + { method: string; signal: AbortSignal }, + ]; + const signal = callArgs[1].signal; + expect(signal.aborted).toBe(false); + + // Unmount the component - should abort in-flight request + unmount(); + + expect(signal.aborted).toBe(true); + + vi.useRealTimers(); + }); + // TODO: Fix async/timer interaction - component works but test has timing issues with fake timers it.skip("should perform debounced search when typing query", async (): Promise => { vi.useFakeTimers(); @@ -93,7 +233,7 @@ describe("LinkAutocomplete", (): void => { meta: { total: 1, page: 1, limit: 10, totalPages: 1 }, }; - mockApiGet.mockResolvedValue(mockResults); + mockApiRequest.mockResolvedValue(mockResults); render(); @@ -108,7 +248,7 @@ describe("LinkAutocomplete", (): void => { }); // Should not call API immediately - expect(mockApiGet).not.toHaveBeenCalled(); + expect(mockApiRequest).not.toHaveBeenCalled(); // Fast-forward 300ms and let promises resolve await act(async () => { @@ -116,7 +256,11 @@ describe("LinkAutocomplete", (): void => { }); await waitFor(() => { - expect(mockApiGet).toHaveBeenCalledWith("/api/knowledge/search?q=test&limit=10"); + expect(mockApiRequest).toHaveBeenCalledWith( + "/api/knowledge/search?q=test&limit=10", + // eslint-disable-next-line @typescript-eslint/no-unsafe-assignment + expect.objectContaining({ method: "GET", signal: expect.any(AbortSignal) }) + ); }); await waitFor(() => { @@ -168,7 +312,7 @@ describe("LinkAutocomplete", (): void => { meta: { total: 2, page: 1, limit: 10, totalPages: 1 }, }; - mockApiGet.mockResolvedValue(mockResults); + mockApiRequest.mockResolvedValue(mockResults); render(); @@ -241,7 +385,7 @@ describe("LinkAutocomplete", (): void => { meta: { total: 1, page: 1, limit: 10, totalPages: 1 }, }; - mockApiGet.mockResolvedValue(mockResults); + mockApiRequest.mockResolvedValue(mockResults); render(); @@ -299,7 +443,7 @@ describe("LinkAutocomplete", (): void => { meta: { total: 1, page: 1, limit: 10, totalPages: 1 }, }; - mockApiGet.mockResolvedValue(mockResults); + mockApiRequest.mockResolvedValue(mockResults); render(); @@ -407,7 +551,7 @@ describe("LinkAutocomplete", (): void => { it.skip("should show 'No entries found' when search returns no results", async (): Promise => { vi.useFakeTimers(); - mockApiGet.mockResolvedValue({ + mockApiRequest.mockResolvedValue({ data: [], meta: { total: 0, page: 1, limit: 10, totalPages: 0 }, }); @@ -444,7 +588,7 @@ describe("LinkAutocomplete", (): void => { const searchPromise = new Promise((resolve) => { resolveSearch = resolve; }); - mockApiGet.mockReturnValue( + mockApiRequest.mockReturnValue( searchPromise as Promise<{ data: unknown[]; meta: { total: number; page: number; limit: number; totalPages: number }; @@ -510,7 +654,7 @@ describe("LinkAutocomplete", (): void => { meta: { total: 1, page: 1, limit: 10, totalPages: 1 }, }; - mockApiGet.mockResolvedValue(mockResults); + mockApiRequest.mockResolvedValue(mockResults); render();