From 5ee5c7af7bde42de81490d6e851fa458f1bb7fd0 Mon Sep 17 00:00:00 2001 From: Tobias Skarhed <1438972+tskarhed@users.noreply.github.com> Date: Fri, 25 Oct 2024 10:10:02 +0200 Subject: [PATCH] Combobox: Invalidate async calls using counter instead of value (#95049) * Combobox async: Invalidate using timestamp instead of value * Simulate keyboard delay in test * Use a counter instead of timestamps * Fix feedback * Expand tests to include case discussed on PR --------- Co-authored-by: Joao Silva --- .../src/components/Combobox/Combobox.test.tsx | 41 +++++++++++++------ .../components/Combobox/useLatestAsyncCall.ts | 7 ++-- 2 files changed, 32 insertions(+), 16 deletions(-) diff --git a/packages/grafana-ui/src/components/Combobox/Combobox.test.tsx b/packages/grafana-ui/src/components/Combobox/Combobox.test.tsx index 56a26d8ed55..aadb3b65f48 100644 --- a/packages/grafana-ui/src/components/Combobox/Combobox.test.tsx +++ b/packages/grafana-ui/src/components/Combobox/Combobox.test.tsx @@ -179,6 +179,8 @@ describe('Combobox', () => { return new Promise((resolve) => setTimeout(() => resolve([{ value: 'first' }]), 1000)); } else if (searchTerm === 'ab') { return new Promise((resolve) => setTimeout(() => resolve([{ value: 'second' }]), 200)); + } else if (searchTerm === 'abc') { + return new Promise((resolve) => setTimeout(() => resolve([{ value: 'third' }]), 500)); } return Promise.resolve([]); }); @@ -186,26 +188,39 @@ describe('Combobox', () => { render(); const input = screen.getByRole('combobox'); - await user.click(input); // First request + await user.click(input); + await user.keyboard('abc'); + jest.advanceTimersByTime(200); // Resolve the second request, should be ignored - await user.keyboard('ab'); // Second request - jest.advanceTimersByTime(210); // Resolve the second request - - let item: HTMLElement | null = await screen.findByRole('option', { name: 'second' }); let firstItem = screen.queryByRole('option', { name: 'first' }); + let secondItem = screen.queryByRole('option', { name: 'second' }); + let thirdItem = screen.queryByRole('option', { name: 'third' }); - expect(item).toBeInTheDocument(); expect(firstItem).not.toBeInTheDocument(); + expect(secondItem).not.toBeInTheDocument(); + expect(thirdItem).not.toBeInTheDocument(); - act(() => { - jest.advanceTimersByTime(1100); // Resolve the first request - }); + jest.advanceTimersByTime(500); // Resolve the third request, should be shown - item = screen.queryByRole('option', { name: 'first' }); - firstItem = screen.queryByRole('option', { name: 'second' }); + firstItem = screen.queryByRole('option', { name: 'first' }); + secondItem = screen.queryByRole('option', { name: 'second' }); + thirdItem = await screen.findByRole('option', { name: 'third' }); - expect(item).not.toBeInTheDocument(); - expect(firstItem).toBeInTheDocument(); + expect(firstItem).not.toBeInTheDocument(); + expect(secondItem).not.toBeInTheDocument(); + expect(thirdItem).toBeInTheDocument(); + + jest.advanceTimersByTime(1000); // Resolve the first request, should be ignored + + firstItem = screen.queryByRole('option', { name: 'first' }); + secondItem = screen.queryByRole('option', { name: 'second' }); + thirdItem = screen.queryByRole('option', { name: 'third' }); + + expect(firstItem).not.toBeInTheDocument(); + expect(secondItem).not.toBeInTheDocument(); + expect(thirdItem).toBeInTheDocument(); + + jest.clearAllTimers(); }); it('should allow custom value while async is being run', async () => { diff --git a/packages/grafana-ui/src/components/Combobox/useLatestAsyncCall.ts b/packages/grafana-ui/src/components/Combobox/useLatestAsyncCall.ts index 3e923ac05a1..df6747ebab9 100644 --- a/packages/grafana-ui/src/components/Combobox/useLatestAsyncCall.ts +++ b/packages/grafana-ui/src/components/Combobox/useLatestAsyncCall.ts @@ -7,16 +7,17 @@ type AsyncFn = (value: T) => Promise; * Used to prevent a faster call being overwritten by an earlier slower call. */ export function useLatestAsyncCall(fn: AsyncFn): AsyncFn { - const latestValue = useRef(); + const latestValueCount = useRef(0); const wrappedFn = useCallback( (value: T) => { - latestValue.current = value; + latestValueCount.current++; + const requestCount = latestValueCount.current; return new Promise((resolve, reject) => { fn(value).then((result) => { // Only resolve if the value is still the latest - if (latestValue.current === value) { + if (requestCount === latestValueCount.current) { resolve(result); } else { reject(new StaleResultError());