Merge pull request #31447 from Believe-SA/fix-techdocs-header-reload

fix(techdocs): Techdocs page re-rendering on every subpage navigation
2025-10-20 09:47:24 +02:00
parent 6d396ee333 52e01b98bc
commit 78c5d40bf2
4 changed files with 400 additions and 54 deletions
@@ -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.
@@ -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 <Progress />;
  }

@@ -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 }) => (
+    <TestApiProvider apis={[[catalogApiRef, mockCatalogApi]]}>
+      {children}
+    </TestApiProvider>
+  );
+
+  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);
+    });
+  });
+});
@@ -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<string | null>(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,
+  };
+}