fix: Improve the AppRouter component no custom 404 route handling (#137)

* Improve the AppRouter component no custom 404 route handling

* Added changeset file

.changeset/cool-onions-compete.md

@@ -0,0 +1,6 @@
+---
+"@squide/react-router": patch
+"@squide/firefly": patch
+---
+
+Improved the 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;
  }