From f75caf9f3d3e9b5352686c9fe4f9fd35abc43716 Mon Sep 17 00:00:00 2001 From: Patrik Oldsberg Date: Fri, 20 Oct 2023 12:44:25 +0200 Subject: [PATCH] search-react: fix search bar race Signed-off-by: Patrik Oldsberg --- .changeset/real-carrots-brake.md | 5 ++++ .../components/SearchBar/SearchBar.test.tsx | 1 - .../src/components/SearchBar/SearchBar.tsx | 27 +++++++++++++++---- .../SearchModal/SearchModal.test.tsx | 3 +-- 4 files changed, 28 insertions(+), 8 deletions(-) create mode 100644 .changeset/real-carrots-brake.md diff --git a/.changeset/real-carrots-brake.md b/.changeset/real-carrots-brake.md new file mode 100644 index 0000000000..e9ae1f0c27 --- /dev/null +++ b/.changeset/real-carrots-brake.md @@ -0,0 +1,5 @@ +--- +'@backstage/plugin-search-react': patch +--- + +Fixed a rare occurrence where a race in the search bar could throw away user input or cause the clear button not to work. diff --git a/plugins/search-react/src/components/SearchBar/SearchBar.test.tsx b/plugins/search-react/src/components/SearchBar/SearchBar.test.tsx index d9b29584f8..8a3dd84286 100644 --- a/plugins/search-react/src/components/SearchBar/SearchBar.test.tsx +++ b/plugins/search-react/src/components/SearchBar/SearchBar.test.tsx @@ -339,7 +339,6 @@ describe('SearchBar', () => { value = 'new value'; await user.clear(textbox); - await waitFor(() => expect(textbox.value).toBe('')); // make sure new term is captured await user.type(textbox, value); diff --git a/plugins/search-react/src/components/SearchBar/SearchBar.tsx b/plugins/search-react/src/components/SearchBar/SearchBar.tsx index cfd0538694..d101efa669 100644 --- a/plugins/search-react/src/components/SearchBar/SearchBar.tsx +++ b/plugins/search-react/src/components/SearchBar/SearchBar.tsx @@ -30,6 +30,7 @@ import React, { KeyboardEvent, useCallback, useEffect, + useRef, useState, } from 'react'; import useDebounce from 'react-use/lib/useDebounce'; @@ -88,14 +89,28 @@ export const SearchBarBase: ForwardRefExoticComponent = const configApi = useApi(configApiRef); const [value, setValue] = useState(''); + const forwardedValueRef = useRef(''); useEffect(() => { - setValue(prevValue => - prevValue !== defaultValue ? String(defaultValue) : prevValue, - ); - }, [defaultValue]); + setValue(prevValue => { + // We only update the value if our current value is the same as it was + // for the most recent onChange call. Otherwise it means that the users + // has continued typing and we should not replace their input. + if (prevValue === forwardedValueRef.current) { + return String(defaultValue); + } + return prevValue; + }); + }, [defaultValue, forwardedValueRef]); - useDebounce(() => onChange(value), debounceTime, [value]); + useDebounce( + () => { + forwardedValueRef.current = value; + onChange(value); + }, + debounceTime, + [value], + ); const handleChange = useCallback( (e: ChangeEvent) => { @@ -115,7 +130,9 @@ export const SearchBarBase: ForwardRefExoticComponent = ); const handleClear = useCallback(() => { + forwardedValueRef.current = ''; onChange(''); + setValue(''); if (onClear) { onClear(); } diff --git a/plugins/search/src/components/SearchModal/SearchModal.test.tsx b/plugins/search/src/components/SearchModal/SearchModal.test.tsx index 6fce560800..0d0bfdef5c 100644 --- a/plugins/search/src/components/SearchModal/SearchModal.test.tsx +++ b/plugins/search/src/components/SearchModal/SearchModal.test.tsx @@ -15,7 +15,7 @@ */ import React from 'react'; -import { screen, waitFor } from '@testing-library/react'; +import { screen } from '@testing-library/react'; import { renderInTestApp, TestApiRegistry } from '@backstage/test-utils'; import userEvent from '@testing-library/user-event'; import { configApiRef } from '@backstage/core-plugin-api'; @@ -205,7 +205,6 @@ describe('SearchModal', () => { const input = screen.getByLabelText('Search'); await userEvent.clear(input); - await waitFor(() => expect(input.value).toBe('')); await userEvent.type(input, 'new term{enter}'); expect(navigate).toHaveBeenCalledWith('/search?query=new term');