fix: AppRouter component (#131)

* Updated the AppRouter API to explicitly define the ReactRouter as a children * Some fixes * Fixed logging issues and a racing condition with protected date and deferred data * Added a signal to onLoadPublicData and onLoadProtectedData handlers * Fixed AppRouter tests * Updated documentation with the new AppRouter props * Documentation touch ups * Removed useRefState * Fixing CI * AppRouter children now accept a function returning a ReactNode * Added changeset files * Reverted AppRouter children function to ReactElement
gsoft-inc · Jan 16, 2024 · 7caa44b · 7caa44b
1 parent 10e340e
commit 7caa44b
Show file tree

Hide file tree

Showing 27 changed files with 2,538 additions and 1,592 deletions.
diff --git a/.changeset/afraid-mirrors-hunt.md b/.changeset/afraid-mirrors-hunt.md
@@ -0,0 +1,5 @@
+---
+"@squide/core": minor
+---
+
+- Added a `usePlugin` hook.
diff --git a/.changeset/polite-geckos-occur.md b/.changeset/polite-geckos-occur.md
@@ -0,0 +1,72 @@
+---
+"@squide/firefly": major
+---
+
+- The `AppRouter` component now requires to define a `RouterProvider` as a child. This change has been made to provide more flexibility on the consumer side about the definition of the React Router router.
+
+Before:
+
+```tsx
+<AppRouter
+    fallbackElement={...}
+    errorElement={...}
+    waitForMsw={...}
+ />
+```
+
+Now:
+
+```tsx
+<AppRouter
+    fallbackElement={...}
+    errorElement={...}
+    waitForMsw={...}
+>
+    {(routes, providerProps) => (
+        <RouterProvider router={createBrowserRouter(routes)} {...providerProps} />
+    )}
+</AppRouter>
+```
+
+- When in development and using React strict mode, the public and protected handler can be called twice. This issue highlighted that the `AppRouter` component doesn't equipe correctly the handlers to dispose of previous HTTP requests if they are called multiple times because of re-renders. Therefore, the handlers now receives an [AbortSignal](https://developer.mozilla.org/en-US/docs/Web/API/AbortSignal) that should be forwared to the HTTP client initiating the fetch request.
+- The fix also requires the consumer to provide new properties (`isPublicDataLoaded` and `isProtectedDataLoaded`) indicating whether or not the public and/or protected data has been loaded.
+
+```tsx
+async function fetchPublicData(setFeatureFlags: (featureFlags: FeatureFlags) => void, signal: AbortSignal) {
+    try {
+        const response = await fetch("/api/feature-flags", {
+            signal
+        });
+
+        if (response.ok) {
+            const data = await response.json();
+
+            setFeatureFlags(data);
+        }
+    } catch (error: unknown) {
+        if (!signal.aborted) {
+            throw error;
+        }
+    }
+}
+
+const [featureFlags, setFeatureFlags] = useState<FeatureFlags>();
+
+const handleLoadPublicData = useCallback((signal: AbortSignal) => {
+    return fetchPublicData(setFeatureFlags, signal);
+}, []);
+
+<AppRouter
+    onLoadPublicData={handleLoadPublicData}
+    isPublicDataLoaded={!!featureFlags}
+    fallbackElement={...}
+    errorElement={...}
+    waitForMsw={...}
+>
+    {(routes, providerProps) => (
+        <RouterProvider router={createBrowserRouter(routes)} {...providerProps} />
+    )}
+</AppRouter>
+```
+
+- Fixed an issue where the deferred registrations could be completed before the protected data has been loaded.
diff --git a/.changeset/six-teachers-hunt.md b/.changeset/six-teachers-hunt.md
@@ -0,0 +1,17 @@
+---
+"@squide/react-router": major
+---
+
+- To be consistent with the other API of Squide, the `useNavigationItems` hook  now accept an object literal of options rather than an optional `menuId` argument.
+
+Before:
+
+```tsx
+const items = useNavigationItems("my-custom-menu");
+```
+
+Now:
+
+```tsx
+const items = useNavigationItems({ menuId: "my-custom-menu" });
+```
diff --git a/docs/getting-started/create-host.md b/docs/getting-started/create-host.md
@@ -114,18 +114,23 @@ root.render(
 );
 ```
 
-Then, render the [AppRouter](../reference/routing/appRouter.md) component. The `AppRouter` component will render a React Router [browser instance](https://reactrouter.com/en/main/routers/create-browser-router) configured with the registered routes:
+Then, render the [AppRouter](../reference/routing/appRouter.md) component to define a React Router [browser instance](https://reactrouter.com/en/main/routers/create-browser-router) configured with the registered routes:
 
 ```tsx host/src/App.tsx
 import { AppRouter } from "@squide/firefly";
+import { RouterProvider, createBrowserRouter } from "react-router-dom";
 
 export function App() {
     return (
         <AppRouter
             fallbackElement={<div>Loading...</div>}
             errorElement={<div>An error occured!</div>}
             waitForMsw={false}
-        />
+        >
+            {(routes, providerProps) => (
+                <RouterProvider router={createBrowserRouter(routes)} {...providerProps} />
+            )}
+        </AppRouter>
     );
 }
 ```
@@ -143,7 +148,7 @@ import {
     isNavigationLink,
     type RenderItemFunction,
     type RenderSectionFunction
-} from "@squide/react-router";
+} from "@squide/firefly";
 
 const renderItem: RenderItemFunction = (item, index, level) => {
     // To keep thing simple, this sample doesn't support nested navigation items.
@@ -153,6 +158,8 @@ const renderItem: RenderItemFunction = (item, index, level) => {
         return null;
     }
 
+    const { label, linkProps, additionalProps } = item;
+
     return (
         <li key={`${level}-${index}`}>
             <Link {...linkProps} {...additionalProps}>

diff --git a/docs/guides/add-authentication.md b/docs/guides/add-authentication.md
@@ -309,36 +309,63 @@ export const requestHandlers: HttpHandler[] = [
 
 Then, update the host application `App` component to load the session when a user navigate to a protected page for the first time:
 
-```tsx !#15,21 host/src/App.tsx
+```tsx !#20,22,31,33-35,39-40 host/src/App.tsx
 import { AppRouter } from "@squide/firefly";
 import type { Session } from "@sample/shared";
 import { sessionManager } from "./session.ts";
+import { RouterProvider, createBrowserRouter } from "react-router-dom";
 
-async function handleLoadProtectedData() {
-    const response = await fetch("/api/session");
-    const data = await response.json();
+async function fetchProtectedData(setIsSessionLoaded: (isLoaded: boolean) => void,signal: AbortSignal) {
+    try {
+        const response = await fetch("/api/session", {
+            signal
+        });
 
-    const session: Session = {
-        user: {
-            name: data.username
-        }
-    };
+        const data = await response.json();
 
-    sessionManager.setSession(session);
+        const session: Session = {
+            user: {
+                name: data.username
+            }
+        };
+
+        sessionManager.setSession(session);
+
+        setIsSessionLoaded(true);
+    } catch (error: unknown) {
+        if (!signal.aborted) {
+            throw error;
+        }
+    }
 }
 
 export function App() {
+    const [isSessionLoaded, setIsSessionLoaded] = useState(false);
+
+    const handleLoadProtectedData = useCallback((signal: AbortSignal) => {
+        return fetchProtectedData(setIsSessionLoaded, signal);
+    }, []);
+
     return (
         <AppRouter
             onLoadProtectedData={handleLoadProtectedData}
+            isProtectedDataLoaded={isSessionLoaded}
             fallbackElement={<div>Loading...</div>}
             errorElement={<div>An error occured!</div>}
             waitForMsw={true}
-        />
+        >
+            {(routes, providerProps) => (
+                <RouterProvider router={createBrowserRouter(routes)} {...providerProps} />
+            )}
+        </AppRouter>
     );
 }
 ```
 
+!!!info
+Since the `sessionManager` doesn't trigger a re-render, a `isSessionLoaded` state value is added to trigger a re-render when the session has been loadded.
+!!!
+
 ## Add an authentication boundary
 
 Next, create a new React Router authentication boundary component using the [useIsAuthenticated](../reference/session/useIsAuthenticated.md) hook:
@@ -435,7 +462,7 @@ export const requestHandlers: HttpHandler[] = [
 
 Then, introduce a new `AuthenticatedLayout` displaying the name of the logged-in user along with a logout button:
 
-```tsx !#39,41-58,70,73 host/src/AuthenticatedLayout.tsx
+```tsx !#41,43-60,72,75 host/src/AuthenticatedLayout.tsx
 import { useCallback, type ReactNode, type MouseEvent, type HTMLButtonElement } from "react";
 import { Link, Outlet, navigate } from "react-router-dom";
 import { 
@@ -455,6 +482,8 @@ const renderItem: RenderItemFunction = (item, index, level) => {
         return null;
     }
 
+    const { label, linkProps, additionalProps } = item;
+
     return (
         <li key={`${level}-${index}`}>
             <Link {...linkProps} {...additionalProps}>