From f958063b548902d19a8b4e560f499301bd2bb3b8 Mon Sep 17 00:00:00 2001 From: dormouse-bot <287024035+dormouse-bot@users.noreply.github.com> Date: Tue, 22 Sep 2026 12:00:42 +0000 Subject: [PATCH 1/2] fix(theme-store): let only the newest theme search write its results Two stale writers reach `results`/`error`/`loading` in ThemeStoreDialog, both of them searches that have already started, which the existing debounce cancel cannot stop: - A request still in flight when the store closes resolves afterwards and repopulates the list the close's reset effect just cleared. The component only renders null while closed, so the state survives: reopening shows the previous query's rows under the empty-state line. - An earlier query whose response lands after a later one's overwrites the newer results, so the list no longer matches the box. One epoch ref, bumped on every search and on every close, gates all three state writes on still being the newest search. --- .../theme-picker/ThemeStoreDialog.test.tsx | 70 ++++++++++++++++++- .../theme-picker/ThemeStoreDialog.tsx | 15 +++- 2 files changed, 81 insertions(+), 4 deletions(-) diff --git a/lib/src/components/theme-picker/ThemeStoreDialog.test.tsx b/lib/src/components/theme-picker/ThemeStoreDialog.test.tsx index f5cec80b4..dab2136fc 100644 --- a/lib/src/components/theme-picker/ThemeStoreDialog.test.tsx +++ b/lib/src/components/theme-picker/ThemeStoreDialog.test.tsx @@ -16,12 +16,23 @@ vi.mock('../../lib/themes', () => ({ setActiveThemeId: vi.fn(), })); -import { searchThemes } from '../../lib/themes'; +import { searchThemes, type OpenVSXExtension } from '../../lib/themes'; import { ThemeStoreDialog } from './ThemeStoreDialog'; import { setNativeFieldValue } from '../../lib/dom'; const searchThemesMock = vi.mocked(searchThemes); +function extension(name: string, displayName: string): OpenVSXExtension { + return { + namespace: 'test', + name, + displayName, + description: '', + version: '1.0.0', + downloadCount: 1, + }; +} + globalThis.IS_REACT_ACT_ENVIRONMENT = true; // jsdom does not implement the native modal methods. @@ -89,4 +100,61 @@ describe('ThemeStoreDialog', () => { vi.useRealTimers(); } }); + + it('discards a search already in flight when the store closes', async () => { + let resolveSearch!: (result: { extensions: OpenVSXExtension[] }) => void; + searchThemesMock.mockImplementationOnce( + () => new Promise((resolve) => { resolveSearch = resolve; }), + ); + + vi.useFakeTimers(); + try { + render(true); + typeQuery('dracula'); + act(() => { vi.advanceTimersByTime(300); }); // the request leaves + expect(searchThemesMock).toHaveBeenCalledTimes(1); + + render(false); // close while it is still in flight + await act(async () => { + resolveSearch({ extensions: [extension('dracula', 'Dracula Official')] }); + }); + render(true); + + expect(container.textContent).not.toContain('Dracula Official'); + expect(container.textContent).toContain('Search for a VS Code theme to install'); + } finally { + vi.useRealTimers(); + } + }); + + it('ignores a superseded search whose response arrives last', async () => { + let resolveFirst!: (result: { extensions: OpenVSXExtension[] }) => void; + let resolveSecond!: (result: { extensions: OpenVSXExtension[] }) => void; + searchThemesMock + .mockImplementationOnce(() => new Promise((resolve) => { resolveFirst = resolve; })) + .mockImplementationOnce(() => new Promise((resolve) => { resolveSecond = resolve; })); + + vi.useFakeTimers(); + try { + render(true); + typeQuery('dra'); + act(() => { vi.advanceTimersByTime(300); }); + typeQuery('nord'); + act(() => { vi.advanceTimersByTime(300); }); + expect(searchThemesMock).toHaveBeenCalledTimes(2); + + // The newer query answers first; the older one lands afterwards. + await act(async () => { + resolveSecond({ extensions: [extension('nord', 'Nord Theme')] }); + }); + await act(async () => { + resolveFirst({ extensions: [extension('dracula', 'Dracula Official')] }); + }); + + expect(container.textContent).toContain('Nord Theme'); + expect(container.textContent).not.toContain('Dracula Official'); + } finally { + vi.useRealTimers(); + } + }); }); diff --git a/lib/src/components/theme-picker/ThemeStoreDialog.tsx b/lib/src/components/theme-picker/ThemeStoreDialog.tsx index f56d878cf..a55358ff4 100644 --- a/lib/src/components/theme-picker/ThemeStoreDialog.tsx +++ b/lib/src/components/theme-picker/ThemeStoreDialog.tsx @@ -28,6 +28,12 @@ export function ThemeStoreDialog({ const [error, setError] = useState(null); const debounceRef = useRef | null>(null); const dialogRef = useRef(null); + // Every search takes a new epoch, and so does every close, so only the newest + // search may write its outcome. Cancelling the debounce below stops the + // searches that have not started yet; a search already in flight needs this, + // in two shapes — an earlier query answering after a later one, and a request + // outliving the close, repopulating the slate the reset effect just cleared. + const searchEpoch = useRef(0); useEffect(() => { const dialog = dialogRef.current; @@ -46,6 +52,7 @@ export function ThemeStoreDialog({ // Cancel any debounce scheduled by the last keystroke; otherwise it fires // doSearch after close and repopulates results/loading for the old query. if (debounceRef.current) clearTimeout(debounceRef.current); + searchEpoch.current += 1; setQuery(''); setResults([]); setError(null); @@ -60,6 +67,8 @@ export function ThemeStoreDialog({ }, []); const doSearch = useCallback(async (value: string) => { + const epoch = ++searchEpoch.current; + const newest = () => searchEpoch.current === epoch; if (!value.trim()) { setResults([]); return; @@ -68,11 +77,11 @@ export function ThemeStoreDialog({ setError(null); try { const response = await searchThemes(value, 0, 20); - setResults(response.extensions); + if (newest()) setResults(response.extensions); } catch (reason) { - setError(reason instanceof Error ? reason.message : 'Search failed'); + if (newest()) setError(reason instanceof Error ? reason.message : 'Search failed'); } finally { - setLoading(false); + if (newest()) setLoading(false); } }, []); From e647f86ca87c552865ec1e3166a4a4dc4964f34a Mon Sep 17 00:00:00 2001 From: dormouse-bot <287024035+dormouse-bot@users.noreply.github.com> Date: Tue, 22 Sep 2026 12:09:21 +0000 Subject: [PATCH 2/2] fix(theme-store): clear the spinner when the box is emptied mid-request MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The epoch is taken before the empty-query early return, so `doSearch('')` supersedes an in-flight request and gates off its `setLoading(false)` — and that branch returned without touching `loading`, leaving the dialog on "Searching..." with an empty box until the next completed search. The empty-query path is the newest search, so it clears the spinner itself. --- .../theme-picker/ThemeStoreDialog.test.tsx | 28 +++++++++++++++++++ .../theme-picker/ThemeStoreDialog.tsx | 4 +++ 2 files changed, 32 insertions(+) diff --git a/lib/src/components/theme-picker/ThemeStoreDialog.test.tsx b/lib/src/components/theme-picker/ThemeStoreDialog.test.tsx index dab2136fc..2b68cf09d 100644 --- a/lib/src/components/theme-picker/ThemeStoreDialog.test.tsx +++ b/lib/src/components/theme-picker/ThemeStoreDialog.test.tsx @@ -127,6 +127,34 @@ describe('ThemeStoreDialog', () => { } }); + it('stops searching when the box is emptied while a request is in flight', async () => { + let resolveSearch!: (result: { extensions: OpenVSXExtension[] }) => void; + searchThemesMock.mockImplementationOnce( + () => new Promise((resolve) => { resolveSearch = resolve; }), + ); + + vi.useFakeTimers(); + try { + render(true); + typeQuery('dracula'); + act(() => { vi.advanceTimersByTime(300); }); + + // Emptying the box supersedes the in-flight request, so nothing it + // resolves with may show — but the spinner it turned on must still go. + typeQuery(''); + act(() => { vi.advanceTimersByTime(300); }); + await act(async () => { + resolveSearch({ extensions: [extension('dracula', 'Dracula Official')] }); + }); + + expect(container.textContent).not.toContain('Searching...'); + expect(container.textContent).not.toContain('Dracula Official'); + expect(container.textContent).toContain('Search for a VS Code theme to install'); + } finally { + vi.useRealTimers(); + } + }); + it('ignores a superseded search whose response arrives last', async () => { let resolveFirst!: (result: { extensions: OpenVSXExtension[] }) => void; let resolveSecond!: (result: { extensions: OpenVSXExtension[] }) => void; diff --git a/lib/src/components/theme-picker/ThemeStoreDialog.tsx b/lib/src/components/theme-picker/ThemeStoreDialog.tsx index a55358ff4..426170011 100644 --- a/lib/src/components/theme-picker/ThemeStoreDialog.tsx +++ b/lib/src/components/theme-picker/ThemeStoreDialog.tsx @@ -71,6 +71,10 @@ export function ThemeStoreDialog({ const newest = () => searchEpoch.current === epoch; if (!value.trim()) { setResults([]); + // An emptied box is the newest search, so it inherits the spinner any + // request it just superseded turned on: that request's `finally` is + // gated off, and nothing else here would clear `loading`. + setLoading(false); return; } setLoading(true);