From 4c98f05f0a435c2386b255b89a6d0b019fd4f167 Mon Sep 17 00:00:00 2001 From: Jacob Raihle kdm951 Date: Tue, 15 Jul 2025 16:29:14 +0300 Subject: [PATCH] Fix the race condition in EntityListProvider Signed-off-by: Jacob Raihle kdm951 --- .../src/hooks/useEntityListProvider.tsx | 118 ++++++++++-------- 1 file changed, 67 insertions(+), 51 deletions(-) diff --git a/plugins/catalog-react/src/hooks/useEntityListProvider.tsx b/plugins/catalog-react/src/hooks/useEntityListProvider.tsx index 56af7e9a73..f424dafb56 100644 --- a/plugins/catalog-react/src/hooks/useEntityListProvider.tsx +++ b/plugins/catalog-react/src/hooks/useEntityListProvider.tsx @@ -14,7 +14,9 @@ * limitations under the License. */ +import { QueryEntitiesResponse } from '@backstage/catalog-client'; import { Entity } from '@backstage/catalog-model'; +import { useApi } from '@backstage/core-plugin-api'; import { compact, isEqual } from 'lodash'; import qs from 'qs'; import { @@ -22,6 +24,7 @@ import { PropsWithChildren, useCallback, useContext, + useEffect, useMemo, useState, } from 'react'; @@ -50,8 +53,6 @@ import { reduceCatalogFilters, reduceEntityFilters, } from '../utils/filters'; -import { useApi } from '@backstage/core-plugin-api'; -import { QueryEntitiesResponse } from '@backstage/catalog-client'; /** @public */ export type DefaultEntityFilters = { @@ -239,7 +240,7 @@ export const EntityListProvider = ( // The main async filter worker. Note that while it has a lot of dependencies // in terms of its implementation, the triggering only happens (debounced) // based on the requested filters changing. - const [{ loading, error }, refresh] = useAsyncFn( + const [{ value: resolvedValue, loading, error }, refresh] = useAsyncFn( async () => { const kindValue = requestedFilters.kind?.value?.toLocaleLowerCase('en-US'); @@ -249,19 +250,6 @@ export const EntityListProvider = ( : requestedFilters; const compacted = compact(Object.values(adjustedFilters)); - const queryParams = Object.keys(requestedFilters).reduce( - (params, key) => { - const filter = requestedFilters[key as keyof EntityFilters] as - | EntityFilter - | undefined; - if (filter?.toQueryValue) { - params[key] = filter.toQueryValue(); - } - return params; - }, - {} as Record, - ); - if (paginationMode !== 'none') { if (cursor) { if (cursor !== outputState.appliedCursor) { @@ -270,14 +258,14 @@ export const EntityListProvider = ( cursor, limit, }); - setOutputState({ + return { appliedFilters: requestedFilters, appliedCursor: cursor, backendEntities: response.items, entities: response.items.filter(entityFilter), pageInfo: response.pageInfo, totalItems: response.totalItems, - }); + }; } } else { const entityFilter = reduceEntityFilters(compacted); @@ -296,7 +284,7 @@ export const EntityListProvider = ( limit, offset, }); - setOutputState({ + return { appliedFilters: requestedFilters, backendEntities: response.items, entities: response.items.filter(entityFilter), @@ -304,7 +292,7 @@ export const EntityListProvider = ( totalItems: response.totalItems, limit, offset, - }); + }; } } } else { @@ -324,43 +312,22 @@ export const EntityListProvider = ( filter: backendFilter, }); const entities = response.items.filter(entityFilter); - setOutputState({ + return { appliedFilters: requestedFilters, backendEntities: response.items, entities, totalItems: entities.length, - }); - } else { - const entities = outputState.backendEntities.filter(entityFilter); - setOutputState({ - appliedFilters: requestedFilters, - backendEntities: outputState.backendEntities, - entities, - totalItems: entities.length, - }); + }; } + const entities = outputState.backendEntities.filter(entityFilter); + return { + appliedFilters: requestedFilters, + backendEntities: outputState.backendEntities, + entities, + totalItems: entities.length, + }; } - - if (isMounted()) { - const oldParams = qs.parse(location.search, { - ignoreQueryPrefix: true, - }); - const newParams = qs.stringify( - { - ...oldParams, - filters: queryParams, - ...(paginationMode === 'none' ? {} : { cursor, limit, offset }), - }, - { addQueryPrefix: true, arrayFormat: 'repeat' }, - ); - const newUrl = `${window.location.pathname}${newParams}`; - // We use direct history manipulation since useSearchParams and - // useNavigate in react-router-dom cause unnecessary extra rerenders. - // Also make sure to replace the state rather than pushing, since we - // don't want there to be back/forward slots for every single filter - // change. - window.history?.replaceState(null, document.title, newUrl); - } + return undefined; }, [ catalogApi, @@ -379,6 +346,55 @@ export const EntityListProvider = ( // filters will be calling this in rapid succession. useDebounce(refresh, 10, [requestedFilters, cursor, limit, offset]); + useEffect(() => { + if (resolvedValue === undefined) { + return; + } + setOutputState(resolvedValue); + if (isMounted()) { + const queryParams = Object.keys(requestedFilters).reduce( + (params, key) => { + const filter = requestedFilters[key as keyof EntityFilters] as + | EntityFilter + | undefined; + if (filter?.toQueryValue) { + params[key] = filter.toQueryValue(); + } + return params; + }, + {} as Record, + ); + + const oldParams = qs.parse(location.search, { + ignoreQueryPrefix: true, + }); + const newParams = qs.stringify( + { + ...oldParams, + filters: queryParams, + ...(paginationMode === 'none' ? {} : { cursor, limit, offset }), + }, + { addQueryPrefix: true, arrayFormat: 'repeat' }, + ); + const newUrl = `${window.location.pathname}${newParams}`; + // We use direct history manipulation since useSearchParams and + // useNavigate in react-router-dom cause unnecessary extra rerenders. + // Also make sure to replace the state rather than pushing, since we + // don't want there to be back/forward slots for every single filter + // change. + window.history?.replaceState(null, document.title, newUrl); + } + }, [ + cursor, + isMounted, + limit, + location.search, + offset, + requestedFilters, + resolvedValue, + paginationMode, + ]); + const updateFilters = useCallback( ( update: