mirror of
https://github.com/apache/superset.git
synced 2026-05-21 15:55:10 +00:00
refactor(AsyncSelect): simplify search-restore tracking and tighten tests
Replace wasSearchingRef with usePrevious(inputValue), restructure the fetchPage().then() branching so allValuesLoaded lives in the non-search branch (removes the dead resultData variable), and harden the new tests with disjoint datasets and negative assertions so they would fail against the original merge-on-search bug. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
This commit is contained in:
@@ -926,10 +926,19 @@ test('replaces cached options with search results instead of merging', async ()
|
||||
});
|
||||
|
||||
test('shows all options when filterOption is false', async () => {
|
||||
const loadOptions = jest.fn(async () => ({
|
||||
data: OPTIONS.slice(0, 10),
|
||||
totalCount: 20,
|
||||
const page0Data = Array.from({ length: 10 }, (_, i) => ({
|
||||
label: `Base ${i}`,
|
||||
value: i,
|
||||
}));
|
||||
const searchData = Array.from({ length: 5 }, (_, i) => ({
|
||||
label: `Server ${i}`,
|
||||
value: 100 + i,
|
||||
}));
|
||||
const loadOptions = jest.fn(async (search: string) =>
|
||||
search === ''
|
||||
? { data: page0Data, totalCount: 100 }
|
||||
: { data: searchData, totalCount: 5 },
|
||||
);
|
||||
|
||||
render(
|
||||
<AsyncSelect
|
||||
@@ -945,7 +954,8 @@ test('shows all options when filterOption is false', async () => {
|
||||
await waitFor(() => expect(loadOptions).toHaveBeenCalledTimes(2));
|
||||
|
||||
const options = await findAllSelectOptions();
|
||||
expect(options).toHaveLength(10);
|
||||
expect(options).toHaveLength(5);
|
||||
expect(options[0]).toHaveTextContent('Server 0');
|
||||
});
|
||||
|
||||
test('preserves new option entry across search fetch when allowNewOptions is on', async () => {
|
||||
@@ -972,6 +982,8 @@ test('preserves new option entry across search fetch when allowNewOptions is on'
|
||||
const options = await findAllSelectOptions();
|
||||
expect(options).toHaveLength(1);
|
||||
expect(options[0]).toHaveTextContent('newval');
|
||||
// Stale page-0 options must not bleed through.
|
||||
expect(screen.queryByText('Option 0')).not.toBeInTheDocument();
|
||||
});
|
||||
|
||||
test('restores base options when search is cleared', async () => {
|
||||
@@ -995,12 +1007,16 @@ test('restores base options when search is cleared', async () => {
|
||||
await waitFor(() => expect(loadOptions).toHaveBeenCalledTimes(2));
|
||||
let options = await findAllSelectOptions();
|
||||
expect(options).toHaveLength(1);
|
||||
expect(options[0]).toHaveTextContent('Search Match');
|
||||
|
||||
await type('{backspace}{backspace}{backspace}{backspace}{backspace}{backspace}');
|
||||
// type() clears the input before typing, so passing '' clears the search.
|
||||
await type('');
|
||||
await waitFor(async () => {
|
||||
options = await findAllSelectOptions();
|
||||
expect(options).toHaveLength(10);
|
||||
});
|
||||
expect(options[0]).toHaveTextContent('Option 0');
|
||||
expect(screen.queryByText('Search Match')).not.toBeInTheDocument();
|
||||
});
|
||||
|
||||
test('does not duplicate options when using numeric values', async () => {
|
||||
|
||||
@@ -161,7 +161,6 @@ const AsyncSelect = forwardRef(
|
||||
const selectValueRef = useRef(selectValue);
|
||||
const fetchedQueries = useRef(new Map<string, number>());
|
||||
const initialOptionsRef = useRef<SelectOptionsType>(EMPTY_OPTIONS);
|
||||
const wasSearchingRef = useRef(false);
|
||||
const mappedMode = isSingleMode ? undefined : 'multiple';
|
||||
const allowFetch = !fetchOnlyOnSearch || inputValue;
|
||||
const [maxTagCount, setMaxTagCount] = useState(
|
||||
@@ -337,38 +336,31 @@ const AsyncSelect = forwardRef(
|
||||
const fetchOptions = options as SelectOptionsPagePromise;
|
||||
fetchOptions(search, page, pageSize)
|
||||
.then(({ data, totalCount }: SelectOptionsTypePage) => {
|
||||
let resultData: SelectOptionsType = data;
|
||||
if (search && page === 0) {
|
||||
// Preserve optimistic isNewOption entries inserted by
|
||||
// handleOnSearch so allowNewOptions users can still click
|
||||
// the value they typed when the server returns no match.
|
||||
setSelectOptions((prevOptions: SelectOptionsType) => {
|
||||
const dataValues = new Set(
|
||||
data.map((opt: SelectOptionsType[number]) => opt.value),
|
||||
);
|
||||
// Replace cached options with server results; preserve
|
||||
// optimistic isNewOption entries inserted by handleOnSearch
|
||||
// so allowNewOptions users can still click the value they
|
||||
// typed when the server returns no match.
|
||||
setSelectOptions(prevOptions => {
|
||||
const dataValues = new Set(data.map(opt => opt.value));
|
||||
const preservedNew = prevOptions.filter(
|
||||
(opt: SelectOptionsType[number]) =>
|
||||
opt.isNewOption && !dataValues.has(opt.value),
|
||||
opt => opt.isNewOption && !dataValues.has(opt.value),
|
||||
);
|
||||
return preservedNew
|
||||
.concat(data)
|
||||
.sort(sortComparatorForNoSearch);
|
||||
});
|
||||
} else {
|
||||
resultData = mergeData(data);
|
||||
const mergedData = mergeData(data);
|
||||
if (!search) {
|
||||
initialOptionsRef.current = resultData;
|
||||
initialOptionsRef.current = mergedData;
|
||||
if (!fetchOnlyOnSearch && mergedData.length >= totalCount) {
|
||||
setAllValuesLoaded(true);
|
||||
}
|
||||
}
|
||||
}
|
||||
fetchedQueries.current.set(key, totalCount);
|
||||
setTotalCount(totalCount);
|
||||
if (
|
||||
!fetchOnlyOnSearch &&
|
||||
search === '' &&
|
||||
resultData.length >= totalCount
|
||||
) {
|
||||
setAllValuesLoaded(true);
|
||||
}
|
||||
})
|
||||
.catch(internalOnError)
|
||||
.finally(() => {
|
||||
@@ -526,7 +518,6 @@ const AsyncSelect = forwardRef(
|
||||
setAllValuesLoaded(false);
|
||||
setSelectOptions(EMPTY_OPTIONS);
|
||||
initialOptionsRef.current = EMPTY_OPTIONS;
|
||||
wasSearchingRef.current = false;
|
||||
}, [options]);
|
||||
|
||||
useEffect(() => {
|
||||
@@ -541,19 +532,21 @@ const AsyncSelect = forwardRef(
|
||||
[debouncedFetchPage],
|
||||
);
|
||||
|
||||
const previousInputValue = usePrevious(inputValue, '');
|
||||
useEffect(() => {
|
||||
if (loadingEnabled && allowFetch) {
|
||||
// trigger fetch every time inputValue changes
|
||||
if (inputValue) {
|
||||
wasSearchingRef.current = true;
|
||||
debouncedFetchPage(inputValue, 0);
|
||||
} else {
|
||||
if (wasSearchingRef.current && initialOptionsRef.current.length > 0) {
|
||||
// On returning to empty input after a search, restore the cached
|
||||
// base options so the dropdown shows the original page-0 list
|
||||
// instead of the stale search results.
|
||||
if (previousInputValue && initialOptionsRef.current.length > 0) {
|
||||
setSelectOptions(
|
||||
[...initialOptionsRef.current].sort(sortComparatorForNoSearch),
|
||||
);
|
||||
}
|
||||
wasSearchingRef.current = false;
|
||||
fetchPage('', 0);
|
||||
}
|
||||
}
|
||||
@@ -562,6 +555,7 @@ const AsyncSelect = forwardRef(
|
||||
fetchPage,
|
||||
allowFetch,
|
||||
inputValue,
|
||||
previousInputValue,
|
||||
debouncedFetchPage,
|
||||
sortComparatorForNoSearch,
|
||||
]);
|
||||
|
||||
Reference in New Issue
Block a user