Merge pull request #22257 from backstage/collect-legacy-routes-throws

collectLegacyRoutes throws error in case invalid element is found

.changeset/wet-emus-work.md

@@ -0,0 +1,5 @@
+---
+'@backstage/core-compat-api': patch
+---
+
+collectLegacyRoutes throws in case invalid <Route /> element is found

packages/core-compat-api/src/collectLegacyRoutes.test.tsx

@@ -27,7 +27,7 @@ import { PuppetDbPage } from '@backstage/plugin-puppetdb';
 import { StackstormPage } from '@backstage/plugin-stackstorm';
 import { ScoreBoardPage } from '@oriflame/backstage-plugin-score-card';
 import React, { Fragment } from 'react';
-import { Route, Routes } from 'react-router-dom';
+import { Navigate, Route, Routes } from 'react-router-dom';
 
 import { collectLegacyRoutes } from './collectLegacyRoutes';
 // eslint-disable-next-line @backstage/no-relative-monorepo-imports
@@ -279,4 +279,98 @@ describe('collectLegacyRoutes', () => {
       screen.findByText('plugins: test'),
     ).resolves.toBeInTheDocument();
   });
+
+  it('should throw if invalid Route has been detected', async () => {
+    const plugin = createPlugin({
+      id: 'test',
+    });
+    const routeRef = createRouteRef({ id: 'test' });
+    const Page = plugin.provide(
+      createRoutableExtension({
+        name: 'Test',
+        mountPoint: routeRef,
+        component: async () => () => {
+          const app = useApp();
+          return <div>plugins: {app.getPlugins().map(p => p.getId())}</div>;
+        },
+      }),
+    );
+
+    expect(() =>
+      collectLegacyRoutes(
+        <FlatRoutes>
+          <Route path="/" element={<Page />} />
+          <Route path="/" element={<Page />} />
+          <div />
+        </FlatRoutes>,
+      ),
+    ).toThrow(
+      /Invalid element inside FlatRoutes, expected Route but found div./,
+    );
+  });
+
+  it('should throw if invalid element has been detected', async () => {
+    const plugin = createPlugin({
+      id: 'test',
+    });
+    const routeRef = createRouteRef({ id: 'test' });
+    const Page = plugin.provide(
+      createRoutableExtension({
+        name: 'Test',
+        mountPoint: routeRef,
+        component: async () => () => {
+          const app = useApp();
+          return <div>plugins: {app.getPlugins().map(p => p.getId())}</div>;
+        },
+      }),
+    );
+
+    expect(() =>
+      collectLegacyRoutes(
+        <FlatRoutes>
+          <Route path="/" element={<Page />} />a string
+        </FlatRoutes>,
+      ),
+    ).toThrow(
+      /Invalid element inside FlatRoutes, expected Route but found element of type string./,
+    );
+  });
+
+  it('should throw if <Route /> has no path', async () => {
+    const plugin = createPlugin({
+      id: 'test',
+    });
+    const routeRef = createRouteRef({ id: 'test' });
+    const Page = plugin.provide(
+      createRoutableExtension({
+        name: 'Test',
+        mountPoint: routeRef,
+        component: () =>
+          Promise.resolve(() => {
+            const app = useApp();
+            return <div>plugins: {app.getPlugins().map(p => p.getId())}</div>;
+          }),
+      }),
+    );
+
+    expect(() =>
+      collectLegacyRoutes(
+        <FlatRoutes>
+          <Route element={<Page />} />
+        </FlatRoutes>,
+      ),
+    ).toThrow(/Route element inside FlatRoutes had no path prop value given/);
+  });
+
+  it('should throw if element cannot be converted', async () => {
+    expect(() =>
+      collectLegacyRoutes(
+        <FlatRoutes>
+          <Route element={<Navigate to="/somewhere" />} />
+        </FlatRoutes>,
+      ),
+    ).toThrow(
+      /Route with path undefined has en element that can not be converted/,
+    );
+  });
 });

packages/core-compat-api/src/collectLegacyRoutes.tsx

@@ -175,10 +175,16 @@ export function collectLegacyRoutes(
     flatRoutesElement.props.children,
     (route: ReactNode) => {
       // TODO(freben): Handle feature flag and permissions framework wrapper elements
-      if (!React.isValidElement(route) || route.type !== Route) {
-        return;
+      if (!React.isValidElement(route)) {
+        throw new Error(
+          `Invalid element inside FlatRoutes, expected Route but found element of type ${typeof route}.`,
+        );
+      }
+      if (route.type !== Route) {
+        throw new Error(
+          `Invalid element inside FlatRoutes, expected Route but found ${route.type}.`,
+        );
       }
-
       const routeElement = route.props.element;
       const path: string | undefined = route.props.path;
       const plugin = getComponentData<LegacyBackstagePlugin>(
@@ -189,8 +195,16 @@ export function collectLegacyRoutes(
         routeElement,
         'core.mountPoint',
       );
-      if (!plugin || !path) {
-        return;
+      if (!plugin) {
+        throw new Error(
+          // TODO(vinzscam): add See <link-to-app-migration-docs> for more info
+          `Route with path ${path} has en element that can not be converted as it does not belong to a plugin. Make sure that the top-level React element of the element prop is an extension from a Backstage plugin, or remove the Route completely.`,
+        );
+      }
+      if (path === undefined) {
+        throw new Error(
+          `Route element inside FlatRoutes had no path prop value given`,
+        );
       }
 
       const extensions = getPluginExtensions(plugin);