core-app-api: added validation of external route bindings

Signed-off-by: Patrik Oldsberg <poldsberg@gmail.com>

.changeset/short-apples-return.md

@@ -0,0 +1,5 @@
+---
+'@backstage/core-app-api': patch
+---
+
+Added validation during the application startup that detects if there are any plugins present that have not had their required external routes bound. Failing the validation will cause a hard crash as it is a programmer error. It lets you detect early on that there are dangling routes, rather than having them cause an error later on.

packages/core-app-api/src/app/AppManager.test.tsx

@@ -545,8 +545,47 @@ describe('Integration Test', () => {
     });
     expect(errorLogs).toEqual([
       expect.stringContaining(
-        'Parameter :thing is duplicated in path /test/:thing/some/:thing',
+        'The above error occurred in the <Provider> component',
       ),
+    ]);
+  });
+
+  it('should throw an error when required external plugin routes are not bound', () => {
+    const app = new AppManager({
+      apis: [],
+      defaultApis: [],
+      themes: [
+        {
+          id: 'light',
+          title: 'Light Theme',
+          variant: 'light',
+          Provider: ({ children }) => <>{children}</>,
+        },
+      ],
+      icons,
+      plugins: [],
+      components,
+      configLoader: async () => [],
+    });
+
+    const Provider = app.getProvider();
+    const Router = app.getRouter();
+    const { error: errorLogs } = withLogCollector(() => {
+      expect(() =>
+        render(
+          <Provider>
+            <Router>
+              <Routes>
+                <Route path="/test/:thing" element={<ExposedComponent />} />
+              </Routes>
+            </Router>
+          </Provider>,
+        ),
+      ).toThrow(
+        /^External route 'extRouteRef1' of the 'blob' plugin must be bound to a target route/,
+      );
+    });
+    expect(errorLogs).toEqual([
       expect.stringContaining(
         'The above error occurred in the <Provider> component',
       ),

packages/core-app-api/src/app/AppManager.tsx

@@ -64,7 +64,10 @@ import {
 } from '../routing/collectors';
 import { RoutingProvider } from '../routing/RoutingProvider';
 import { RouteTracker } from '../routing/RouteTracker';
-import { validateRoutes } from '../routing/validation';
+import {
+  validateRouteParameters,
+  validateRouteBindings,
+} from '../routing/validation';
 import { AppContextProvider } from './AppContext';
 import { AppIdentityProxy } from '../apis/implementations/IdentityApi/AppIdentityProxy';
 import {
@@ -193,7 +196,7 @@ export class AppManager implements BackstageApp {
   private readonly themes: AppTheme[];
   private readonly configLoader?: AppConfigLoader;
   private readonly defaultApis: Iterable<AnyApiFactory>;
-  private readonly bindRoutes: AppOptions['bindRoutes'];
+  private readonly routeBindings: Map<ExternalRouteRef, RouteRef | SubRouteRef>;
 
   private readonly appIdentityProxy = new AppIdentityProxy();
   private readonly apiFactoryRegistry: ApiFactoryRegistry;
@@ -206,7 +209,7 @@ export class AppManager implements BackstageApp {
     this.themes = options.themes as AppTheme[];
     this.configLoader = options.configLoader ?? defaultConfigLoader;
     this.defaultApis = options.defaultApis ?? [];
-    this.bindRoutes = options.bindRoutes;
+    this.routeBindings = generateBoundRoutes(options.bindRoutes);
     this.apiFactoryRegistry = new ApiFactoryRegistry();
   }
 
@@ -225,6 +228,9 @@ export class AppManager implements BackstageApp {
   getProvider(): ComponentType<{}> {
     const appContext = new AppContextImpl(this);
 
+    // We only validate routes once
+    let routesHaveBeenValidated = false;
+
     const Provider = ({ children }: PropsWithChildren<{}>) => {
       const appThemeApi = useMemo(
         () => AppThemeSelector.createWithStorage(this.themes),
@@ -245,8 +251,6 @@ export class AppManager implements BackstageApp {
             },
           });
 
-          validateRoutes(result.routePaths, result.routeParents);
-
           // TODO(Rugvip): Restructure the public API so that we can get an immediate view of
           //               the app, rather than having to wait for the provider to render.
           //               For now we need to push the additional plugins we find during
@@ -259,6 +263,15 @@ export class AppManager implements BackstageApp {
           return result;
         }, [children]);
 
+      if (!routesHaveBeenValidated) {
+        routesHaveBeenValidated = true;
+        validateRouteParameters(routePaths, routeParents);
+        validateRouteBindings(
+          this.routeBindings,
+          this.plugins as Iterable<BackstagePlugin<any, any>>,
+        );
+      }
+
       const loadedConfig = useConfigLoader(
         this.configLoader,
         this.components,
@@ -318,7 +331,7 @@ export class AppManager implements BackstageApp {
                 routePaths={routePaths}
                 routeParents={routeParents}
                 routeObjects={routeObjects}
-                routeBindings={generateBoundRoutes(this.bindRoutes)}
+                routeBindings={this.routeBindings}
                 basePath={getBasePath(loadedConfig.api)}
               >
                 {children}

packages/core-app-api/src/routing/RoutingProvider.test.tsx

@@ -39,7 +39,7 @@ import {
   routeParentCollector,
   routeObjectCollector,
 } from './collectors';
-import { validateRoutes } from './validation';
+import { validateRouteParameters } from './validation';
 import { RouteResolver } from './RouteResolver';
 import { AnyRouteRef, RouteFunc } from './types';
 import { AppContextProvider } from '../app/AppContext';
@@ -323,7 +323,7 @@ describe('discovery', () => {
       },
     });
 
-    expect(() => validateRoutes(routePaths, routeParents)).toThrow(
+    expect(() => validateRouteParameters(routePaths, routeParents)).toThrow(
       'Parameter :id is duplicated in path /foo/:id/bar/:id',
     );
   });

packages/core-app-api/src/routing/validation.ts

@@ -14,9 +14,16 @@
  * limitations under the License.
  */
 
+import {
+  BackstagePlugin,
+  ExternalRouteRef,
+  RouteRef,
+  SubRouteRef,
+} from '@backstage/core-plugin-api';
 import { AnyRouteRef } from './types';
 
-export function validateRoutes(
+// Validates that there is no duplication of route parameter names
+export function validateRouteParameters(
   routePaths: Map<AnyRouteRef, string>,
   routeParents: Map<AnyRouteRef, AnyRouteRef | undefined>,
 ) {
@@ -54,3 +61,30 @@ export function validateRoutes(
     }
   }
 }
+
+// Validates that all non-optional external routes have been bound
+export function validateRouteBindings(
+  routeBindings: Map<ExternalRouteRef, RouteRef | SubRouteRef>,
+  plugins: Iterable<BackstagePlugin<{}, Record<string, ExternalRouteRef>>>,
+) {
+  for (const plugin of plugins) {
+    if (!plugin.externalRoutes) {
+      continue;
+    }
+
+    for (const [name, externalRouteRef] of Object.entries(
+      plugin.externalRoutes,
+    )) {
+      if (externalRouteRef.optional) {
+        continue;
+      }
+
+      if (!routeBindings.has(externalRouteRef)) {
+        throw new Error(
+          `External route '${name}' of the '${plugin.getId()}' plugin must be bound to a target route. ` +
+            'See https://backstage.io/link?bind-routes for details.',
+        );
+      }
+    }
+  }
+}