diff --git a/plugins/catalog-react/src/hooks/useEntityListProvider.tsx b/plugins/catalog-react/src/hooks/useEntityListProvider.tsx index a18575befa..a359a8f2bc 100644 --- a/plugins/catalog-react/src/hooks/useEntityListProvider.tsx +++ b/plugins/catalog-react/src/hooks/useEntityListProvider.tsx @@ -31,10 +31,10 @@ import { useContext, useEffect, useMemo, + useRef, useState, } from 'react'; import { useLocation } from 'react-router-dom'; -import useAsyncFn from 'react-use/esm/useAsyncFn'; import useDebounce from 'react-use/esm/useDebounce'; import useMountedState from 'react-use/esm/useMountedState'; import { catalogApiRef } from '../api'; @@ -141,15 +141,11 @@ export const OldEntityListContext = createContext< EntityListContextProps | undefined >(undefined); -type OutputState = { - appliedFilters: EntityFilters; - appliedCursor?: string; - entities: Entity[]; +type BackendState = { backendEntities: Entity[]; pageInfo?: QueryEntitiesResponse['pageInfo']; totalItems?: number; - offset?: number; - limit?: number; + appliedCursor?: string; }; /** @@ -236,187 +232,152 @@ export const EntityListProvider = ( const [offset, setOffset] = useState(initialOffset); const [limit, setLimit] = useState(initialLimit); - const [outputState, setOutputState] = useState>( - () => { - return { - appliedFilters: {} as EntityFilters, - entities: [], - backendEntities: [], - pageInfo: {}, - offset, - limit, - }; - }, - ); + const [backendState, setBackendState] = useState({ + backendEntities: [], + }); + const [loading, setLoading] = useState(true); + const [error, setError] = useState(); - // 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 [{ value: resolvedValue, loading, error }, refresh] = useAsyncFn( - async () => { - const kindValue = - requestedFilters.kind?.value?.toLocaleLowerCase('en-US'); - const adjustedFilters = - kindValue === 'user' || kindValue === 'group' - ? { ...requestedFilters, owners: undefined } - : requestedFilters; - const compacted = compact(Object.values(adjustedFilters)); - const entityFilter = reduceEntityFilters(compacted); + // Tracks the params of the last API call so identical requests are + // skipped even when requestedFilters changes (e.g. a label change + // or a frontend-only filter addition). + const lastFetchParamsRef = useRef(undefined); + // Generation counter — only the most recent fetch updates state, + // so out-of-order responses from overlapping requests are discarded. + const fetchGenRef = useRef(0); - if (paginationMode !== 'none') { - if (cursor) { - if (cursor !== outputState.appliedCursor) { - const response = await catalogApi.queryEntities({ - cursor, - limit, - }); - return { - appliedFilters: requestedFilters, - appliedCursor: cursor, - backendEntities: response.items, - entities: response.items.filter(entityFilter), - pageInfo: response.pageInfo, - totalItems: response.totalItems, - }; - } - const entities = outputState.backendEntities.filter(entityFilter); + const refresh = useCallback(async () => { + const kindValue = requestedFilters.kind?.value?.toLocaleLowerCase('en-US'); + const adjustedFilters = + kindValue === 'user' || kindValue === 'group' + ? { ...requestedFilters, owners: undefined } + : requestedFilters; + const compacted = compact(Object.values(adjustedFilters)); + + let fetchParams: unknown; + let doFetch: () => Promise; + + if (paginationMode !== 'none') { + if (cursor) { + fetchParams = { cursor, limit }; + doFetch = async () => { + const response = await catalogApi.queryEntities({ + cursor, + limit, + }); return { - appliedFilters: requestedFilters, - appliedCursor: outputState.appliedCursor, - backendEntities: outputState.backendEntities, - entities, - pageInfo: outputState.pageInfo, - totalItems: outputState.totalItems, - limit: outputState.limit, - offset: outputState.offset, + backendEntities: response.items, + pageInfo: response.pageInfo, + totalItems: response.totalItems, + appliedCursor: cursor, }; - } - + }; + } else { const backendFilter = reduceCatalogFilters(compacted); - const previousBackendFilter = reduceCatalogFilters( - compact(Object.values(outputState.appliedFilters)), - ); - - if ( - (paginationMode === 'offset' && - (outputState.limit !== limit || outputState.offset !== offset)) || - !isEqual(previousBackendFilter, backendFilter) - ) { + fetchParams = { ...backendFilter, limit, offset }; + doFetch = async () => { const response = await catalogApi.queryEntities({ ...backendFilter, limit, offset, }); return { - appliedFilters: requestedFilters, backendEntities: response.items, - entities: response.items.filter(entityFilter), pageInfo: response.pageInfo, totalItems: response.totalItems, - limit, - offset, }; - } - const entities = outputState.backendEntities.filter(entityFilter); - return { - appliedFilters: requestedFilters, - backendEntities: outputState.backendEntities, - entities, - pageInfo: outputState.pageInfo, - totalItems: outputState.totalItems, - limit: outputState.limit, - offset: outputState.offset, }; } - + } else { const backendFilter = reduceBackendCatalogFilters(compacted); const { orderFields } = reduceCatalogFilters(compacted); - const previousBackendFilter = reduceBackendCatalogFilters( - compact(Object.values(outputState.appliedFilters)), - ); - - // TODO(mtlewis): currently entities will never be requested unless - // there's at least one filter, we should allow an initial request - // to happen with no filters. - if (!isEqual(previousBackendFilter, backendFilter)) { - // TODO(timbonicus): should limit fields here, but would need filter - // fields + table columns + fetchParams = { filter: backendFilter, order: orderFields }; + doFetch = async () => { const response = await catalogApi.getEntities({ filter: backendFilter, order: orderFields, }); - const entities = response.items.filter(entityFilter); - return { - appliedFilters: requestedFilters, - backendEntities: response.items, - entities, - totalItems: entities.length, - }; - } - const entities = outputState.backendEntities.filter(entityFilter); - return { - appliedFilters: requestedFilters, - backendEntities: outputState.backendEntities, - entities, - totalItems: entities.length, + return { backendEntities: response.items }; }; - }, - [ - catalogApi, - queryParameters, - requestedFilters, - outputState, - cursor, - paginationMode, - limit, - offset, - ], - { loading: true }, - ); + } - // Slight debounce on the refresh, since (especially on page load) several - // filters will be calling this in rapid succession. - useDebounce(refresh, 10, [requestedFilters, cursor, limit, offset]); - - useEffect(() => { - if (resolvedValue === undefined) { + if (isEqual(fetchParams, lastFetchParamsRef.current)) { 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, - ); + lastFetchParamsRef.current = fetchParams; - const oldParams = qs.parse(location.search, { - ignoreQueryPrefix: true, - arrayLimit: 10000, - }); - 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); + const gen = ++fetchGenRef.current; + setLoading(true); + setError(undefined); + + try { + const result = await doFetch(); + if (gen === fetchGenRef.current) { + setBackendState(result); + } + } catch (e) { + if (gen === fetchGenRef.current) { + setError(e as Error); + } + } finally { + if (gen === fetchGenRef.current) { + setLoading(false); + } } + }, [catalogApi, requestedFilters, cursor, paginationMode, limit, offset]); + + // Slight debounce on the refresh, since (especially on page load) + // several filters will be calling updateFilters in rapid succession. + useDebounce(refresh, 10, [requestedFilters, cursor, limit, offset]); + + // Frontend filtering — synchronous, no debounce needed. Updates + // instantly when requestedFilters or backendEntities change. + const adjustedFilters = useMemo(() => { + const kindValue = requestedFilters.kind?.value?.toLocaleLowerCase('en-US'); + return kindValue === 'user' || kindValue === 'group' + ? { ...requestedFilters, owners: undefined } + : requestedFilters; + }, [requestedFilters]); + + const entities = useMemo(() => { + const compacted = compact(Object.values(adjustedFilters)); + const entityFilter = reduceEntityFilters(compacted); + return backendState.backendEntities.filter(entityFilter); + }, [adjustedFilters, backendState.backendEntities]); + + // Sync filter state to URL query parameters. 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. + useEffect(() => { + if (!isMounted()) { + return; + } + 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, + arrayLimit: 10000, + }); + const newParams = qs.stringify( + { + ...oldParams, + filters: queryParams, + ...(paginationMode === 'none' ? {} : { cursor, limit, offset }), + }, + { addQueryPrefix: true, arrayFormat: 'repeat' }, + ); + const newUrl = `${window.location.pathname}${newParams}`; + window.history?.replaceState(null, document.title, newUrl); }, [ cursor, isMounted, @@ -424,7 +385,6 @@ export const EntityListProvider = ( location.search, offset, requestedFilters, - resolvedValue, paginationMode, ]); @@ -454,36 +414,31 @@ export const EntityListProvider = ( [paginationMode], ); - // Use resolvedValue directly when available to avoid an extra render cycle. - // Without this, there's a render where loading has flipped back to false but - // outputState hasn't been updated yet (it syncs via useEffect), causing a - // flash of stale data between the loading state and the new results. - const latestOutput = resolvedValue ?? outputState; - const pageInfo = useMemo(() => { if (paginationMode !== 'cursor') { return undefined; } - const prevCursor = latestOutput.pageInfo?.prevCursor; - const nextCursor = latestOutput.pageInfo?.nextCursor; + const prevCursor = backendState.pageInfo?.prevCursor; + const nextCursor = backendState.pageInfo?.nextCursor; return { prev: prevCursor ? () => setCursor(prevCursor) : undefined, next: nextCursor ? () => setCursor(nextCursor) : undefined, }; - }, [paginationMode, latestOutput.pageInfo]); + }, [paginationMode, backendState.pageInfo]); const value = useMemo( () => ({ - filters: latestOutput.appliedFilters, - entities: latestOutput.entities, - backendEntities: latestOutput.backendEntities, + filters: requestedFilters, + entities, + backendEntities: backendState.backendEntities, updateFilters, queryParameters, loading, error, pageInfo, - totalItems: latestOutput.totalItems, + totalItems: + paginationMode === 'none' ? entities.length : backendState.totalItems, limit, offset, setLimit, @@ -491,15 +446,17 @@ export const EntityListProvider = ( paginationMode, }), [ - latestOutput, + requestedFilters, + entities, + backendState, updateFilters, queryParameters, loading, error, pageInfo, + paginationMode, limit, offset, - paginationMode, setLimit, setOffset, ],