From d46acd694893c2291ebbe932efc1e0afcda69a04 Mon Sep 17 00:00:00 2001 From: Vincenzo Scamporlino Date: Fri, 26 May 2023 23:29:13 +0200 Subject: [PATCH] catalog-react: improve memo readability Signed-off-by: Vincenzo Scamporlino --- .../UserListPicker/useAllEntitiesCount.ts | 26 ++++++--------- .../UserListPicker/useOwnedEntitiesCount.ts | 32 ++++++++----------- .../UserListPicker/useStarredEntitiesCount.ts | 18 +++++------ 3 files changed, 32 insertions(+), 44 deletions(-) diff --git a/plugins/catalog-react/src/components/UserListPicker/useAllEntitiesCount.ts b/plugins/catalog-react/src/components/UserListPicker/useAllEntitiesCount.ts index 499762ff87..13217c6800 100644 --- a/plugins/catalog-react/src/components/UserListPicker/useAllEntitiesCount.ts +++ b/plugins/catalog-react/src/components/UserListPicker/useAllEntitiesCount.ts @@ -22,40 +22,32 @@ import { catalogApiRef } from '../../api'; import { useEntityList } from '../../hooks'; import { reduceCatalogFilters } from '../../utils'; -/** - * TODO(vinzscam): we need to find a better way - * for retrieving this value. One possible way, could be to use - * the /entities endpoint: since this method is paginated, - * it should also return how many items matching the provided filters - * are in the catalog - */ export function useAllEntitiesCount() { const catalogApi = useApi(catalogApiRef); const { filters } = useEntityList(); - const refRequest = useRef(); - useMemo(() => { + const prevRequest = useRef(); + const request = useMemo(() => { const { user, ...allFilters } = filters; const compacted = compact(Object.values(allFilters)); const filter = reduceCatalogFilters(compacted); - const request: QueryEntitiesInitialRequest = { + const newRequest: QueryEntitiesInitialRequest = { filter, limit: 0, }; - if (isEqual(request, refRequest.current)) { - return refRequest.current; + if (isEqual(newRequest, prevRequest.current)) { + return prevRequest.current; } - refRequest.current = request; - - return request; + prevRequest.current = newRequest; + return newRequest; }, [filters]); const { value: count, loading } = useAsync(async () => { - const { totalItems } = await catalogApi.queryEntities(refRequest.current); + const { totalItems } = await catalogApi.queryEntities(request); return totalItems; - }, [refRequest.current]); + }, [request]); return { count, loading }; } diff --git a/plugins/catalog-react/src/components/UserListPicker/useOwnedEntitiesCount.ts b/plugins/catalog-react/src/components/UserListPicker/useOwnedEntitiesCount.ts index f7fb9d4516..f2ff985c09 100644 --- a/plugins/catalog-react/src/components/UserListPicker/useOwnedEntitiesCount.ts +++ b/plugins/catalog-react/src/components/UserListPicker/useOwnedEntitiesCount.ts @@ -32,13 +32,12 @@ export function useOwnedEntitiesCount() { // Trigger load only on mount const { value: ownershipEntityRefs, loading: loadingEntityRefs } = useAsync( async () => (await identityApi.getBackstageIdentity()).ownershipEntityRefs, - [], ); - const refRequest = useRef(); + const prevRequest = useRef(); - useMemo(async () => { + const request = useMemo(() => { const compacted = compact(Object.values(filters)); const allFilter = reduceCatalogFilters(compacted); const { ['metadata.name']: metadata, ...filter } = allFilter; @@ -54,15 +53,12 @@ export function useOwnedEntitiesCount() { const ownedBy = ownedByFilter.length > 0 ? ownedByFilter : ownershipEntityRefs; if (ownedByFilter.length > 0 && commonOwnedBy.length === 0) { - // don't send any request if another filter sets - // totally different values for relations.ownedBy filter. - // TODO(vinzscam): check conflicts between UserOwnersFilter and EntityOwnerFilter. - // both set filters on the same relations.ownedBy key, so the conflicts need - // to be addressed properly. - refRequest.current = undefined; - return null; + // detect whether another filter sets values that will produce + // empty results in order to avoid sending an additional request. + prevRequest.current = undefined; + return undefined; } - const request: QueryEntitiesInitialRequest = { + const newRequest: QueryEntitiesInitialRequest = { filter: { ...filter, 'relations.ownedBy': ownedBy ?? [], @@ -70,13 +66,13 @@ export function useOwnedEntitiesCount() { limit: 0, }; - if (isEqual(request, refRequest.current)) { - return refRequest.current; + if (isEqual(newRequest, prevRequest.current)) { + return prevRequest.current; } - refRequest.current = request; + prevRequest.current = newRequest; - return request; + return newRequest; }, [filters, ownershipEntityRefs]); const { value: count, loading: loadingEntityOwnership } = @@ -84,13 +80,13 @@ export function useOwnedEntitiesCount() { if (!ownershipEntityRefs?.length) { return 0; } - if (!refRequest.current) { + if (!request) { return 0; } - const { totalItems } = await catalogApi.queryEntities(refRequest.current); + const { totalItems } = await catalogApi.queryEntities(request); return totalItems; - }, [refRequest.current]); + }, [request]); const loading = loadingEntityRefs || loadingEntityOwnership; const filter = useMemo( diff --git a/plugins/catalog-react/src/components/UserListPicker/useStarredEntitiesCount.ts b/plugins/catalog-react/src/components/UserListPicker/useStarredEntitiesCount.ts index aff5066a74..93adad0489 100644 --- a/plugins/catalog-react/src/components/UserListPicker/useStarredEntitiesCount.ts +++ b/plugins/catalog-react/src/components/UserListPicker/useStarredEntitiesCount.ts @@ -30,27 +30,27 @@ export function useStarredEntitiesCount() { const { filters } = useEntityList(); const { starredEntities } = useStarredEntities(); - const refRequest = useRef(); - useMemo(async () => { + const prevRequest = useRef(); + const request = useMemo(() => { const { user, ...allFilters } = filters; const compacted = compact(Object.values(allFilters)); const filter = reduceCatalogFilters(compacted); const facet = 'metadata.name'; - const request: QueryEntitiesInitialRequest = { + const newRequest: QueryEntitiesInitialRequest = { filter: { ...filter, [facet]: Array.from(starredEntities).map(e => parseEntityRef(e).name), }, limit: 1000, }; - if (isEqual(request, refRequest.current)) { - return refRequest.current; + if (isEqual(newRequest, prevRequest.current)) { + return prevRequest.current; } - refRequest.current = request; + prevRequest.current = newRequest; - return request; + return newRequest; }, [filters, starredEntities]); const { value: count, loading } = useAsync(async () => { @@ -58,7 +58,7 @@ export function useStarredEntitiesCount() { return 0; } - const response = await catalogApi.queryEntities(refRequest.current); + const response = await catalogApi.queryEntities(request); return response.items .map(e => @@ -69,7 +69,7 @@ export function useStarredEntitiesCount() { }), ) .filter(e => starredEntities.has(e)).length; - }, [refRequest.current, starredEntities]); + }, [request, starredEntities]); const filter = useMemo( () => EntityUserListFilter.starred(Array.from(starredEntities)),