diff --git a/.changeset/quiet-spies-clean.md b/.changeset/quiet-spies-clean.md new file mode 100644 index 0000000000..ba277f4021 --- /dev/null +++ b/.changeset/quiet-spies-clean.md @@ -0,0 +1,5 @@ +--- +'@backstage/plugin-catalog-graph': patch +--- + +Fixed a bug in the `CatalogGraphPage` component where, after clicking on some nodes, clicking the back button would break the navigation. This issue caused the entire navigation to fail and behaved differently across various browsers. diff --git a/plugins/catalog-graph/src/components/CatalogGraphPage/useCatalogGraphPage.test.ts b/plugins/catalog-graph/src/components/CatalogGraphPage/useCatalogGraphPage.test.ts deleted file mode 100644 index a89fa353d6..0000000000 --- a/plugins/catalog-graph/src/components/CatalogGraphPage/useCatalogGraphPage.test.ts +++ /dev/null @@ -1,147 +0,0 @@ -/* - * Copyright 2021 The Backstage Authors - * - * Licensed under the Apache License, Version 2.0 (the "License"); - * you may not use this file except in compliance with the License. - * You may obtain a copy of the License at - * - * http://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, software - * distributed under the License is distributed on an "AS IS" BASIS, - * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. - * See the License for the specific language governing permissions and - * limitations under the License. - */ -import { RELATION_MEMBER_OF } from '@backstage/catalog-model'; -import { act, renderHook } from '@testing-library/react'; -import { useLocation as useLocationMocked } from 'react-router-dom'; -import { Direction } from '../EntityRelationsGraph'; -import { useCatalogGraphPage } from './useCatalogGraphPage'; - -jest.mock('react-router-dom', () => ({ - useLocation: jest.fn(), -})); - -jest.spyOn(window.history, 'replaceState'); -jest.spyOn(window.history, 'pushState'); - -const useLocation = useLocationMocked as jest.Mock< - ReturnType ->; -const windowHistoryReplaceState = window.history.replaceState as jest.Mock< - ReturnType ->; -const windowHistoryPushState = window.history.pushState as jest.Mock< - ReturnType ->; - -describe('useCatalogGraphPage', () => { - beforeEach(() => { - useLocation.mockReturnValue({ - search: '?', - state: {}, - key: '', - pathname: '', - hash: '', - }); - }); - - afterEach(() => jest.resetAllMocks()); - - test('should use initial state', () => { - const { result } = renderHook(() => - useCatalogGraphPage({ - initialState: { - rootEntityRefs: ['b:d/c'], - maxDepth: 2, - direction: Direction.RIGHT_LEFT, - mergeRelations: false, - unidirectional: false, - showFilters: false, - selectedKinds: ['API'], - selectedRelations: [RELATION_MEMBER_OF], - }, - }), - ); - - expect(result.current.rootEntityNames).toEqual([ - { kind: 'b', namespace: 'd', name: 'c' }, - ]); - expect(result.current.maxDepth).toEqual(2); - expect(result.current.direction).toEqual(Direction.RIGHT_LEFT); - expect(result.current.mergeRelations).toEqual(false); - expect(result.current.unidirectional).toEqual(false); - expect(result.current.showFilters).toEqual(false); - expect(result.current.selectedKinds).toEqual(['api']); - expect(result.current.selectedRelations).toEqual([RELATION_MEMBER_OF]); - }); - - test('should use state from url', () => { - useLocation.mockReturnValueOnce({ - search: - '?rootEntityRefs[]=b:d/c&maxDepth=2&direction=RL&mergeRelations=false&unidirectional=false&showFilters=false&selectedKinds[]=api&selectedRelations[]=memberOf', - state: {}, - key: '', - pathname: '', - hash: '', - }); - - const { result } = renderHook(() => useCatalogGraphPage({})); - - expect(result.current.rootEntityNames).toEqual([ - { kind: 'b', namespace: 'd', name: 'c' }, - ]); - expect(result.current.maxDepth).toEqual(2); - expect(result.current.direction).toEqual(Direction.RIGHT_LEFT); - expect(result.current.mergeRelations).toEqual(false); - expect(result.current.unidirectional).toEqual(false); - expect(result.current.showFilters).toEqual(false); - expect(result.current.selectedKinds).toEqual(['api']); - expect(result.current.selectedRelations).toEqual([RELATION_MEMBER_OF]); - }); - - test('should update state in url (replace if setting changes)', () => { - const { result } = renderHook(() => useCatalogGraphPage({})); - - act(() => result.current.setMaxDepth(5)); - - expect(windowHistoryReplaceState).toHaveBeenCalledWith( - null, - '', - '/?maxDepth=5&unidirectional=true&mergeRelations=true&direction=LR&showFilters=true', - ); - - act(() => result.current.setUnidirectional(false)); - - expect(windowHistoryReplaceState).toHaveBeenCalledWith( - null, - '', - '/?maxDepth=5&unidirectional=false&mergeRelations=true&direction=LR&showFilters=true', - ); - }); - - test('should update state in url (only push if different root entity)', () => { - const { result, rerender } = renderHook(() => - useCatalogGraphPage({ - initialState: { - rootEntityRefs: ['component:default/first'], - }, - }), - ); - - act(() => - result.current.setRootEntityNames([ - { kind: 'Component', namespace: 'default', name: 'my' }, - ]), - ); - - rerender(); - - expect(windowHistoryPushState).toHaveBeenCalledWith( - null, - '', - '/?rootEntityRefs%5B%5D=component%3Adefault%2Fmy&maxDepth=%E2%88%9E&unidirectional=true&mergeRelations=true&direction=LR&showFilters=true', - ); - }); -}); diff --git a/plugins/catalog-graph/src/components/CatalogGraphPage/useCatalogGraphPage.test.tsx b/plugins/catalog-graph/src/components/CatalogGraphPage/useCatalogGraphPage.test.tsx new file mode 100644 index 0000000000..85cb606482 --- /dev/null +++ b/plugins/catalog-graph/src/components/CatalogGraphPage/useCatalogGraphPage.test.tsx @@ -0,0 +1,182 @@ +/* + * Copyright 2021 The Backstage Authors + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +import { RELATION_MEMBER_OF } from '@backstage/catalog-model'; +import { renderHook, waitFor } from '@testing-library/react'; +import React from 'react'; +import { act } from 'react-dom/test-utils'; +import { BrowserRouter } from 'react-router-dom'; +import { Direction } from '../EntityRelationsGraph'; +import { useCatalogGraphPage } from './useCatalogGraphPage'; + +const wrapper = ({ children }: { children?: React.ReactNode }) => { + return {children}; +}; + +describe('useCatalogGraphPage', () => { + test('should use initial state', () => { + const { result } = renderHook(props => useCatalogGraphPage(props), { + initialProps: { + initialState: { + rootEntityRefs: ['b:d/c'], + maxDepth: 2, + direction: Direction.RIGHT_LEFT, + mergeRelations: false, + unidirectional: false, + showFilters: false, + selectedKinds: ['API'], + selectedRelations: [RELATION_MEMBER_OF], + }, + }, + wrapper, + }); + + expect(result.current.rootEntityNames).toEqual([ + { kind: 'b', namespace: 'd', name: 'c' }, + ]); + expect(result.current.maxDepth).toEqual(2); + expect(result.current.direction).toEqual(Direction.RIGHT_LEFT); + expect(result.current.mergeRelations).toEqual(false); + expect(result.current.unidirectional).toEqual(false); + expect(result.current.showFilters).toEqual(false); + expect(result.current.selectedKinds).toEqual(['api']); + expect(result.current.selectedRelations).toEqual([RELATION_MEMBER_OF]); + }); + + test('should use state from url', () => { + act(() => { + history.pushState( + {}, + '', + '?rootEntityRefs[]=b:d/c&maxDepth=2&direction=RL&mergeRelations=false&unidirectional=false&showFilters=false&selectedKinds[]=api&selectedRelations[]=memberOf', + ); + }); + + const { result } = renderHook(props => useCatalogGraphPage(props), { + initialProps: {}, + wrapper, + }); + + expect(result.current.rootEntityNames).toEqual([ + { kind: 'b', namespace: 'd', name: 'c' }, + ]); + expect(result.current.maxDepth).toEqual(2); + expect(result.current.direction).toEqual(Direction.RIGHT_LEFT); + expect(result.current.mergeRelations).toEqual(false); + expect(result.current.unidirectional).toEqual(false); + expect(result.current.showFilters).toEqual(false); + expect(result.current.selectedKinds).toEqual(['api']); + expect(result.current.selectedRelations).toEqual([RELATION_MEMBER_OF]); + }); + + test('should update state in url (replace if setting changes)', () => { + const { result } = renderHook(props => useCatalogGraphPage(props), { + wrapper, + initialProps: {}, + }); + + act(() => result.current.setMaxDepth(5)); + + expect(window.location.search).toEqual( + '?rootEntityRefs%5B%5D=b%3Ad%2Fc&maxDepth=5&selectedKinds%5B%5D=api&selectedRelations%5B%5D=memberOf&unidirectional=false&mergeRelations=false&direction=RL&showFilters=false&curve=curveMonotoneX', + ); + + act(() => result.current.setUnidirectional(false)); + + expect(window.location.search).toEqual( + '?rootEntityRefs%5B%5D=b%3Ad%2Fc&maxDepth=5&selectedKinds%5B%5D=api&selectedRelations%5B%5D=memberOf&unidirectional=false&mergeRelations=false&direction=RL&showFilters=false&curve=curveMonotoneX', + ); + }); + + test('should update state in url (only push if different root entity)', () => { + const oldLength = window.history.length; + const { result } = renderHook(props => useCatalogGraphPage(props), { + initialProps: { + initialState: { + rootEntityRefs: ['component:default/first'], + }, + }, + wrapper, + }); + + act(() => + result.current.setRootEntityNames([ + { kind: 'component', namespace: 'default', name: 'my' }, + ]), + ); + + expect(window.history.length).toEqual(oldLength + 1); + expect(window.location.search).toEqual( + '?rootEntityRefs%5B%5D=component%3Adefault%2Fmy&maxDepth=5&selectedKinds%5B%5D=api&selectedRelations%5B%5D=memberOf&unidirectional=false&mergeRelations=false&direction=RL&showFilters=false&curve=curveMonotoneX', + ); + }); + + test('should update state to last state on back', async () => { + const { result } = renderHook(props => useCatalogGraphPage(props), { + wrapper, + initialProps: { + initialState: { + rootEntityRefs: ['component:default/first'], + }, + }, + }); + + act(() => + result.current.setRootEntityNames([ + { kind: 'component', namespace: 'default', name: 'first' }, + ]), + ); + + expect(window.location.search).toEqual( + '?rootEntityRefs%5B%5D=component%3Adefault%2Ffirst&maxDepth=5&selectedKinds%5B%5D=api&selectedRelations%5B%5D=memberOf&unidirectional=false&mergeRelations=false&direction=RL&showFilters=false&curve=curveMonotoneX', + ); + + act(() => + result.current.setRootEntityNames([ + { kind: 'component', namespace: 'default', name: 'second' }, + ]), + ); + + expect(window.location.search).toEqual( + '?rootEntityRefs%5B%5D=component%3Adefault%2Fsecond&maxDepth=5&selectedKinds%5B%5D=api&selectedRelations%5B%5D=memberOf&unidirectional=false&mergeRelations=false&direction=RL&showFilters=false&curve=curveMonotoneX', + ); + + act(() => { + result.current.setRootEntityNames([ + { kind: 'component', namespace: 'default', name: 'third' }, + ]); + }); + + act(() => { + window.history.back(); + }); + + await waitFor(() => { + expect(result.current.rootEntityNames).toEqual([ + { kind: 'component', namespace: 'default', name: 'second' }, + ]); + }); + + act(() => { + window.history.back(); + }); + + await waitFor(() => { + expect(result.current.rootEntityNames).toEqual([ + { kind: 'component', namespace: 'default', name: 'first' }, + ]); + }); + }); +}); diff --git a/plugins/catalog-graph/src/components/CatalogGraphPage/useCatalogGraphPage.ts b/plugins/catalog-graph/src/components/CatalogGraphPage/useCatalogGraphPage.ts index 6e47dedc50..1f6fb74997 100644 --- a/plugins/catalog-graph/src/components/CatalogGraphPage/useCatalogGraphPage.ts +++ b/plugins/catalog-graph/src/components/CatalogGraphPage/useCatalogGraphPage.ts @@ -27,13 +27,12 @@ import { useMemo, useState, } from 'react'; -import { useLocation } from 'react-router-dom'; -import usePrevious from 'react-use/esm/usePrevious'; +import { useLocation, useNavigate } from 'react-router-dom'; import { Direction } from '../EntityRelationsGraph'; export type CatalogGraphPageValue = { rootEntityNames: CompoundEntityRef[]; - setRootEntityNames: Dispatch>; + setRootEntityNames: (value: CompoundEntityRef[]) => void; maxDepth: number; setMaxDepth: Dispatch>; selectedRelations: string[] | undefined; @@ -70,6 +69,8 @@ export function useCatalogGraphPage({ }; }): CatalogGraphPageValue { const location = useLocation(); + const navigate = useNavigate(); + const query = useMemo( () => (qs.parse(location.search, { arrayLimit: 0, ignoreQueryPrefix: true }) || @@ -87,19 +88,46 @@ export function useCatalogGraphPage({ [location.search], ); - // Initial state - const [rootEntityNames, setRootEntityNames] = useState( + const rootEntityNames = useMemo( () => (Array.isArray(query.rootEntityRefs) ? query.rootEntityRefs : initialState?.rootEntityRefs ?? [] ).map(r => parseEntityRef(r)), + [initialState?.rootEntityRefs, query.rootEntityRefs], ); + + const setRootEntityNames = useCallback( + (value: CompoundEntityRef[]) => { + const areSame = + rootEntityNames.length === value.length && + rootEntityNames.every( + (r, i) => stringifyEntityRef(r) === stringifyEntityRef(value[i]), + ); + + if (areSame) { + return; + } + + const newSearch = qs.stringify( + { + ...query, + rootEntityRefs: value.map(r => stringifyEntityRef(r)), + }, + { arrayFormat: 'brackets', addQueryPrefix: true }, + ); + + navigate(newSearch); + }, + [rootEntityNames, navigate, query], + ); + const [maxDepth, setMaxDepth] = useState(() => typeof query.maxDepth === 'string' ? parseMaxDepth(query.maxDepth) : initialState?.maxDepth ?? Number.POSITIVE_INFINITY, ); + const [selectedRelations, setSelectedRelations] = useState< string[] | undefined >(() => @@ -107,105 +135,53 @@ export function useCatalogGraphPage({ ? query.selectedRelations : initialState?.selectedRelations, ); + const [selectedKinds, setSelectedKinds] = useState(() => (Array.isArray(query.selectedKinds) ? query.selectedKinds : initialState?.selectedKinds )?.map(k => k.toLocaleLowerCase('en-US')), ); + const [unidirectional, setUnidirectional] = useState(() => typeof query.unidirectional === 'string' ? query.unidirectional === 'true' : initialState?.unidirectional ?? true, ); + const [mergeRelations, setMergeRelations] = useState(() => typeof query.mergeRelations === 'string' ? query.mergeRelations === 'true' : initialState?.mergeRelations ?? true, ); + const [direction, setDirection] = useState(() => typeof query.direction === 'string' ? query.direction : initialState?.direction ?? Direction.LEFT_RIGHT, ); + const [curve, setCurve] = useState<'curveStepBefore' | 'curveMonotoneX'>(() => typeof query.curve === 'string' ? query.curve : initialState?.curve ?? 'curveMonotoneX', ); + const [showFilters, setShowFilters] = useState(() => typeof query.showFilters === 'string' ? query.showFilters === 'true' : initialState?.showFilters ?? true, ); + const toggleShowFilters = useCallback( () => setShowFilters(s => !s), [setShowFilters], ); - // Update from query parameters - const prevQueryParams = usePrevious(location.search); useEffect(() => { - // Only respond to changes to url query params - if (location.search === prevQueryParams) { - return; - } - - if (Array.isArray(query.rootEntityRefs)) { - setRootEntityNames(query.rootEntityRefs.map(r => parseEntityRef(r))); - } - - if (typeof query.maxDepth === 'string') { - setMaxDepth(parseMaxDepth(query.maxDepth)); - } - - if (Array.isArray(query.selectedKinds)) { - setSelectedKinds(query.selectedKinds); - } - - if (Array.isArray(query.selectedRelations)) { - setSelectedRelations(query.selectedRelations); - } - - if (typeof query.unidirectional === 'string') { - setUnidirectional(query.unidirectional === 'true'); - } - - if (typeof query.mergeRelations === 'string') { - setMergeRelations(query.mergeRelations === 'true'); - } - - if (typeof query.direction === 'string') { - setDirection(query.direction); - } - - if (typeof query.showFilters === 'string') { - setShowFilters(query.showFilters === 'true'); - } - }, [ - prevQueryParams, - location.search, - query, - setRootEntityNames, - setMaxDepth, - setSelectedKinds, - setSelectedRelations, - setUnidirectional, - setMergeRelations, - setDirection, - setShowFilters, - ]); - - // Update query parameters - const previousRootEntityRefs = usePrevious( - rootEntityNames.map(e => stringifyEntityRef(e)), - ); - - useEffect(() => { - const rootEntityRefs = rootEntityNames.map(e => stringifyEntityRef(e)); const newParams = qs.stringify( { - rootEntityRefs, + rootEntityRefs: rootEntityNames.map(stringifyEntityRef), maxDepth: isFinite(maxDepth) ? maxDepth : '∞', selectedKinds, selectedRelations, @@ -213,36 +189,23 @@ export function useCatalogGraphPage({ mergeRelations, direction, showFilters, + curve, }, { arrayFormat: 'brackets', addQueryPrefix: true }, ); - const newUrl = `${window.location.pathname}${newParams}`; - // We directly manipulate window history here in order to not re-render - // infinitely (state => location => state => etc). The intention of this - // code is just to ensure the right query/filters are loaded when a user - // clicks the "back" button after clicking a result. - // Only push a new history entry if we switched to another entity, but not - // if we just changed a viewer setting. - if ( - !previousRootEntityRefs || - (rootEntityRefs.length === previousRootEntityRefs.length && - rootEntityRefs.every((v, i) => v === previousRootEntityRefs[i])) - ) { - window.history.replaceState(null, document.title, newUrl); - } else { - window.history.pushState(null, document.title, newUrl); - } + navigate(newParams, { replace: true }); }, [ - rootEntityNames, maxDepth, + curve, selectedKinds, selectedRelations, unidirectional, mergeRelations, direction, showFilters, - previousRootEntityRefs, + rootEntityNames, + navigate, ]); return {