fix: Fixing deferred registrations with protected data (#135)

* Fixed deferred registrations with protected loaded

* Added changeset files

* Removed dead code

.changeset/slimy-pumas-breathe.md

@@ -0,0 +1,5 @@
+---
+"@squide/firefly": patch
+---
+
+Fixing a remaining issue with deferred registrations that depends on protected data.

.changeset/thin-shoes-call.md

@@ -0,0 +1,5 @@
+---
+"@squide/react-router": patch
+---
+
+Internal changes to `useRouteMatch` and `useIsRouteProtected`.

docs/reference/default.md

@@ -41,7 +41,7 @@ toc:
 - [ManagedRoutes](./routing/ManagedRoutes.md)
 - [useRenderedNavigationItems](./routing/useRenderedNavigationItems.md)
 - [useRouteMatch](./routing/useRouteMatch.md)
-- [useIsRouteMatchProtected](./routing/useIsRouteMatchProtected.md)
+- [useIsRouteProtected](./routing/useIsRouteProtected.md)
 
 ### Logging
 

docs/reference/routing/useIsRouteProtected.md

@@ -0,0 +1,37 @@
+---
+order: -100
+toc:
+    depth: 2-3
+---
+
+# useIsRouteProtected
+
+Determine whether or not a route is considered as `protected`.
+
+> To take advantage of this hook, make sure to add a [$visibility hint](../runtime/runtime-class.md#register-a-public-route) to your public pages.
+
+## Reference
+
+```ts
+const isProtected = useIsRouteProtected(route)
+```
+
+### Parameters
+
+- `route`: A `Route` object.
+
+### Returns
+
+A `boolean` value indicating whether or not the matching route is `protected`.
+
+## Usage
+
+```ts
+import { useLocation } from "react-router-dom";
+import { useIsRouteProtected, useRouteMatch } from "@squide/firefly";
+
+const location = useLocation();
+const route = useRouteMatch(location);
+
+const isActiveRouteProtected = useIsRouteProtected(route);
+```

docs/reference/routing/useRouteMatch.md

@@ -16,10 +16,14 @@ const match = useRouteMatch(locationArg)
 ### Parameters
 
 - `locationArg`: The location to match the route paths against.
+- `options`: An optional object literal of options:
+    - `throwWhenThereIsNoMatch`: Whether or not to throw an `Error` if no route match `locationArg`.
 
 ### Returns
 
-A `Route` object if there's a matching route, otherwise an `Error` is thrown.
+A `Route` object if there's a matching route, otherwise if `throwWhenThereIsNoMatch` is enabled and no route match the given location, an `Error` is thrown.
+
+If `throwWhenThereIsNoMatch` is disabled and there's no route matching `locationArg`, `undefined` is returned.
 
 ## Usage
 
@@ -30,6 +34,7 @@ import { useLocation } from "react-router-dom";
 import { useRouteMatch } from "@squide/firefly";
 
 const location = useLocation();
+
 const activeRoute = useRouteMatch(location);
 ```
 

packages/firefly/src/AppRouter.tsx

@@ -1,6 +1,6 @@
 import { isNil, useLogOnceLogger } from "@squide/core";
 import { useIsMswStarted } from "@squide/msw";
-import { useIsRouteMatchProtected, useRoutes, type Route } from "@squide/react-router";
+import { 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";
@@ -154,7 +154,8 @@ export function BootstrappingRoute(props: BootstrappingRouteProps) {
     useLoadPublicData(areModulesRegistered, areModulesReady, isMswStarted, isPublicDataLoaded, onLoadPublicData);
 
     // Only throw when there's no match if the modules has been registered, otherwise it's expected that there are no registered routes.
-    const isActiveRouteProtected = useIsRouteMatchProtected(location, { throwWhenThereIsNoMatch: areModulesReady });
+    const activeRoute = useRouteMatch(location, { throwWhenThereIsNoMatch: areModulesReady });
+    const isActiveRouteProtected = useIsRouteProtected(activeRoute);
 
     // Try to load the protected data if an handler is defined.
     useLoadProtectedData(areModulesRegistered, areModulesReady, isMswStarted, isActiveRouteProtected, isProtectedDataLoaded, onLoadProtectedData);
@@ -170,7 +171,7 @@ export function BootstrappingRoute(props: BootstrappingRouteProps) {
         }
     }, [areModulesRegistered, areModulesReady, isMswStarted, isPublicDataLoaded, isProtectedDataLoaded, isActiveRouteProtected, onCompleteRegistrations]);
 
-    if (!areModulesReady || !isMswStarted || !isPublicDataLoaded || (isActiveRouteProtected && !isProtectedDataLoaded)) {
+    if (!areModulesReady || !isMswStarted || !activeRoute || !isPublicDataLoaded || (isActiveRouteProtected && !isProtectedDataLoaded)) {
         return fallbackElement;
     }
 
@@ -221,6 +222,19 @@ export function AppRouter(props: AppRouterProps) {
     }, [errorElement]);
 
     return useMemo(() => {
+        // 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")
+            });
+        }
+
         return renderRouterProvider([
             {
                 element: (

packages/firefly/src/NoMatchRouteFallback.tsx

@@ -0,0 +1,18 @@
+export function NoMatchRouteFallback() {
+    return (
+        <div>
+            <h1>404 not found</h1>
+            <p>This page has been dynamically added by Squide to fix an issue with the <code>AppRouter</code> component. <strong>Please replace this page in your application by a <a href="https://reactrouter.com/en/main/start/tutorial#handling-not-found-errors">custom match page</a>.</strong></p>
+            <p>The code should be like the following:</p>
+            <pre style={{ backgroundColor: "rgb(243, 245, 249)", color: "rgb(21, 25, 40)", padding: "4px 10px", border: "1px solid rgb(225, 229, 239)" }}>
+{`runtime.registerRoute({
+    $visibility: "public",
+    path: "*",
+    lazy: import("./NoMatchPage.tsx")
+});`}
+            </pre>
+        </div>
+    )
+}
+
+export const Component = NoMatchRouteFallback;

packages/firefly/src/index.ts

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

packages/react-router/src/index.ts

@@ -2,7 +2,7 @@ export * from "./navigationItemRegistry.ts";
 export * from "./outlets.ts";
 export * from "./reactRouterRuntime.ts";
 export * from "./routeRegistry.ts";
-export * from "./useIsRouteMatchProtected.ts";
+export * from "./useIsRouteProtected.ts";
 export * from "./useNavigationItems.ts";
 export * from "./useRenderedNavigationItems.tsx";
 export * from "./useRouteMatch.ts";

packages/react-router/src/useIsRouteProtected.ts

@@ -0,0 +1,24 @@
+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.
+        // 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.
+        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.
+        return true;
+    }
+
+    return route.$visibility === "protected";
+}

packages/react-router/src/useRouteMatch.ts

@@ -1,7 +1,11 @@
 import { matchRoutes } from "react-router-dom";
 import { useRoutes } from "./useRoutes.ts";
 
-export function useRouteMatch(locationArg: Partial<Location>) {
+export interface UseIsRouteMatchOptions {
+    throwWhenThereIsNoMatch?: boolean;
+}
+
+export function useRouteMatch(locationArg: Partial<Location>, { throwWhenThereIsNoMatch = true }: UseIsRouteMatchOptions = {}) {
     const routes = useRoutes();
 
     const matchingRoutes = matchRoutes(routes, locationArg) ?? [];
@@ -10,6 +14,10 @@ export function useRouteMatch(locationArg: Partial<Location>) {
         // When a route is nested, it also returns all the parts that constituate the whole route (for example the layouts and the boundaries).
         // We only want to know the visiblity of the actual route that has been requested, which is always the last entry.
         return matchingRoutes[matchingRoutes.length - 1]!.route;
+    } else {
+        if (throwWhenThereIsNoMatch) {
+            throw new Error(`[squide] There's no matching route for the location: "${locationArg.pathname}". Did you add routes to React Router without using the runtime.registerRoute() function?`);
+        }
     }
 
     return undefined;

samples/endpoints/local-module/src/register.tsx

@@ -71,6 +71,10 @@ function registerRoutes(runtime: FireflyRuntime, i18nextInstance: i18n): Deferre
     });
 
     return ({ featureFlags } = {}) => {
+        if (!runtime.getSession()) {
+            throw new Error("The deferred registratons are broken again as they are executed before the protected data has been loaded.");
+        }
+
         if (featureFlags?.featureA) {
             runtime.registerRoute({
                 path: "/feature-a",

samples/endpoints/remote-module/src/register.tsx

@@ -109,6 +109,10 @@ function registerRoutes(runtime: FireflyRuntime, i18nextInstance: i18n): Deferre
     });
 
     return ({ featureFlags } = {}) => {
+        if (!runtime.getSession()) {
+            throw new Error("The deferred registratons are broken again as they are executed before the protected data has been loaded.");
+        }
+
         if (featureFlags?.featureB) {
             runtime.registerRoute({
                 path: "/feature-b",

samples/endpoints/shell/src/AppRouter.tsx

@@ -6,6 +6,7 @@ import { useTranslation } from "react-i18next";
 import { RouterProvider, createBrowserRouter } from "react-router-dom";
 import { AppRouterErrorBoundary } from "./AppRouterErrorBoundary.tsx";
 import { i18NextInstanceKey } from "./i18next.ts";
+import { useRefState } from "./useRefState.tsx";
 
 export interface DeferredRegistrationData {
     featureFlags?: FeatureFlags;
@@ -55,6 +56,7 @@ async function fetchSubscription(signal: AbortSignal) {
 function fetchProtectedData(
     setSession: (session: Session) => void,
     setSubscription: (subscription: Subscription) => void,
+    setIsProtectedDataLoaded: (isProtectedDataLoaded: boolean) => void,
     signal: AbortSignal,
     logger: Logger
 ) {
@@ -70,6 +72,7 @@ function fetchProtectedData(
             logger.debug("[shell] %cSubscription is ready%c:", "color: white; background-color: green;", "", subscription);
 
             setSubscription(subscription);
+            setIsProtectedDataLoaded(true);
         })
         .catch((error: unknown) => {
             if (isApiError(error) && error.status === 401) {
@@ -92,7 +95,9 @@ function Loading() {
 
 export function AppRouter({ waitForMsw, sessionManager, telemetryService }: AppRouterProps) {
     const [featureFlags, setFeatureFlags] = useState<FeatureFlags>();
-    const [subscription, setSubscription] = useState<Subscription>();
+
+    const [subscriptionRef, setSubscription] = useRefState<Subscription>();
+    const [isProtectedDataLoaded, setIsProtectedDataLoaded] = useState(false);
 
     const logger = useLogger();
     const runtime = useRuntime();
@@ -112,7 +117,7 @@ export function AppRouter({ waitForMsw, sessionManager, telemetryService }: AppR
             changeLanguage(session.user.preferredLanguage);
         };
 
-        return fetchProtectedData(setSession, setSubscription, signal, logger);
+        return fetchProtectedData(setSession, setSubscription, setIsProtectedDataLoaded, signal, logger);
     }, [logger, sessionManager, changeLanguage]);
 
     const handleCompleteRegistrations = useCallback(() => {
@@ -124,7 +129,7 @@ export function AppRouter({ waitForMsw, sessionManager, telemetryService }: AppR
 
     return (
         <FeatureFlagsContext.Provider value={featureFlags}>
-            <SubscriptionContext.Provider value={subscription}>
+            <SubscriptionContext.Provider value={subscriptionRef.current!}>
                 <TelemetryServiceContext.Provider value={telemetryService}>
                     <FireflyAppRouter
                         fallbackElement={<Loading />}
@@ -133,7 +138,7 @@ export function AppRouter({ waitForMsw, sessionManager, telemetryService }: AppR
                         onLoadPublicData={handleLoadPublicData}
                         onLoadProtectedData={handleLoadProtectedData}
                         isPublicDataLoaded={!!featureFlags}
-                        isProtectedDataLoaded={!!sessionManager.getSession() && !!subscription}
+                        isProtectedDataLoaded={isProtectedDataLoaded}
                         onCompleteRegistrations={handleCompleteRegistrations}
                     >
                         {(routes, providerProps) => (

samples/endpoints/shell/src/useRefState.tsx

@@ -0,0 +1,13 @@
+import { RefObject, useCallback, useRef } from "react";
+
+export function useRefState<T>(initialValue?: T): [RefObject<T | undefined>, (newValue: T) => void] {
+    const valueRef = useRef<T | undefined>(initialValue);
+
+    const setValue = useCallback((newValue: T) => {
+        if (valueRef.current !== newValue) {
+            valueRef.current = newValue;
+        }
+    }, [valueRef]);
+
+    return [valueRef, setValue];
+}