Merge pull request #20586 from backstage/camilaibs/improve-error-boundary-component

[DI]  Improve `ExtensionBoundary` component

.changeset/fluffy-years-shake.md

@@ -0,0 +1,5 @@
+---
+'@backstage/plugin-catalog': patch
+---
+
+Use default extensions boundary and suspense on the alpha declarative `createCatalogFilterExtension` extension factory.

.changeset/sixty-tips-argue.md

@@ -0,0 +1,5 @@
+---
+'@backstage/frontend-plugin-api': patch
+---
+
+Improve the extension boundary component and create a default extension suspense component.

.changeset/young-days-talk.md

@@ -0,0 +1,5 @@
+---
+'@backstage/plugin-search-react': patch
+---
+
+Use default extensions boundary and suspense on the alpha declarative `createSearchResultListItem` extension factory.

packages/frontend-plugin-api/api-report.md

@@ -338,6 +338,10 @@ export interface ExtensionBoundaryProps {
   // (undocumented)
   children: ReactNode;
   // (undocumented)
+  id: string;
+  // (undocumented)
+  routable?: boolean;
+  // (undocumented)
   source?: BackstagePlugin;
 }
 

packages/frontend-plugin-api/package.json

@@ -38,9 +38,11 @@
     "react-router-dom": "6.0.0-beta.0 || ^6.3.0"
   },
   "dependencies": {
+    "@backstage/core-components": "workspace:^",
     "@backstage/core-plugin-api": "workspace:^",
     "@backstage/types": "workspace:^",
     "@backstage/version-bridge": "workspace:^",
+    "@material-ui/core": "^4.12.4",
     "@types/react": "^16.13.1 || ^17.0.0",
     "lodash": "^4.17.21",
     "zod": "^3.21.4",

packages/frontend-plugin-api/src/components/ErrorBoundary.tsx

@@ -0,0 +1,80 @@
+/*
+ * Copyright 2023 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 React, { Component, PropsWithChildren } from 'react';
+// TODO: Dependency on MUI should be removed from core packages
+import { Button } from '@material-ui/core';
+import { ErrorPanel } from '@backstage/core-components';
+import { BackstagePlugin } from '../wiring';
+
+type DefaultErrorBoundaryFallbackProps = PropsWithChildren<{
+  plugin?: BackstagePlugin;
+  error: Error;
+  resetError: () => void;
+}>;
+
+const DefaultErrorBoundaryFallback = ({
+  plugin,
+  error,
+  resetError,
+}: DefaultErrorBoundaryFallbackProps) => {
+  const title = `Error in ${plugin?.id}`;
+
+  return (
+    <ErrorPanel title={title} error={error} defaultExpanded>
+      <Button variant="outlined" onClick={resetError}>
+        Retry
+      </Button>
+    </ErrorPanel>
+  );
+};
+
+type ErrorBoundaryProps = PropsWithChildren<{ plugin?: BackstagePlugin }>;
+type ErrorBoundaryState = { error?: Error };
+
+/** @internal */
+export class ErrorBoundary extends Component<
+  ErrorBoundaryProps,
+  ErrorBoundaryState
+> {
+  static getDerivedStateFromError(error: Error) {
+    return { error };
+  }
+
+  state: ErrorBoundaryState = { error: undefined };
+
+  handleErrorReset = () => {
+    this.setState({ error: undefined });
+  };
+
+  render() {
+    const { error } = this.state;
+    const { plugin, children } = this.props;
+
+    if (error) {
+      // TODO: use a configurable error boundary fallback
+      return (
+        <DefaultErrorBoundaryFallback
+          plugin={plugin}
+          error={error}
+          resetError={this.handleErrorReset}
+        />
+      );
+    }
+
+    return children;
+  }
+}

packages/frontend-plugin-api/src/components/ExtensionBoundary.test.tsx

@@ -0,0 +1,134 @@
+/*
+ * Copyright 2023 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 React, { useEffect } from 'react';
+import { screen, waitFor } from '@testing-library/react';
+import {
+  MockAnalyticsApi,
+  MockConfigApi,
+  TestApiProvider,
+  renderWithEffects,
+} from '@backstage/test-utils';
+import { ExtensionBoundary } from './ExtensionBoundary';
+import {
+  Extension,
+  coreExtensionData,
+  createExtension,
+  createPlugin,
+} from '../wiring';
+import { analyticsApiRef, useAnalytics } from '@backstage/core-plugin-api';
+import { createApp } from '@backstage/frontend-app-api';
+import { JsonObject } from '@backstage/types';
+import { createRouteRef } from '../routing';
+
+function renderExtensionInTestApp(
+  extension: Extension<unknown>,
+  options?: {
+    config?: JsonObject;
+  },
+) {
+  const { config = {} } = options ?? {};
+
+  const app = createApp({
+    features: [
+      createPlugin({
+        id: 'plugin',
+        extensions: [extension],
+      }),
+    ],
+    configLoader: async () => new MockConfigApi(config),
+  });
+
+  return renderWithEffects(app.createRoot());
+}
+
+const wrapInBoundaryExtension = (element: JSX.Element) => {
+  const id = 'plugin.extension';
+  const routeRef = createRouteRef();
+  return createExtension({
+    id,
+    attachTo: { id: 'core.routes', input: 'routes' },
+    output: {
+      element: coreExtensionData.reactElement,
+      path: coreExtensionData.routePath,
+      routeRef: coreExtensionData.routeRef.optional(),
+    },
+    factory({ bind, source }) {
+      bind({
+        routeRef,
+        path: '/',
+        element: (
+          <ExtensionBoundary id={id} source={source}>
+            {element}
+          </ExtensionBoundary>
+        ),
+      });
+    },
+  });
+};
+
+describe('ExtensionBoundary', () => {
+  it('should render children when there is no error', async () => {
+    const text = 'Text Component';
+    const TextComponent = () => {
+      return <p>{text}</p>;
+    };
+    await renderExtensionInTestApp(wrapInBoundaryExtension(<TextComponent />));
+    await waitFor(() => expect(screen.getByText(text)).toBeInTheDocument());
+  });
+
+  it('should show app error component when an error is thrown', async () => {
+    const error = 'Something went wrong';
+    const ErrorComponent = () => {
+      throw new Error(error);
+    };
+    await renderExtensionInTestApp(wrapInBoundaryExtension(<ErrorComponent />));
+    await waitFor(() => expect(screen.getByText(error)).toBeInTheDocument());
+  });
+
+  it('should wrap children with analytics context', async () => {
+    const action = 'render';
+    const subject = 'analytics';
+    const analyticsApiMock = new MockAnalyticsApi();
+
+    const AnalyticsComponent = () => {
+      const analytics = useAnalytics();
+      useEffect(() => {
+        analytics.captureEvent(action, subject);
+      }, [analytics]);
+      return null;
+    };
+
+    await renderExtensionInTestApp(
+      wrapInBoundaryExtension(
+        <TestApiProvider apis={[[analyticsApiRef, analyticsApiMock]]}>
+          <AnalyticsComponent />
+        </TestApiProvider>,
+      ),
+    );
+
+    await waitFor(() =>
+      expect(analyticsApiMock.getEvents()[0]).toMatchObject({
+        action,
+        subject,
+        context: {
+          extension: 'plugin.extension',
+          routeRef: 'unknown',
+        },
+      }),
+    );
+  });
+});

packages/frontend-plugin-api/src/components/ExtensionBoundary.tsx

@@ -14,16 +14,59 @@
  * limitations under the License.
  */
 
-import React, { ReactNode } from 'react';
+import React, { PropsWithChildren, ReactNode, useEffect } from 'react';
+import { AnalyticsContext, useAnalytics } from '@backstage/core-plugin-api';
 import { BackstagePlugin } from '../wiring';
+import { ErrorBoundary } from './ErrorBoundary';
+import { ExtensionSuspense } from './ExtensionSuspense';
+// eslint-disable-next-line @backstage/no-relative-monorepo-imports
+import { routableExtensionRenderedEvent } from '../../../core-plugin-api/src/analytics/Tracker';
+
+type RouteTrackerProps = PropsWithChildren<{
+  disableTracking?: boolean;
+}>;
+
+const RouteTracker = (props: RouteTrackerProps) => {
+  const { disableTracking, children } = props;
+  const analytics = useAnalytics();
+
+  // This event, never exposed to end-users of the analytics API,
+  // helps inform which extension metadata gets associated with a
+  // navigation event when the route navigated to is a gathered
+  // mountpoint.
+  useEffect(() => {
+    if (disableTracking) return;
+    analytics.captureEvent(routableExtensionRenderedEvent, '');
+  }, [analytics, disableTracking]);
+
+  return <>{children}</>;
+};
 
 /** @public */
 export interface ExtensionBoundaryProps {
-  children: ReactNode;
+  id: string;
   source?: BackstagePlugin;
+  routable?: boolean;
+  children: ReactNode;
 }
 
 /** @public */
 export function ExtensionBoundary(props: ExtensionBoundaryProps) {
-  return <>{props.children}</>;
+  const { id, source, routable, children } = props;
+
+  // Skipping "routeRef" attribute in the new system, the extension "id" should provide more insight
+  const attributes = {
+    extension: id,
+    pluginId: source?.id,
+  };
+
+  return (
+    <ExtensionSuspense>
+      <ErrorBoundary plugin={source}>
+        <AnalyticsContext attributes={attributes}>
+          <RouteTracker disableTracking={!routable}>{children}</RouteTracker>
+        </AnalyticsContext>
+      </ErrorBoundary>
+    </ExtensionSuspense>
+  );
 }

packages/frontend-plugin-api/src/components/ExtensionSuspense.test.tsx

@@ -0,0 +1,54 @@
+/*
+ * Copyright 2023 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 React, { lazy } from 'react';
+import { screen, waitFor } from '@testing-library/react';
+import { renderWithEffects, wrapInTestApp } from '@backstage/test-utils';
+import { ExtensionSuspense } from './ExtensionSuspense';
+
+describe('ExtensionSuspense', () => {
+  it('should render the app progress component as fallback', async () => {
+    const LazyComponent = lazy(() => new Promise(() => {}));
+
+    await renderWithEffects(
+      wrapInTestApp(
+        <ExtensionSuspense>
+          <LazyComponent />
+        </ExtensionSuspense>,
+      ),
+    );
+
+    expect(screen.getByTestId('progress')).toBeInTheDocument();
+  });
+
+  it('should render the lazy loaded children component', async () => {
+    const LazyComponent = lazy(() =>
+      Promise.resolve({ default: () => <div>Lazy Component</div> }),
+    );
+
+    await renderWithEffects(
+      wrapInTestApp(
+        <ExtensionSuspense>
+          <LazyComponent />
+        </ExtensionSuspense>,
+      ),
+    );
+
+    await waitFor(() =>
+      expect(screen.getByText('Lazy Component')).toBeInTheDocument(),
+    );
+  });
+});

packages/frontend-plugin-api/src/components/ExtensionSuspense.tsx

@@ -0,0 +1,33 @@
+/*
+ * Copyright 2023 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 React, { ReactNode, Suspense } from 'react';
+import { useApp } from '@backstage/core-plugin-api';
+
+/** @public */
+export interface ExtensionSuspenseProps {
+  children: ReactNode;
+}
+
+/** @public */
+export function ExtensionSuspense(props: ExtensionSuspenseProps) {
+  const { children } = props;
+
+  const app = useApp();
+  const { Progress } = app.getComponents();
+
+  return <Suspense fallback={<Progress />}>{children}</Suspense>;
+}

packages/frontend-plugin-api/src/extensions/createPageExtension.test.tsx

@@ -15,10 +15,23 @@
  */
 
 import React from 'react';
+import { renderWithEffects, wrapInTestApp } from '@backstage/test-utils';
+import { useAnalytics } from '@backstage/core-plugin-api';
+import { waitFor } from '@testing-library/react';
 import { PortableSchema } from '../schema';
-import { coreExtensionData, createExtensionInput } from '../wiring';
+import {
+  ExtensionInputValues,
+  coreExtensionData,
+  createExtensionInput,
+  createPlugin,
+} from '../wiring';
 import { createPageExtension } from './createPageExtension';
 
+jest.mock('@backstage/core-plugin-api', () => ({
+  ...jest.requireActual('@backstage/core-plugin-api'),
+  useAnalytics: jest.fn(),
+}));
+
 describe('createPageExtension', () => {
   it('creates the extension properly', () => {
     const configSchema: PortableSchema<{ path: string }> = {
@@ -100,4 +113,35 @@ describe('createPageExtension', () => {
       factory: expect.any(Function),
     });
   });
+
+  it('capture page view event in analytics', async () => {
+    const captureEvent = jest.fn();
+
+    (useAnalytics as jest.Mock).mockReturnValue({
+      captureEvent,
+    });
+
+    const extension = createPageExtension({
+      id: 'plugin.page',
+      defaultPath: '/',
+      loader: async () => <div>Component</div>,
+    });
+
+    extension.factory({
+      bind: (values: ExtensionInputValues<any>) =>
+        renderWithEffects(
+          wrapInTestApp(values.element as unknown as JSX.Element),
+        ),
+      source: createPlugin({ id: 'plugin ' }),
+      config: { path: '/' },
+      inputs: {},
+    });
+
+    await waitFor(() =>
+      expect(captureEvent).toHaveBeenCalledWith(
+        '_ROUTABLE-EXTENSION-RENDERED',
+        '',
+      ),
+    );
+  });
 });

packages/frontend-plugin-api/src/extensions/createPageExtension.tsx

@@ -14,7 +14,7 @@
  * limitations under the License.
  */
 
-import React from 'react';
+import React, { lazy } from 'react';
 import { ExtensionBoundary } from '../components';
 import { createSchemaFromZod, PortableSchema } from '../schema';
 import {
@@ -22,10 +22,10 @@ import {
   createExtension,
   Extension,
   ExtensionInputValues,
+  AnyExtensionInputMap,
 } from '../wiring';
-import { AnyExtensionInputMap } from '../wiring/createExtension';
-import { Expand } from '../types';
 import { RouteRef } from '../routing';
+import { Expand } from '../types';
 
 /**
  * Helper for creating extensions for a routable React page component.
@@ -55,6 +55,8 @@ export function createPageExtension<
     }) => Promise<JSX.Element>;
   },
 ): Extension<TConfig> {
+  const { id } = options;
+
   const configSchema =
     'configSchema' in options
       ? options.configSchema
@@ -63,18 +65,18 @@ export function createPageExtension<
         ) as PortableSchema<TConfig>);
 
   return createExtension({
-    id: options.id,
+    id,
     attachTo: options.attachTo ?? { id: 'core.routes', input: 'routes' },
+    configSchema,
+    inputs: options.inputs,
     disabled: options.disabled,
     output: {
       element: coreExtensionData.reactElement,
       path: coreExtensionData.routePath,
       routeRef: coreExtensionData.routeRef.optional(),
     },
-    inputs: options.inputs,
-    configSchema,
     factory({ bind, config, inputs, source }) {
-      const LazyComponent = React.lazy(() =>
+      const ExtensionComponent = lazy(() =>
         options
           .loader({ config, inputs })
           .then(element => ({ default: () => element })),
@@ -82,14 +84,12 @@ export function createPageExtension<
 
       bind({
         path: config.path,
+        routeRef: options.routeRef,
         element: (
-          <ExtensionBoundary source={source}>
-            <React.Suspense fallback="...">
-              <LazyComponent />
-            </React.Suspense>
+          <ExtensionBoundary id={id} source={source} routable>
+            <ExtensionComponent />
           </ExtensionBoundary>
         ),
-        routeRef: options.routeRef,
       });
     },
   });

plugins/catalog/src/alpha.tsx

@@ -51,7 +51,6 @@ import {
   rootRouteRef,
   viewTechDocRouteRef,
 } from './routes';
-import { Progress } from '@backstage/core-components';
 import { useEntityFromUrl } from './components/CatalogEntityPage/useEntityFromUrl';
 
 /** @alpha */
@@ -97,8 +96,10 @@ export function createCatalogFilterExtension<
   configSchema?: PortableSchema<TConfig>;
   loader: (options: { config: TConfig }) => Promise<JSX.Element>;
 }) {
+  const id = `catalog.filter.${options.id}`;
+
   return createExtension({
-    id: `catalog.filter.${options.id}`,
+    id,
     attachTo: { id: 'plugin.catalog.page.index', input: 'filters' },
     inputs: options.inputs ?? {},
     configSchema: options.configSchema,
@@ -106,7 +107,7 @@ export function createCatalogFilterExtension<
       element: coreExtensionData.reactElement,
     },
     factory({ bind, config, source }) {
-      const LazyComponent = React.lazy(() =>
+      const ExtensionComponent = React.lazy(() =>
         options
           .loader({ config })
           .then(element => ({ default: () => element })),
@@ -114,10 +115,8 @@ export function createCatalogFilterExtension<
 
       bind({
         element: (
-          <ExtensionBoundary source={source}>
-            <React.Suspense fallback={<Progress />}>
-              <LazyComponent />
-            </React.Suspense>
+          <ExtensionBoundary id={id} source={source}>
+            <ExtensionComponent />
           </ExtensionBoundary>
         ),
       });

plugins/search-react/src/alpha.tsx

@@ -14,7 +14,7 @@
  * limitations under the License.
  */
 
-import React, { lazy, Suspense } from 'react';
+import React, { lazy } from 'react';
 
 import { ListItemProps } from '@material-ui/core';
 
@@ -25,7 +25,6 @@ import {
   createExtensionDataRef,
   createSchemaFromZod,
 } from '@backstage/frontend-plugin-api';
-import { Progress } from '@backstage/core-components';
 import { SearchDocument, SearchResult } from '@backstage/plugin-search-common';
 
 import { SearchResultListItemExtension } from './extensions';
@@ -87,6 +86,8 @@ export type SearchResultItemExtensionOptions<
 export function createSearchResultListItemExtension<
   TConfig extends { noTrack?: boolean },
 >(options: SearchResultItemExtensionOptions<TConfig>) {
+  const id = `plugin.search.result.item.${options.id}`;
+
   const configSchema =
     'configSchema' in options
       ? options.configSchema
@@ -95,15 +96,19 @@ export function createSearchResultListItemExtension<
             noTrack: z.boolean().default(false),
           }),
         ) as PortableSchema<TConfig>);
+
   return createExtension({
-    id: `plugin.search.result.item.${options.id}`,
-    attachTo: options.attachTo ?? { id: 'plugin.search.page', input: 'items' },
+    id,
+    attachTo: options.attachTo ?? {
+      id: 'plugin.search.page',
+      input: 'items',
+    },
     configSchema,
     output: {
       item: searchResultItemExtensionData,
     },
     factory({ bind, config, source }) {
-      const LazyComponent = lazy(() =>
+      const ExtensionComponent = lazy(() =>
         options
           .component({ config })
           .then(component => ({ default: component })),
@@ -113,16 +118,14 @@ export function createSearchResultListItemExtension<
         item: {
           predicate: options.predicate,
           component: props => (
-            <ExtensionBoundary source={source}>
-              <Suspense fallback={<Progress />}>
-                <SearchResultListItemExtension
-                  rank={props.rank}
-                  result={props.result}
-                  noTrack={config.noTrack}
-                >
-                  <LazyComponent {...props} />
-                </SearchResultListItemExtension>
-              </Suspense>
+            <ExtensionBoundary id={id} source={source}>
+              <SearchResultListItemExtension
+                rank={props.rank}
+                result={props.result}
+                noTrack={config.noTrack}
+              >
+                <ExtensionComponent {...props} />
+              </SearchResultListItemExtension>
             </ExtensionBoundary>
           ),
         },

yarn.lock

@@ -4246,11 +4246,13 @@ __metadata:
   resolution: "@backstage/frontend-plugin-api@workspace:packages/frontend-plugin-api"
   dependencies:
     "@backstage/cli": "workspace:^"
+    "@backstage/core-components": "workspace:^"
     "@backstage/core-plugin-api": "workspace:^"
     "@backstage/frontend-app-api": "workspace:^"
     "@backstage/test-utils": "workspace:^"
     "@backstage/types": "workspace:^"
     "@backstage/version-bridge": "workspace:^"
+    "@material-ui/core": ^4.12.4
     "@testing-library/jest-dom": ^6.0.0
     "@testing-library/react": ^14.0.0
     "@types/react": ^16.13.1 || ^17.0.0