Improve the AppRouter component no custom 404 route handling

packages/firefly/src/AppRouter.tsx

@@ -1,6 +1,6 @@
 import { isNil, useLogOnceLogger } from "@squide/core";
 import { useIsMswStarted } from "@squide/msw";
-import { useIsRouteProtected, useRouteMatch, useRoutes, type Route } from "@squide/react-router";
+import { findRouteByPath, useIsRouteProtected, useRouteMatch, useRoutes, type Route } from "@squide/react-router";
 import { useAreModulesReady, useAreModulesRegistered } from "@squide/webpack-module-federation";
 import { cloneElement, useCallback, useEffect, useMemo, type ReactElement } from "react";
 import { ErrorBoundary, useErrorBoundary } from "react-error-boundary";
@@ -225,14 +225,11 @@ export function AppRouter(props: AppRouterProps) {
         // HACK:
         // When there's a direct hit on a deferred route, since the route has not been registered yet (because it's a deferred registration),
         // the React Router router instance doesn't know about that route and will therefore fallback to the no match route.
-        // If there's no custom no match route defined with path="*", React Router will not even bother trying to render a route and will defer to
-        // it's default no match route, which breaks the AppRouter logic.
-        // To circumvent this issue, if the application doesn't register a no match route, Squide add one by default.
-        if (!routes.some(x => x.path === "*")) {
-            routes.push({
-                path: "*",
-                lazy: () => import("./NoMatchRouteFallback.tsx")
-            });
+        // If there's isn't a custom no match route defined with path="*", React Router will fallback it's default no match router instead
+        // of rendering a route which will break the AppRouter component.
+        // To circumvent this issue, if the application doesn't register a custom no match route, an Error is thrown.
+        if (areModulesRegistered && !findRouteByPath(routes, "*")) {
+            throw new Error("For the AppRouter component to work properly, the application must be a define a custom no match router. For additional information, refer to: https://reactrouter.com/en/main/start/tutorial#handling-not-found-errors.");
         }
 
         return renderRouterProvider([

packages/firefly/src/index.ts

@@ -4,7 +4,5 @@ export * from "@squide/react-router";
 export * from "@squide/webpack-module-federation";
 
 export * from "./AppRouter.tsx";
-export * from "./NoMatchRouteFallback.tsx";
-
 export * from "./fireflyRuntime.tsx";
 

packages/firefly/tests/AppRouter.test.tsx

@@ -56,10 +56,9 @@ beforeEach(() => {
 test("when no data handlers are provided, msw is disabled, there's no deferred registrations, and modules are not registered yet, render the fallback", async () => {
     const runtime = new FireflyRuntime();
 
-    // Must add at least a route otherwise useRouteMatchProtected will throw.
     runtime.registerRoute({
         path: "*",
-        element: <div>A wildcard route</div>
+        element: <div>A custom no match route</div>
     }, {
         hoist: true
     });
@@ -77,10 +76,9 @@ test("when no data handlers are provided, msw is disabled, there's no deferred r
 test("when no data handlers are provided, msw is disabled, there's no deferred registrations, and modules are registered, render the router", async () => {
     const runtime = new FireflyRuntime();
 
-    // Must add at least a route otherwise useRouteMatchProtected will throw.
     runtime.registerRoute({
         path: "*",
-        element: <div>A wildcard route</div>
+        element: <div>A custom no match route</div>
     }, {
         hoist: true
     });
@@ -104,10 +102,9 @@ test("when no data handlers are provided, msw is disabled, there's no deferred r
 test("when no data handlers are provided, msw is disabled, modules are registered but there's uncompleted deferred registrations, render the fallback", async () => {
     const runtime = new FireflyRuntime();
 
-    // Must add at least a route otherwise useRouteMatchProtected will throw.
     runtime.registerRoute({
         path: "*",
-        element: <div>A wildcard route</div>
+        element: <div>A custom no match route</div>
     }, {
         hoist: true
     });
@@ -124,11 +121,9 @@ test("when no data handlers are provided, msw is disabled, modules are registere
 test("when a onLoadPublicData handler is provided and the public data is not loaded, render the fallback element", async () => {
     const runtime = new FireflyRuntime();
 
-    // Must add at least a route otherwise useRouteMatchProtected will throw.
     runtime.registerRoute({
-        $visibility: "public",
         path: "*",
-        element: <div>A wildcard route</div>
+        element: <div>A custom no match route</div>
     }, {
         hoist: true
     });
@@ -148,6 +143,13 @@ test("when a onLoadPublicData handler is provided and the public data is not loa
 test("when a onLoadPublicData handler is provided and the public data is loaded, render the router", async () => {
     const runtime = new FireflyRuntime();
 
+    runtime.registerRoute({
+        path: "*",
+        element: <div>A custom no match route</div>
+    }, {
+        hoist: true
+    });
+
     runtime.registerRoute({
         $visibility: "public",
         index: true,
@@ -178,10 +180,9 @@ test("when a onLoadPublicData handler is provided and the public data is loaded,
 test("when a onLoadProtectedData handler is provided and the protected data is not loaded, render the fallback element", async () => {
     const runtime = new FireflyRuntime();
 
-    // Must add at least a route otherwise React Router complains the router has no routes.
     runtime.registerRoute({
         path: "*",
-        element: <div>A wildcard route</div>
+        element: <div>A custom no match route</div>
     }, {
         hoist: true
     });
@@ -201,6 +202,13 @@ test("when a onLoadProtectedData handler is provided and the protected data is n
 test("when a onLoadProtectedData handler is provided and the protected data is loaded, render the router", async () => {
     const runtime = new FireflyRuntime();
 
+    runtime.registerRoute({
+        path: "*",
+        element: <div>A custom no match route</div>
+    }, {
+        hoist: true
+    });
+
     runtime.registerRoute({
         index: true,
         element: <div data-testid="route">A route</div>
@@ -230,10 +238,9 @@ test("when a onLoadProtectedData handler is provided and the protected data is l
 test("when msw is enabled and msw is not started, render the fallback element", async () => {
     const runtime = new FireflyRuntime();
 
-    // Must add at least a route otherwise React Router complains the router has no routes.
     runtime.registerRoute({
         path: "*",
-        element: <div>A wildcard route</div>
+        element: <div>A custom no match route</div>
     }, {
         hoist: true
     });
@@ -250,6 +257,13 @@ test("when msw is enabled and msw is not started, render the fallback element",
 test("when msw is enabled and msw is started, render the router", async () => {
     const runtime = new FireflyRuntime();
 
+    runtime.registerRoute({
+        path: "*",
+        element: <div>A custom no match route</div>
+    }, {
+        hoist: true
+    });
+
     runtime.registerRoute({
         index: true,
         element: <div data-testid="route">A route</div>
@@ -271,6 +285,13 @@ test("when msw is enabled and msw is started, render the router", async () => {
 test("when a onCompleteRegistrations handler is provided and there's no deferred registrations, render the router", async () => {
     const runtime = new FireflyRuntime();
 
+    runtime.registerRoute({
+        path: "*",
+        element: <div>A custom no match route</div>
+    }, {
+        hoist: true
+    });
+
     runtime.registerRoute({
         index: true,
         element: <div data-testid="route">A route</div>
@@ -291,10 +312,9 @@ test("when a onCompleteRegistrations handler is provided and there's no deferred
 test("when a onCompleteRegistrations handler is provided and the deferred registrations are not completed, render the fallback element", async () => {
     const runtime = new FireflyRuntime();
 
-    // Must add at least a route otherwise React Router complains the router has no routes.
     runtime.registerRoute({
         path: "*",
-        element: <div>A wildcard route</div>
+        element: <div>A custom no match route</div>
     }, {
         hoist: true
     });
@@ -313,10 +333,9 @@ test("when a onCompleteRegistrations handler is provided and the deferred regist
 test("when a onCompleteRegistrations handler is provided and the deferred registrations are completed, render the router", async () => {
     const runtime = new FireflyRuntime();
 
-    // Must add at least a route otherwise React Router complains the router has no routes.
     runtime.registerRoute({
         path: "*",
-        element: <div>A wildcard route</div>
+        element: <div>A custom no match route</div>
     }, {
         hoist: true
     });
@@ -352,10 +371,9 @@ test("when a onCompleteRegistrations handler is provided and the deferred regist
 test("when a onCompleteRegistrations handler is provided and a onLoadPublicData handler is provided, do not complete the deferred registrations and render the route until the public date is loaded", async () => {
     const runtime = new FireflyRuntime();
 
-    // Must add at least a route otherwise React Router complains the router has no routes.
     runtime.registerRoute({
         path: "*",
-        element: <div>A wildcard route</div>
+        element: <div>A custom no match route</div>
     }, {
         hoist: true
     });
@@ -411,10 +429,9 @@ test("when a onCompleteRegistrations handler is provided and a onLoadPublicData
 test("when a onCompleteRegistrations handler is provided and a onLoadProtectedData handler is provided, do not complete the deferred registrations and render the route until the protected date is loaded", async () => {
     const runtime = new FireflyRuntime();
 
-    // Must add at least a route otherwise React Router complains the router has no routes.
     runtime.registerRoute({
         path: "*",
-        element: <div>A wildcard route</div>
+        element: <div>A custom no match route</div>
     }, {
         hoist: true
     });
@@ -470,10 +487,9 @@ test("when a onCompleteRegistrations handler is provided and a onLoadProtectedDa
 test("when a onCompleteRegistrations handler is provided, a onLoadPublicData handler and a onLoadProtectedData handler are provided, do not complete the deferred registrations and render the route until the public and protected date are loaded", async () => {
     const runtime = new FireflyRuntime();
 
-    // Must add at least a route otherwise React Router complains the router has no routes.
     runtime.registerRoute({
         path: "*",
-        element: <div>A wildcard route</div>
+        element: <div>A custom no match route</div>
     }, {
         hoist: true
     });
@@ -540,11 +556,9 @@ test("when an error occurs while loading the public data, render the error eleme
 
     const runtime = new FireflyRuntime();
 
-    // Must add at least a route otherwise useRouteMatchProtected will throw.
     runtime.registerRoute({
-        $visibility: "public",
         path: "*",
-        element: <div>A wildcard route</div>
+        element: <div>A custom no match route</div>
     }, {
         hoist: true
     });
@@ -569,10 +583,9 @@ test("when an error occurs while loading the protected data, render the error el
 
     const runtime = new FireflyRuntime();
 
-    // Must add at least a route otherwise useRouteMatchProtected will throw.
     runtime.registerRoute({
         path: "*",
-        element: <div>A wildcard route</div>
+        element: <div>A custom no match route</div>
     }, {
         hoist: true
     });
@@ -589,3 +602,13 @@ test("when an error occurs while loading the protected data, render the error el
 
     spy.mockRestore();
 });
+
+test("throw an error if no custom no match route are registered", async () => {
+    const runtime = new FireflyRuntime();
+
+    await registerLocalModules([() => {}], runtime);
+
+    expect(() => renderWithRuntime(runtime, createAppRouter({
+        waitForMsw: false
+    }))).toThrow(/For the AppRouter component to work properly, the application must be a define a custom no match router/);
+});

packages/react-router/src/findRouteByPath.ts

@@ -0,0 +1,19 @@
+import { Route } from "./routeRegistry.ts";
+
+export function findRouteByPath(routes: Route[], path: string): Route | undefined {
+    for (const route of routes.values()) {
+        if (route.path === path) {
+            return route;
+        }
+
+        if (route.children) {
+            const childRoute = findRouteByPath(route?.children, path);
+
+            if (childRoute) {
+                return childRoute;
+            }
+        }
+    }
+
+    return undefined;
+}

packages/react-router/src/index.ts

@@ -1,3 +1,4 @@
+export * from "./findRouteByPath.ts";
 export * from "./navigationItemRegistry.ts";
 export * from "./outlets.ts";
 export * from "./reactRouterRuntime.ts";

packages/react-router/src/useIsRouteProtected.ts

@@ -3,20 +3,20 @@ import { Route } from "./routeRegistry.ts";
 export function useIsRouteProtected(route?: Route) {
     if (!route) {
         // HACK:
-        // An unregistrered route will be considered as "protected" by default to facilitate the implementation of deferred routes.
+        // An unregistrered route is considered as "protected" by default to facilitate the implementation of deferred routes.
         // The issue is that when there's a direct hit on a deferred route, it cannot be determined whether or not a deferred route is public or protected
         // because the deferred route hasn't been registered yet (since it's a deferred route).
-        // If that deferred route depends on protected data, if we don't return "true" here, the deferred route will be registered without providing the protected data
-        // which will cause a runtime error.
+        // If that deferred route depends on protected data, if we don't return "true" here, the deferred route will be registered before the protected data
+        // is loaded which will probably cause a runtime error.
         return true;
     }
 
     if (route.path === "*") {
         // HACK:
-        // With the current AppRouter implementation, when there's a direct hit to a deferred route, since the route has not been registered yet to
-        // the React Router router instance, the router is trying to render the no match route which confuse this hook as it would return a boolean value
-        // for the visibility of the no match route (which is usually public) rather than the visiblity of the route asked by the consumer (which may be
-        // protected). To circumvent this issue, true is returned for no match route. Anyway, that would really make sense to direct hit the no match route.
+        // With the current AppRouter component implementation, when there's a direct hit to a deferred route, since the route has not been registered yet to
+        // the React Router router instance, the router is trying to render the no match route. Therefore this hook returns the a boolean value based
+        // on the visibility status of the no match route instead of the actual page request by the user.
+        // To circumvent this issue, "true" is returned for the no match route.
         return true;
     }