Merge pull request #5229 from SDA-SE/feat/chunks

Handle chunk loading errors

.changeset/forty-queens-hear.md

@@ -0,0 +1,7 @@
+---
+'@backstage/core': patch
+'@backstage/core-api': patch
+---
+
+Introduce a `load-chunk` step in the `BootErrorPage` to show make chunk loading
+errors visible to the user.

packages/core-api/src/app/types.ts

@@ -24,7 +24,7 @@ import { AppConfig } from '@backstage/config';
 import { SubRouteRef } from '../routing/types';
 
 export type BootErrorPageProps = {
-  step: 'load-config';
+  step: 'load-config' | 'load-chunk';
   error: Error;
 };
 

packages/core-api/src/extensions/extensions.tsx

@@ -15,9 +15,10 @@
  */
 
 import React, { lazy, Suspense } from 'react';
+import { useApp } from '../app';
+import { BackstagePlugin, Extension } from '../plugin/types';
 import { RouteRef, useRouteRef } from '../routing';
 import { attachComponentData } from './componentData';
-import { Extension, BackstagePlugin } from '../plugin/types';
 
 type ComponentLoader<T> =
   | {
@@ -40,30 +41,41 @@ export function createRoutableExtension<
   return createReactExtension({
     component: {
       lazy: () =>
-        component().then(InnerComponent => {
-          const RoutableExtensionWrapper: any = (props: any) => {
-            // Validate that the routing is wired up correctly in the App.tsx
-            try {
-              useRouteRef(mountPoint);
-            } catch {
-              throw new Error(
-                `Routable extension component with mount point ${mountPoint} was not discovered in the app element tree. ` +
-                  'Routable extension components may not be rendered by other components and must be ' +
-                  'directly available as an element within the App provider component.',
-              );
-            }
-            return <InnerComponent {...props} />;
-          };
+        component().then(
+          InnerComponent => {
+            const RoutableExtensionWrapper: any = (props: any) => {
+              // Validate that the routing is wired up correctly in the App.tsx
+              try {
+                useRouteRef(mountPoint);
+              } catch {
+                throw new Error(
+                  `Routable extension component with mount point ${mountPoint} was not discovered in the app element tree. ` +
+                    'Routable extension components may not be rendered by other components and must be ' +
+                    'directly available as an element within the App provider component.',
+                );
+              }
+              return <InnerComponent {...props} />;
+            };
 
-          const componentName =
-            (InnerComponent as { displayName?: string }).displayName ||
-            InnerComponent.name ||
-            'LazyComponent';
+            const componentName =
+              (InnerComponent as { displayName?: string }).displayName ||
+              InnerComponent.name ||
+              'LazyComponent';
 
-          RoutableExtensionWrapper.displayName = `RoutableExtension(${componentName})`;
+            RoutableExtensionWrapper.displayName = `RoutableExtension(${componentName})`;
 
-          return RoutableExtensionWrapper as T;
-        }),
+            return RoutableExtensionWrapper as T;
+          },
+          error => {
+            const RoutableExtensionWrapper: any = (_: any) => {
+              const app = useApp();
+              const { BootErrorPage } = app.getComponents();
+
+              return <BootErrorPage step="load-chunk" error={error} />;
+            };
+            return RoutableExtensionWrapper;
+          },
+        ),
     },
     data: {
       'core.mountPoint': mountPoint,

packages/core/src/api-wrappers/createApp.test.tsx

@@ -14,7 +14,10 @@
  * limitations under the License.
  */
 
-import { defaultConfigLoader } from './createApp';
+import { render } from '@testing-library/react';
+import React from 'react';
+import { MemoryRouter } from 'react-router';
+import { defaultConfigLoader, OptionallyWrapInRouter } from './createApp';
 
 (process as any).env = { NODE_ENV: 'test' };
 const anyEnv = process.env as any;
@@ -92,3 +95,23 @@ describe('defaultConfigLoader', () => {
     ]);
   });
 });
+
+describe('OptionallyWrapInRouter', () => {
+  it('should wrap with router if not yet inside a router', async () => {
+    const { getByText } = render(
+      <OptionallyWrapInRouter>Test</OptionallyWrapInRouter>,
+    );
+
+    expect(getByText('Test')).toBeInTheDocument();
+  });
+
+  it('should not wrap with router if already inside a router', async () => {
+    const { getByText } = render(
+      <MemoryRouter>
+        <OptionallyWrapInRouter>Test</OptionallyWrapInRouter>
+      </MemoryRouter>,
+    );
+
+    expect(getByText('Test')).toBeInTheDocument();
+  });
+});

packages/core/src/api-wrappers/createApp.tsx

@@ -14,14 +14,18 @@
  * limitations under the License.
  */
 
-import React from 'react';
+import React, { PropsWithChildren } from 'react';
 import privateExports, {
   AppOptions,
   defaultSystemIcons,
   BootErrorPageProps,
   AppConfigLoader,
 } from '@backstage/core-api';
-import { BrowserRouter, MemoryRouter } from 'react-router-dom';
+import {
+  BrowserRouter,
+  MemoryRouter,
+  useInRouterContext,
+} from 'react-router-dom';
 import LightIcon from '@material-ui/icons/WbSunny';
 import DarkIcon from '@material-ui/icons/Brightness2';
 import { ErrorPage } from '../layout/ErrorPage';
@@ -89,6 +93,13 @@ export const defaultConfigLoader: AppConfigLoader = async (
 // The actual implementation of the app class still lives in core-api,
 // as it needs to be used by dev- and test-utils.
 
+export function OptionallyWrapInRouter({ children }: PropsWithChildren<{}>) {
+  if (useInRouterContext()) {
+    return <>{children}</>;
+  }
+  return <MemoryRouter>{children}</MemoryRouter>;
+}
+
 /**
  * Creates a new Backstage App.
  */
@@ -100,12 +111,14 @@ export function createApp(options?: AppOptions) {
     let message = '';
     if (step === 'load-config') {
       message = `The configuration failed to load, someone should have a look at this error: ${error.message}`;
+    } else if (step === 'load-chunk') {
+      message = `Lazy loaded chunk failed to load, try to reload the page: ${error.message}`;
     }
     // TODO: figure out a nicer way to handle routing on the error page, when it can be done.
     return (
-      <MemoryRouter>
+      <OptionallyWrapInRouter>
         <ErrorPage status="501" statusMessage={message} />
-      </MemoryRouter>
+      </OptionallyWrapInRouter>
     );
   };