diff --git a/.changeset/ninety-cobras-feel.md b/.changeset/ninety-cobras-feel.md new file mode 100644 index 0000000000..e287ef33ed --- /dev/null +++ b/.changeset/ninety-cobras-feel.md @@ -0,0 +1,5 @@ +--- +'@backstage/plugin-techdocs': patch +--- + +Fixed an issue where the entire TechDocs page would re-render when navigating between pages within the same entity's documentation. diff --git a/plugins/techdocs/src/reader/components/TechDocsReaderPage/TechDocsReaderPage.tsx b/plugins/techdocs/src/reader/components/TechDocsReaderPage/TechDocsReaderPage.tsx index c67660fd79..4a2b9e2002 100644 --- a/plugins/techdocs/src/reader/components/TechDocsReaderPage/TechDocsReaderPage.tsx +++ b/plugins/techdocs/src/reader/components/TechDocsReaderPage/TechDocsReaderPage.tsx @@ -14,25 +14,15 @@ * limitations under the License. */ -import { - Children, - ReactElement, - ReactNode, - useEffect, - useMemo, - useCallback, -} from 'react'; -import { useOutlet, useNavigate } from 'react-router-dom'; -import { Page } from '@backstage/core-components'; +import { Children, ReactElement, ReactNode, useMemo } from 'react'; +import { useOutlet } from 'react-router-dom'; +import { Page, Progress } from '@backstage/core-components'; import { CompoundEntityRef } from '@backstage/catalog-model'; import { TECHDOCS_ADDONS_KEY, TECHDOCS_ADDONS_WRAPPER_KEY, TechDocsReaderPageProvider, - buildTechDocsURL, } from '@backstage/plugin-techdocs-react'; -import { TECHDOCS_EXTERNAL_ANNOTATION } from '@backstage/plugin-techdocs-common'; -import useAsync from 'react-use/esm/useAsync'; import { TechDocsReaderPageRenderFunction } from '../../../types'; import { TechDocsReaderPageContent } from '../TechDocsReaderPageContent'; import { TechDocsReaderPageHeader } from '../TechDocsReaderPageHeader'; @@ -41,11 +31,8 @@ import { rootDocsRouteRef } from '../../../routes'; import { getComponentData, useRouteRefParams, - useApi, - useRouteRef, } from '@backstage/core-plugin-api'; import { CookieAuthRefreshProvider } from '@backstage/plugin-auth-react'; -import { catalogApiRef } from '@backstage/plugin-catalog-react'; import { createTheme, styled, @@ -53,7 +40,7 @@ import { ThemeProvider, useTheme, } from '@material-ui/core/styles'; -import { Progress } from '@backstage/core-components'; +import { useExternalRedirect } from './useExternalRedirect'; /* An explanation for the multiple ways of customizing the TechDocs reader page @@ -201,10 +188,6 @@ export const TechDocsReaderPage = (props: TechDocsReaderPageProps) => { const outlet = useOutlet(); - const catalogApi = useApi(catalogApiRef); - const navigate = useNavigate(); - const viewTechdocLink = useRouteRef(rootDocsRouteRef); - const memoizedEntityRef = useMemo( () => ({ kind: entityRef.kind, @@ -214,38 +197,8 @@ export const TechDocsReaderPage = (props: TechDocsReaderPageProps) => { [entityRef.kind, entityRef.name, entityRef.namespace], ); - const externalEntityTechDocsUrl = useAsync(async () => { - try { - const catalogEntity = await catalogApi.getEntityByRef(memoizedEntityRef); - - if ( - catalogEntity?.metadata?.annotations?.[TECHDOCS_EXTERNAL_ANNOTATION] - ) { - return buildTechDocsURL(catalogEntity, viewTechdocLink); - } - } catch (error) { - // Ignore error and allow an attempt at loading the current entity's TechDocs when unable to fetch an external entity from the catalog. - } - - return undefined; - }, [memoizedEntityRef, catalogApi, viewTechdocLink]); - - const handleNavigation = useCallback( - (url: string) => { - navigate(url, { replace: true }); - }, - [navigate], - ); - - useEffect(() => { - if (!externalEntityTechDocsUrl.loading && externalEntityTechDocsUrl.value) { - handleNavigation(externalEntityTechDocsUrl.value); - } - }, [ - externalEntityTechDocsUrl.loading, - externalEntityTechDocsUrl.value, - handleNavigation, - ]); + // Check for external TechDocs redirects and handle navigation + const { shouldShowProgress } = useExternalRedirect(memoizedEntityRef); const page: ReactNode = useMemo(() => { if (children) { @@ -265,7 +218,9 @@ export const TechDocsReaderPage = (props: TechDocsReaderPageProps) => { ); }, [children, outlet]); - if (externalEntityTechDocsUrl.loading || externalEntityTechDocsUrl.value) { + // Show full-page loading spinner when checking for external redirects or about to redirect. + // This replaces the entire page content (header, sidebar, and documentation). + if (shouldShowProgress) { return ; } diff --git a/plugins/techdocs/src/reader/components/TechDocsReaderPage/useExternalRedirect.test.tsx b/plugins/techdocs/src/reader/components/TechDocsReaderPage/useExternalRedirect.test.tsx new file mode 100644 index 0000000000..e7d41f6ff9 --- /dev/null +++ b/plugins/techdocs/src/reader/components/TechDocsReaderPage/useExternalRedirect.test.tsx @@ -0,0 +1,282 @@ +/* + * Copyright 2025 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 { renderHook, waitFor } from '@testing-library/react'; +import { useExternalRedirect } from './useExternalRedirect'; +import { TestApiProvider } from '@backstage/test-utils'; +import { catalogApiRef } from '@backstage/plugin-catalog-react'; +import { TECHDOCS_EXTERNAL_ANNOTATION } from '@backstage/plugin-techdocs-common'; + +const mockNavigate = jest.fn(); +const mockViewTechdocLink = jest.fn(() => '/docs'); + +jest.mock('react-router-dom', () => ({ + ...jest.requireActual('react-router-dom'), + useNavigate: () => mockNavigate, +})); + +jest.mock('@backstage/core-plugin-api', () => ({ + ...jest.requireActual('@backstage/core-plugin-api'), + useRouteRef: () => mockViewTechdocLink, +})); + +describe('useExternalRedirect', () => { + const mockCatalogApi = { + getEntityByRef: jest.fn(), + }; + + const wrapper = ({ children }: { children: React.ReactNode }) => ( + + {children} + + ); + + beforeEach(() => { + jest.clearAllMocks(); + }); + + it('should not show progress when entity has no external annotation', async () => { + mockCatalogApi.getEntityByRef.mockResolvedValue({ + apiVersion: 'backstage.io/v1alpha1', + kind: 'Component', + metadata: { + name: 'test-component', + namespace: 'default', + }, + }); + + const { result } = renderHook( + () => + useExternalRedirect({ + kind: 'Component', + name: 'test-component', + namespace: 'default', + }), + { wrapper }, + ); + + await waitFor(() => { + expect(result.current.loading).toBe(false); + }); + + expect(result.current.shouldShowProgress).toBe(false); + expect(mockNavigate).not.toHaveBeenCalled(); + }); + + it('should navigate when entity has external annotation', async () => { + mockCatalogApi.getEntityByRef.mockResolvedValue({ + apiVersion: 'backstage.io/v1alpha1', + kind: 'Component', + metadata: { + name: 'test-component', + namespace: 'default', + annotations: { + [TECHDOCS_EXTERNAL_ANNOTATION]: 'component:external/docs', + }, + }, + }); + + const { result } = renderHook( + () => + useExternalRedirect({ + kind: 'Component', + name: 'test-component', + namespace: 'default', + }), + { wrapper }, + ); + + await waitFor(() => { + expect(result.current.loading).toBe(false); + }); + + expect(result.current.shouldShowProgress).toBe(true); + expect(mockNavigate).toHaveBeenCalledWith(expect.any(String), { + replace: true, + }); + }); + + it('should handle catalog API errors gracefully', async () => { + mockCatalogApi.getEntityByRef.mockRejectedValue( + new Error('Catalog API error'), + ); + + const { result } = renderHook( + () => + useExternalRedirect({ + kind: 'Component', + name: 'test-component', + namespace: 'default', + }), + { wrapper }, + ); + + await waitFor(() => { + expect(result.current.loading).toBe(false); + }); + + expect(result.current.shouldShowProgress).toBe(false); + expect(mockNavigate).not.toHaveBeenCalled(); + }); + + it('should not re-check when entity key stays the same', async () => { + mockCatalogApi.getEntityByRef.mockResolvedValue({ + apiVersion: 'backstage.io/v1alpha1', + kind: 'Component', + metadata: { + name: 'test-component', + namespace: 'default', + }, + }); + + const { rerender } = renderHook( + () => + useExternalRedirect({ + kind: 'Component', + name: 'test-component', + namespace: 'default', + }), + { wrapper }, + ); + + await waitFor(() => { + expect(mockCatalogApi.getEntityByRef).toHaveBeenCalledTimes(1); + }); + + // Rerender with the same entity - should not trigger another API call + rerender(); + + expect(mockCatalogApi.getEntityByRef).toHaveBeenCalledTimes(1); + }); + + it('should handle deep linking with techdocs-entity-path annotation', async () => { + mockCatalogApi.getEntityByRef.mockResolvedValue({ + apiVersion: 'backstage.io/v1alpha1', + kind: 'Component', + metadata: { + name: 'test-component', + namespace: 'default', + annotations: { + 'backstage.io/techdocs-entity': 'component:external/docs', + 'backstage.io/techdocs-entity-path': '/inner-component-docs', + }, + }, + }); + + const { result } = renderHook( + () => + useExternalRedirect({ + kind: 'Component', + name: 'test-component', + namespace: 'default', + }), + { wrapper }, + ); + + await waitFor(() => { + expect(result.current.loading).toBe(false); + }); + + // Should navigate with the path appended (handled by buildTechDocsURL) + expect(mockNavigate).toHaveBeenCalledWith(expect.any(String), { + replace: true, + }); + expect(result.current.shouldShowProgress).toBe(true); + }); + + it('should handle empty external annotation value', async () => { + mockCatalogApi.getEntityByRef.mockResolvedValue({ + apiVersion: 'backstage.io/v1alpha1', + kind: 'Component', + metadata: { + name: 'test-component', + namespace: 'default', + annotations: { + 'backstage.io/techdocs-entity': '', + }, + }, + }); + + const { result } = renderHook( + () => + useExternalRedirect({ + kind: 'Component', + name: 'test-component', + namespace: 'default', + }), + { wrapper }, + ); + + await waitFor(() => { + expect(result.current.loading).toBe(false); + }); + + // Empty annotation should be treated as no redirect + expect(result.current.shouldShowProgress).toBe(false); + expect(mockNavigate).not.toHaveBeenCalled(); + }); + + it('should re-check when entity changes', async () => { + mockCatalogApi.getEntityByRef + .mockResolvedValueOnce({ + apiVersion: 'backstage.io/v1alpha1', + kind: 'Component', + metadata: { + name: 'first-component', + namespace: 'default', + }, + }) + .mockResolvedValueOnce({ + apiVersion: 'backstage.io/v1alpha1', + kind: 'Component', + metadata: { + name: 'second-component', + namespace: 'default', + }, + }); + + const { rerender } = renderHook( + ({ entityRef }) => useExternalRedirect(entityRef), + { + wrapper, + initialProps: { + entityRef: { + kind: 'Component', + name: 'first-component', + namespace: 'default', + }, + }, + }, + ); + + await waitFor(() => { + expect(mockCatalogApi.getEntityByRef).toHaveBeenCalledTimes(1); + }); + + // Change to a different entity + rerender({ + entityRef: { + kind: 'Component', + name: 'second-component', + namespace: 'default', + }, + }); + + await waitFor(() => { + expect(mockCatalogApi.getEntityByRef).toHaveBeenCalledTimes(2); + }); + }); +}); diff --git a/plugins/techdocs/src/reader/components/TechDocsReaderPage/useExternalRedirect.ts b/plugins/techdocs/src/reader/components/TechDocsReaderPage/useExternalRedirect.ts new file mode 100644 index 0000000000..8d66fab550 --- /dev/null +++ b/plugins/techdocs/src/reader/components/TechDocsReaderPage/useExternalRedirect.ts @@ -0,0 +1,104 @@ +/* + * Copyright 2025 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 { useEffect, useRef } from 'react'; +import { useNavigate } from 'react-router-dom'; +import useAsync from 'react-use/esm/useAsync'; +import { useApi, useRouteRef } from '@backstage/core-plugin-api'; +import { catalogApiRef } from '@backstage/plugin-catalog-react'; +import { + CompoundEntityRef, + stringifyEntityRef, +} from '@backstage/catalog-model'; +import { buildTechDocsURL } from '@backstage/plugin-techdocs-react'; +import { TECHDOCS_EXTERNAL_ANNOTATION } from '@backstage/plugin-techdocs-common'; +import { rootDocsRouteRef } from '../../../routes'; + +/** + * Hook to handle external TechDocs redirects based on entity annotations. + * Checks if an entity has the `backstage.io/techdocs-entity` annotation and + * redirects to the external TechDocs URL if present. + * + * @param entityRef - The entity reference to check for external redirects + * @returns Object containing loading state and whether a redirect is in progress + * + * @internal + */ +export function useExternalRedirect(entityRef: CompoundEntityRef): { + loading: boolean; + shouldShowProgress: boolean; +} { + const catalogApi = useApi(catalogApiRef); + const navigate = useNavigate(); + const viewTechdocLink = useRouteRef(rootDocsRouteRef); + + // Create a stable string key for the entity to use as a dependency. + // This ensures the useAsync hook only re-runs when the entity changes, + // preventing redundant API calls during sub-page navigation within the same entity's documentation. + const entityKey = stringifyEntityRef(entityRef); + // Track which entity we've already checked to avoid redundant checks + // when navigating between pages within the same entity's documentation. + const checkedEntityRef = useRef(null); + const shouldCheckForRedirect = checkedEntityRef.current !== entityKey; + + // Check if this entity should redirect to external TechDocs + const externalRedirectResult = useAsync(async () => { + try { + const catalogEntity = await catalogApi.getEntityByRef(entityRef); + + if ( + catalogEntity?.metadata?.annotations?.[TECHDOCS_EXTERNAL_ANNOTATION] + ) { + return buildTechDocsURL(catalogEntity, viewTechdocLink); + } + } catch (error) { + // Ignore errors and allow the current entity's TechDocs to load. + // This handles cases where the catalog API is unavailable or the entity doesn't exist. + } + + return undefined; + }, [entityKey, catalogApi]); + + useEffect(() => { + // Navigate to external TechDocs if a redirect URL is available + if (!externalRedirectResult.loading && externalRedirectResult.value) { + navigate(externalRedirectResult.value, { replace: true }); + } + + // Mark entity as checked once we've determined there's no redirect needed. + // This prevents the entire page from unmounting/remounting (showing Progress spinner) + // on subsequent sub-page navigation within the same entity. + if (!externalRedirectResult.loading && !externalRedirectResult.value) { + checkedEntityRef.current = entityKey; + } + }, [ + externalRedirectResult.loading, + externalRedirectResult.value, + navigate, + entityKey, + ]); + + // Determine if we should show a loading indicator (which replaces the entire page with a Progress spinner). + // Only show it when: 1) checking a new/unchecked entity, or 2) we have a redirect URL and are navigating. + const shouldShowProgress = + (shouldCheckForRedirect && externalRedirectResult.loading) || + !!externalRedirectResult.value; + + return { + loading: externalRedirectResult.loading, + shouldShowProgress, + }; +}