diff --git a/.changeset/slimy-kids-attack.md b/.changeset/slimy-kids-attack.md
new file mode 100644
index 0000000000..ddb7735571
--- /dev/null
+++ b/.changeset/slimy-kids-attack.md
@@ -0,0 +1,5 @@
+---
+'@backstage/plugin-catalog-react': patch
+---
+
+Tweak the `EntityListProvider` to do single-cycle updates
diff --git a/plugins/catalog-react/src/hooks/useEntityListProvider.test.tsx b/plugins/catalog-react/src/hooks/useEntityListProvider.test.tsx
index fcb599888e..e2dfbd91da 100644
--- a/plugins/catalog-react/src/hooks/useEntityListProvider.test.tsx
+++ b/plugins/catalog-react/src/hooks/useEntityListProvider.test.tsx
@@ -157,6 +157,7 @@ describe('', () => {
user: new UserListFilter('owned', mockUser, () => true),
}),
);
+ await waitFor(() => result.current.entities.length !== 2);
expect(mockCatalogApi.getEntities).toHaveBeenCalledTimes(1);
expect(result.current.entities.length).toBe(1);
});
diff --git a/plugins/catalog-react/src/hooks/useEntityListProvider.tsx b/plugins/catalog-react/src/hooks/useEntityListProvider.tsx
index 159c898b74..a410c243a1 100644
--- a/plugins/catalog-react/src/hooks/useEntityListProvider.tsx
+++ b/plugins/catalog-react/src/hooks/useEntityListProvider.tsx
@@ -14,18 +14,17 @@
* limitations under the License.
*/
+import { Entity } from '@backstage/catalog-model';
+import { useApi } from '@backstage/core';
+import { compact, isEqual } from 'lodash';
import React, {
createContext,
PropsWithChildren,
useCallback,
useContext,
- useEffect,
useState,
} from 'react';
import { useAsyncFn, useDebounce } from 'react-use';
-import { useApi } from '@backstage/core';
-import { Entity } from '@backstage/catalog-model';
-import { reduceCatalogFilters, reduceEntityFilters } from '../utils';
import { catalogApiRef } from '../api';
import {
EntityFilter,
@@ -34,7 +33,7 @@ import {
EntityTypeFilter,
UserListFilter,
} from '../types';
-import { compact, isEqual } from 'lodash';
+import { reduceCatalogFilters, reduceEntityFilters } from '../utils';
export type DefaultEntityFilters = {
kind?: EntityKindFilter;
@@ -80,51 +79,57 @@ export const EntityListContext = createContext<
EntityListContextProps | undefined
>(undefined);
+type OutputState = {
+ appliedFilters: EntityFilters;
+ entities: Entity[];
+ backendEntities: Entity[];
+};
+
export const EntityListProvider = ({
children,
}: PropsWithChildren<{}>) => {
const catalogApi = useApi(catalogApiRef);
+ const [requestedFilters, setRequestedFilters] = useState(
+ {} as EntityFilters,
+ );
+ const [outputState, setOutputState] = useState>({
+ appliedFilters: {} as EntityFilters,
+ entities: [],
+ backendEntities: [],
+ });
- const [filters, setFilters] = useState({} as EntityFilters);
- const [entities, setEntities] = useState([]);
- const [backendEntities, setBackendEntities] = useState([]);
-
- // Store resolved catalog-backend filters and deep compare on filter updates, to avoid refetching
- // when only frontend filters change
- const [backendFilters, setBackendFilters] = useState<
- Record
- >(reduceCatalogFilters(compact(Object.values(filters))));
-
- useEffect(() => {
- const newBackendFilters = reduceCatalogFilters(
- compact(Object.values(filters)),
- );
- if (!isEqual(newBackendFilters, backendFilters)) {
- setBackendFilters(newBackendFilters);
- }
- }, [backendFilters, filters]);
-
+ // 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(async () => {
- // TODO(timbonicus): should limit fields here, but would need filter fields + table columns
- const items = await catalogApi
- .getEntities({
- filter: backendFilters,
- })
- .then(response => response.items);
- setBackendEntities(items);
- }, [backendFilters, catalogApi]);
-
- // Slight debounce on the catalog-backend call, to prevent eager refresh on multiple programmatic
- // filter changes.
- useDebounce(refresh, 10, [backendFilters]);
-
- // Apply frontend filters
- useEffect(() => {
- const resolvedEntities = (backendEntities ?? []).filter(
- reduceEntityFilters(compact(Object.values(filters))),
+ const compacted = compact(Object.values(requestedFilters));
+ const entityFilter = reduceEntityFilters(compacted);
+ const backendFilter = reduceCatalogFilters(compacted);
+ const previousBackendFilter = reduceCatalogFilters(
+ compact(Object.values(outputState.appliedFilters)),
);
- setEntities(resolvedEntities);
- }, [backendEntities, filters]);
+
+ if (!isEqual(previousBackendFilter, backendFilter)) {
+ // TODO(timbonicus): should limit fields here, but would need filter
+ // fields + table columns
+ const response = await catalogApi.getEntities({ filter: backendFilter });
+ setOutputState({
+ appliedFilters: requestedFilters,
+ backendEntities: response.items,
+ entities: response.items.filter(entityFilter),
+ });
+ } else {
+ setOutputState({
+ appliedFilters: requestedFilters,
+ backendEntities: outputState.backendEntities,
+ entities: outputState.backendEntities.filter(entityFilter),
+ });
+ }
+ }, [catalogApi, requestedFilters, outputState]);
+
+ // Slight debounce on the refresh, since (especially on page load) several
+ // filters will be calling this in rapid succession.
+ useDebounce(refresh, 10, [requestedFilters]);
const updateFilters = useCallback(
(
@@ -132,11 +137,11 @@ export const EntityListProvider = ({
| Partial
| ((prevFilters: EntityFilters) => Partial),
) => {
- if (typeof update === 'function') {
- setFilters(prevFilters => ({ ...prevFilters, ...update(prevFilters) }));
- } else {
- setFilters(prevFilters => ({ ...prevFilters, ...update }));
- }
+ setRequestedFilters(prevFilters => {
+ const newFilters =
+ typeof update === 'function' ? update(prevFilters) : update;
+ return { ...prevFilters, ...newFilters };
+ });
},
[],
);
@@ -144,9 +149,9 @@ export const EntityListProvider = ({
return (
{
// related to some theme issues in mui-table
// https://github.com/mbrn/material-table/issues/1293
it('should render', async () => {
- const { getByText, getByTestId } = await renderWrapped();
- expect(getByText(/Owned \(1\)/)).toBeInTheDocument();
+ const { findByText, getByTestId } = await renderWrapped();
+ await expect(findByText(/Owned \(1\)/)).resolves.toBeInTheDocument();
fireEvent.click(getByTestId('user-picker-all'));
- expect(getByText(/All \(2\)/)).toBeInTheDocument();
+ await expect(findByText(/All \(2\)/)).resolves.toBeInTheDocument();
});
+
it('should set initial filter correctly', async () => {
- const { getByText } = await renderWrapped(
+ const { findByText } = await renderWrapped(
,
);
- expect(getByText(/All \(2\)/)).toBeInTheDocument();
+ await expect(findByText(/All \(2\)/)).resolves.toBeInTheDocument();
});
- // this test is for fixing the bug after favoriting an entity, the matching entities defaulting
- // to "owned" filter and not based on the selected filter
- it('should render the correct entities filtered on the selectedfilter', async () => {
- const { getByText, findAllByTitle, getByTestId } = await renderWrapped(
- ,
- );
- expect(getByText(/Owned \(1\)/)).toBeInTheDocument();
- expect(getByText(/Starred/)).toBeInTheDocument();
- fireEvent.click(getByTestId('user-picker-starred'));
- expect(getByText(/Starred \(0\)/)).toBeInTheDocument();
- fireEvent.click(getByTestId('user-picker-all'));
- expect(getByText(/All \(2\)/)).toBeInTheDocument();
- const starredIcons = await findAllByTitle('Add to favorites');
+ // this test is for fixing the bug after favoriting an entity, the matching
+ // entities defaulting to "owned" filter and not based on the selected filter
+ it('should render the correct entities filtered on the selected filter', async () => {
+ await renderWrapped();
+ await expect(screen.findByText(/Owned \(1\)/)).resolves.toBeInTheDocument();
+ fireEvent.click(screen.getByTestId('user-picker-starred'));
+ await expect(
+ screen.findByText(/Starred \(0\)/),
+ ).resolves.toBeInTheDocument();
+ fireEvent.click(screen.getByTestId('user-picker-all'));
+ await expect(screen.findByText(/All \(2\)/)).resolves.toBeInTheDocument();
+
+ const starredIcons = await screen.findAllByTitle('Add to favorites');
fireEvent.click(starredIcons[0]);
- expect(getByText(/All \(2\)/)).toBeInTheDocument();
+ await expect(screen.findByText(/All \(2\)/)).resolves.toBeInTheDocument();
- fireEvent.click(getByTestId('user-picker-starred'));
- waitFor(() => expect(getByText(/Starred \(1\)/)).toBeInTheDocument());
+ fireEvent.click(screen.getByTestId('user-picker-starred'));
+ await expect(
+ screen.findByText(/Starred \(1\)/),
+ ).resolves.toBeInTheDocument();
});
});