diff --git a/.changeset/stupid-foxes-type.md b/.changeset/stupid-foxes-type.md new file mode 100644 index 0000000000..c00d58edba --- /dev/null +++ b/.changeset/stupid-foxes-type.md @@ -0,0 +1,12 @@ +--- +'@backstage/plugin-catalog-react': patch +--- + +Use the history API directly in `useEntityListProvider`. + +This replaces `useSearchParams`/`useNavigate`, since they cause at least one additional re-render compared to using this method. + +Table re-render count is down additionally: + +- Initial render of catalog page: 12 -> 9 +- Full `CatalogPage` test: 57 -> 48 diff --git a/plugins/catalog-react/src/hooks/useEntityListProvider.test.tsx b/plugins/catalog-react/src/hooks/useEntityListProvider.test.tsx index 815c9e289e..5d711016a4 100644 --- a/plugins/catalog-react/src/hooks/useEntityListProvider.test.tsx +++ b/plugins/catalog-react/src/hooks/useEntityListProvider.test.tsx @@ -16,7 +16,6 @@ import React, { PropsWithChildren } from 'react'; import qs from 'qs'; -import { MemoryRouter as Router } from 'react-router-dom'; import { act, renderHook } from '@testing-library/react-hooks'; import { MockStorageApi } from '@backstage/test-utils'; import { CatalogApi } from '@backstage/catalog-client'; @@ -86,26 +85,30 @@ const apis = ApiRegistry.from([ const wrapper = ({ userFilter, - queryParams, children, }: PropsWithChildren<{ userFilter?: UserListFilterKind; - queryParams?: string; }>) => { return ( - - - - - - + + + + ); }; describe('', () => { + const origReplaceState = window.history.replaceState; + beforeEach(() => { + window.history.replaceState = jest.fn(); + }); + afterEach(() => { + window.history.replaceState = origReplaceState; + }); + beforeEach(() => { jest.clearAllMocks(); }); @@ -137,16 +140,13 @@ describe('', () => { }); it('resolves query param filter values', async () => { + const query = qs.stringify({ + filters: { kind: 'component', type: 'service' }, + }); + delete (window as any).location; + (window as any).location = new URL(`http://localhost/catalog?${query}`); const { result, waitFor } = renderHook(() => useEntityListProvider(), { wrapper, - initialProps: { - queryParams: qs.stringify({ - filters: { - kind: 'component', - type: 'service', - }, - }), - }, }); await waitFor(() => !!result.current.queryParameters); expect(result.current.queryParameters).toEqual({ diff --git a/plugins/catalog-react/src/hooks/useEntityListProvider.tsx b/plugins/catalog-react/src/hooks/useEntityListProvider.tsx index d5c4668079..a4e5f6b2ee 100644 --- a/plugins/catalog-react/src/hooks/useEntityListProvider.tsx +++ b/plugins/catalog-react/src/hooks/useEntityListProvider.tsx @@ -25,7 +25,6 @@ import React, { useMemo, useState, } from 'react'; -import { useSearchParams } from 'react-router-dom'; import { useAsyncFn, useDebounce, useMountedState } from 'react-use'; import { catalogApiRef } from '../api'; import { @@ -105,18 +104,25 @@ export const EntityListProvider = ({ }: PropsWithChildren<{}>) => { const isMounted = useMountedState(); const catalogApi = useApi(catalogApiRef); - const [searchParams, setSearchParams] = useSearchParams(); - const allQueryParams = qs.parse(searchParams.toString()); const [requestedFilters, setRequestedFilters] = useState( {} as EntityFilters, ); - const [outputState, setOutputState] = useState>({ - appliedFilters: {} as EntityFilters, - entities: [], - backendEntities: [], - queryParameters: - (allQueryParams.filters as Record) ?? {}, - }); + const [outputState, setOutputState] = useState>( + () => { + const query = qs.parse(window.location.search, { + ignoreQueryPrefix: true, + }); + return { + appliedFilters: {} as EntityFilters, + entities: [], + backendEntities: [], + queryParameters: (query.filters ?? {}) as Record< + string, + string | string[] + >, + }; + }, + ); // The main async filter worker. Note that while it has a lot of dependencies // in terms of its implementation, the triggering only happens (debounced) @@ -167,12 +173,20 @@ export const EntityListProvider = ({ } if (isMounted()) { - setSearchParams( - qs.stringify({ ...allQueryParams, filters: queryParams }), - { - replace: true, - }, + const oldParams = qs.parse(window.location.search, { + ignoreQueryPrefix: true, + }); + const newParams = qs.stringify( + { ...oldParams, filters: queryParams }, + { addQueryPrefix: true }, ); + 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); } }, [catalogApi, requestedFilters, outputState], @@ -208,15 +222,7 @@ export const EntityListProvider = ({ loading, error, }), - [ - outputState.appliedFilters, - outputState.entities, - outputState.backendEntities, - updateFilters, - outputState.queryParameters, - loading, - error, - ], + [outputState, updateFilters, loading, error], ); return ( diff --git a/plugins/catalog/src/components/CatalogPage/CatalogPage.test.tsx b/plugins/catalog/src/components/CatalogPage/CatalogPage.test.tsx index b625451ca0..15f72b32ea 100644 --- a/plugins/catalog/src/components/CatalogPage/CatalogPage.test.tsx +++ b/plugins/catalog/src/components/CatalogPage/CatalogPage.test.tsx @@ -44,6 +44,14 @@ import { } from '@backstage/core-plugin-api'; describe('CatalogPage', () => { + const origReplaceState = window.history.replaceState; + beforeEach(() => { + window.history.replaceState = jest.fn(); + }); + afterEach(() => { + window.history.replaceState = origReplaceState; + }); + const catalogApi: Partial = { getEntities: () => Promise.resolve({