From 1cda1be30779d1a1d5d2e21eac043baff20c0f7e Mon Sep 17 00:00:00 2001 From: Patrick Lafrance Date: Wed, 17 Jan 2024 15:10:06 -0500 Subject: [PATCH] fix: Deferred routes with protected data are now working as expected and a default catch has been added to AppRouter for AbortSignal errors. (#133) * Fixed deferred routes with protected data * Added changeset files --- .changeset/fuzzy-pandas-scream.md | 7 ++ .changeset/hungry-peaches-exist.md | 5 ++ .changeset/sharp-rocks-poke.md | 5 ++ .changeset/smart-seas-think.md | 6 ++ docs/getting-started/create-host.md | 2 +- docs/guides/add-authentication.md | 30 +++---- docs/guides/fetch-initial-data.md | 72 +++++++---------- docs/guides/override-a-react-context.md | 2 +- docs/guides/setup-i18next.md | 32 ++++---- docs/guides/use-feature-flags.md | 52 +++++-------- docs/reference/routing/appRouter.md | 78 +++++++------------ .../routing/useIsRouteMatchProtected.md | 2 + .../src/federation/registerLocalModules.ts | 4 +- packages/firefly/src/AppRouter.tsx | 17 ++-- packages/firefly/tests/AppRouter.test.tsx | 18 ++--- .../src/useIsRouteMatchProtected.ts | 7 +- .../src/completeModuleRegistrations.ts | 2 +- .../src/registerRemoteModules.ts | 4 +- samples/endpoints/shell/src/AppRouter.tsx | 28 +++---- 19 files changed, 171 insertions(+), 202 deletions(-) create mode 100644 .changeset/fuzzy-pandas-scream.md create mode 100644 .changeset/hungry-peaches-exist.md create mode 100644 .changeset/sharp-rocks-poke.md create mode 100644 .changeset/smart-seas-think.md diff --git a/.changeset/fuzzy-pandas-scream.md b/.changeset/fuzzy-pandas-scream.md new file mode 100644 index 000000000..ceec6116a --- /dev/null +++ b/.changeset/fuzzy-pandas-scream.md @@ -0,0 +1,7 @@ +--- +"@squide/firefly": patch +--- + +- To prevent the consumer from always handling the AbortSignal error, a default catch has been added to the `AppRouter` component. The consumer can still handler the AbortSignal error if he needs to. + +- When a user makes a direct hit to a deferred route that depends on protected data, the protected data was undefined. The reason was that by default, an unregistered route was considered as a public route. The code has been updated to consider an uregistered route as a protected route. The upside is that deferred routes can now depends on protected data. The downside is that a public deferred route will trigger the loading of the protected data. As we don't expect to have public deferred route at the moment it doesn't seems like an issue. diff --git a/.changeset/hungry-peaches-exist.md b/.changeset/hungry-peaches-exist.md new file mode 100644 index 000000000..dc49fb8f7 --- /dev/null +++ b/.changeset/hungry-peaches-exist.md @@ -0,0 +1,5 @@ +--- +"@squide/webpack-module-federation": patch +--- + +The completeDeferredRegistrations function data was required instead of optionnal. It has been fixed. diff --git a/.changeset/sharp-rocks-poke.md b/.changeset/sharp-rocks-poke.md new file mode 100644 index 000000000..68e6b99b1 --- /dev/null +++ b/.changeset/sharp-rocks-poke.md @@ -0,0 +1,5 @@ +--- +"@squide/react-router": patch +--- + +The `useIsRouteMatchProtected` hook now returns `true` when the location match an unregistered route. diff --git a/.changeset/smart-seas-think.md b/.changeset/smart-seas-think.md new file mode 100644 index 000000000..6303e5957 --- /dev/null +++ b/.changeset/smart-seas-think.md @@ -0,0 +1,6 @@ +--- +"@squide/core": patch +--- + +The completeDeferredRegistrations function data was required instead of optionnal. It has been fixed. + diff --git a/docs/getting-started/create-host.md b/docs/getting-started/create-host.md index d72aeb666..e52a2b4bb 100644 --- a/docs/getting-started/create-host.md +++ b/docs/getting-started/create-host.md @@ -139,7 +139,7 @@ export function App() { Next, create a layout component to [render the navigation items](/reference/routing/useRenderedNavigationItems.md). In many applications, multiple pages often share a **common layout** that includes elements such as a navigation bar, a user profile menu, and a main content section. In a [React Router](https://reactrouter.com/en/main) application, this shared layout is commonly referred to as a `RootLayout`: -```tsx !#38,41 host/src/RootLayout.tsx +```tsx !#40,43 host/src/RootLayout.tsx import { Suspense } from "react"; import { Link, Outlet } from "react-router-dom"; import { diff --git a/docs/guides/add-authentication.md b/docs/guides/add-authentication.md index 423d76258..117d6d80d 100644 --- a/docs/guides/add-authentication.md +++ b/docs/guides/add-authentication.md @@ -309,34 +309,28 @@ 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 !#20,22,31,33-35,39-40 host/src/App.tsx +```tsx !#19,21,25,27-29,33-34 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 fetchProtectedData(setIsSessionLoaded: (isLoaded: boolean) => void,signal: AbortSignal) { - try { - const response = await fetch("/api/session", { - signal - }); + const response = await fetch("/api/session", { + signal + }); - const data = await response.json(); + const data = await response.json(); - const session: Session = { - user: { - name: data.username - } - }; + const session: Session = { + user: { + name: data.username + } + }; - sessionManager.setSession(session); + sessionManager.setSession(session); - setIsSessionLoaded(true); - } catch (error: unknown) { - if (!signal.aborted) { - throw error; - } - } + setIsSessionLoaded(true); } export function App() { diff --git a/docs/guides/fetch-initial-data.md b/docs/guides/fetch-initial-data.md index 7e488f9c8..8a8d181cf 100644 --- a/docs/guides/fetch-initial-data.md +++ b/docs/guides/fetch-initial-data.md @@ -88,26 +88,20 @@ Ensure that the shared project is configured as a [shared dependency](./add-a-sh Finally, open the host application code and update the `App` component to utilize the `AppRouter` component's [onLoadPublicData](../reference/routing/appRouter.md#load-public-data) handler. This handler will fetch the count and forward the retrieved value through `FetchCountContext`: -```tsx !#9,16-18,23,25-27,30,32,33 host/src/App.tsx +```tsx !#8,17,19-21,24,26-27 host/src/App.tsx import { useState, useCallback } from "react"; import { AppRouter } from "@squide/firefly"; import { FetchCountContext } from "@sample/shared"; import { RouterProvider, createBrowserRouter } from "react-router-dom"; async function fetchPublicData(setFetchCount: (fetchCount: number) => void, signal: AbortSignal) { - try { - const response = await fetch("/api/count", { - signal - }); - - const data = await response.json(); - - setFetchCount(data.count); - } catch (error: unknown) { - if (!signal.aborted) { - throw error; - } - } + const response = await fetch("/api/count", { + signal + }); + + const data = await response.json(); + + setFetchCount(data.count); } export function App() { @@ -289,46 +283,34 @@ Ensure that the shared project is configured as a [shared dependency](./add-a-sh Finally, open the host application code and update the `App` component to utilize the `AppRouter` component's `onLoadProtectedData` handler. This handler will fetch the user tenant subscription and forward the retrieved value through `SubscriptionContext`: -```tsx !#25,36-38,44,50-52,56,59,61 host/src/App.tsx +```tsx !#18,32,38-40,44,47,49 host/src/App.tsx import { useState, useCallback } from "react"; import { AppRouter } from "@squide/firefly"; import { FetchCountContext, SubscriptionContext, type Subscription } from "@sample/shared"; import { RouterProvider, createBrowserRouter } from "react-router-dom"; async function fetchPublicData(setFetchCount: (fetchCount: number) => void, signal: AbortSignal) { - try { - const response = await fetch("/api/count", { - signal - }); - - const data = await response.json(); - - setFetchCount(data.count); - } catch (error: unknown) { - if (!signal.aborted) { - throw error; - } - } + const response = await fetch("/api/count", { + signal + }); + + const data = await response.json(); + + setFetchCount(data.count); } async function fetchProtectedData(setSubscription: (subscription: Subscription) => void, signal: AbortSignal) { - try { - const response = await fetch("/api/subscription", { - signal - }); - - const data = await response.json(); - - const subscription: Subscription = { - status: data.status - }; - - setSubscription(subscription); - } catch (error: unknown) { - if (!signal.aborted) { - throw error; - } - } + const response = await fetch("/api/subscription", { + signal + }); + + const data = await response.json(); + + const subscription: Subscription = { + status: data.status + }; + + setSubscription(subscription); } export function App() { diff --git a/docs/guides/override-a-react-context.md b/docs/guides/override-a-react-context.md index 95d70edbe..bb6797f70 100644 --- a/docs/guides/override-a-react-context.md +++ b/docs/guides/override-a-react-context.md @@ -62,7 +62,7 @@ In the previous code samples, the host application provides a value for the `Bac Now, suppose the requirements change, and one remote module pages need to have a `red` background. The context can be overriden for the remote module by declaring a new provider directly in the routes registration: -```tsx !#9,11 remote-module/src/register.tsx +```tsx !#9 remote-module/src/register.tsx import type { ModuleRegisterFunction, FireflyRuntime } from "@squide/firefly"; import { BackgroundColorContext } from "@sample/shared"; import { ColoredPage } from "./ColoredPage.tsx"; diff --git a/docs/guides/setup-i18next.md b/docs/guides/setup-i18next.md index b1d54d951..328b8c71d 100644 --- a/docs/guides/setup-i18next.md +++ b/docs/guides/setup-i18next.md @@ -422,7 +422,7 @@ Hence, the strategy to select the displayed language should be as follow: To implement this strategy, use the [useChangeLanguage](../reference/i18next/useChangeLanguage.md) hook and the [onLoadProtectedData](../reference/routing/appRouter.md#load-protected-data) handler of the [AppRouter](../reference/routing/appRouter.md) component: -```tsx !#18,35,37-39,46-47 host/src/App.tsx +```tsx !#17,29,31-33,40-41 host/src/App.tsx import { AppRouter } from "@squide/firefly"; import { useChangeLanguage, useI18nextInstance } from "@squide/i18next"; import { useCallback } from "react"; @@ -430,24 +430,18 @@ import { useTranslation } from "react-i18next"; import { RouterProvider, createBrowserRouter } from "react-router-dom"; async function fetchProtectedData(changeLanguage: (language: string) => void, setIsSessionLoaded: (isLoaded: boolean) => void, signal: AbortSignal) { - try { - const response = await fetch("/api/session", { - signal - }); - - if (response.ok) { - const session = await response.json(); - - // When the session has been retrieved, change the displayed language to match - // the preferred language setting. - changeLanguage(session.preferredLanguage); - - setIsSessionLoaded(true); - } - } catch (error: unknown) { - if (!signal.aborted) { - throw error; - } + const response = await fetch("/api/session", { + signal + }); + + if (response.ok) { + const session = await response.json(); + + // When the session has been retrieved, change the displayed language to match + // the preferred language setting. + changeLanguage(session.preferredLanguage); + + setIsSessionLoaded(true); } } diff --git a/docs/guides/use-feature-flags.md b/docs/guides/use-feature-flags.md index 69547e7d0..3cce24618 100644 --- a/docs/guides/use-feature-flags.md +++ b/docs/guides/use-feature-flags.md @@ -81,31 +81,25 @@ Ensure that the shared project is configured as a [shared dependency](./add-a-sh Finally, open the host application code and update the `App` component to utilize the `AppRouter` component's `onLoadPublicData` handler to fetch the feature flags data: -```tsx !#30-32,35,37-38 host/src/App.tsx +```tsx !#24-26,29,31-32 host/src/App.tsx import { useState, useCallback } from "react"; import { AppRouter } from "@squide/firefly"; import { FeatureFlagsContext, type FeatureFlags } from "@sample/shared"; import { RouterProvider, createBrowserRouter } from "react-router-dom"; async function fetchPublicData(setFeatureFlags: (featureFlags: FeatureFlags) => void, signal: AbortSignal) { - try { - const response = await fetch("/api/feature-flags", { - signal - }); + const response = await fetch("/api/feature-flags", { + signal + }); - const data = await response.json(); + const data = await response.json(); - const featureFlags: FeatureFlags = { - featureA: data.featureA, - featureB: data.featureB - }; + const featureFlags: FeatureFlags = { + featureA: data.featureA, + featureB: data.featureB + }; - setFeatureFlags(featureFlags); - } catch (error: unknown) { - if (!signal.aborted) { - throw error; - } - } + setFeatureFlags(featureFlags); } export function App() { @@ -210,28 +204,22 @@ export const register: ModuleRegisterFunction void, signal: AbortSignal) { - try { - const response = await fetch("/api/feature-flags"); - const data = await response.json(); - - const featureFlags: FeatureFlags = { - featureA: data.featureA, - featureB: data.featureB - }; - - setFeatureFlags(featureFlags); - } catch (error: unknown) { - if (!signal.aborted) { - throw error; - } - } + const response = await fetch("/api/feature-flags"); + const data = await response.json(); + + const featureFlags: FeatureFlags = { + featureA: data.featureA, + featureB: data.featureB + }; + + setFeatureFlags(featureFlags); } export function App() { diff --git a/docs/reference/routing/appRouter.md b/docs/reference/routing/appRouter.md index 2be45146a..566d179a8 100644 --- a/docs/reference/routing/appRouter.md +++ b/docs/reference/routing/appRouter.md @@ -127,7 +127,7 @@ The handler must return a [Promise](https://developer.mozilla.org/en-US/docs/Web The `isPublicDataLoaded` property should also be provided to indicate whether or not the initial public data loading is completed. -```tsx !#11,20-22,29,40,41 host/src/App.tsx +```tsx !#10,23,25-27,34-35 host/src/App.tsx import { useState, useCallback } from "react"; import { AppRouter } from "@squide/firefly"; import { Loading } from "./Loading.tsx"; @@ -136,20 +136,14 @@ import type { FeatureFlags } from "@sample/shared"; import { RouterProvider, createBrowserRouter } from "react-router-dom"; 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 response = await fetch("/api/feature-flags", { + signal + }); + + if (response.ok) { + const data = await response.json(); + + setFeatureFlags(data); } } @@ -188,7 +182,7 @@ The handler must return a [Promise](https://developer.mozilla.org/en-US/docs/Web The `isProtectedDataLoaded` property should also be provided to indicate whether or not the initial protected data loading is completed. -```tsx !#11,25-27,34,45,46 host/src/App.tsx +```tsx !#10,28,30-32,39-40 host/src/App.tsx import { useState, useCallback } from "react"; import { AppRouter } from "@squide/firefly"; import { Loading } from "./Loading.tsx"; @@ -197,25 +191,19 @@ import type { Session } from "@sample/shared"; import { RouterProvider, createBrowserRouter } from "react-router-dom"; async function fetchProtectedData(setSession: (session: Session) => void, signal: AbortSignal) { - try { - const response = await fetch("/api/session"), { - signal + const response = await fetch("/api/session"), { + signal + }); + + if (response.ok) { + const data = await response.json(); + + setSession({ + user: { + id: data.userId, + name: data.username + } }); - - if (response.ok) { - const data = await response.json(); - - setSession({ - user: { - id: data.userId, - name: data.username - } - }); - } - } catch (error: unknown) { - if (!signal.aborted) { - throw error; - } } } @@ -252,7 +240,7 @@ Don't forget to forward the [AbortSignal](https://developer.mozilla.org/en-US/do For more information about deferred registrations, refer to the [registerRemoteModules](../registration/registerRemoteModules.md#defer-the-registration-of-routes-or-navigation-items) and [completeModuleRegistrations](../registration/completeModuleRegistrations.md) documentation. -```tsx !#38-40,50 host/src/App.tsx +```tsx !#32-34,44 host/src/App.tsx import { useState, useCallback } from "react"; import { AppRouter } from "@squide/firefly"; import { completeModuleRegistrations } from "@squide/webpack-module-federation"; @@ -262,20 +250,14 @@ import type { FeatureFlags } from "@sample/shared"; import { RouterProvider, createBrowserRouter } from "react-router-dom"; async function fetchPublicData(setFeatureFlags: (featureFlags: FeatureFlags) => void, signal: AbortSignal) { - try { - const response = await fetch("/api/feature-flags", { - signal - }); + const response = await fetch("/api/feature-flags", { + signal + }); + + if (response.ok) { + const data = await response.json(); - if (response.ok) { - const data = await response.json(); - - setFeatureFlags(data); - } - } catch (error: unknown) { - if (!signal.aborted) { - throw error; - } + setFeatureFlags(data); } } diff --git a/docs/reference/routing/useIsRouteMatchProtected.md b/docs/reference/routing/useIsRouteMatchProtected.md index 0842d7dbb..d5d0c6560 100644 --- a/docs/reference/routing/useIsRouteMatchProtected.md +++ b/docs/reference/routing/useIsRouteMatchProtected.md @@ -25,6 +25,8 @@ const isProtected = useIsRouteMatchProtected(locationArg) A `boolean` value indicating whether or not the matching route is `protected`. 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`, `true` is returned. + ## Usage ### Using `useLocation` diff --git a/packages/core/src/federation/registerLocalModules.ts b/packages/core/src/federation/registerLocalModules.ts index c371bd692..4eefba81e 100644 --- a/packages/core/src/federation/registerLocalModules.ts +++ b/packages/core/src/federation/registerLocalModules.ts @@ -67,7 +67,7 @@ export class LocalModuleRegistry { return errors; } - async completeModuleRegistrations(runtime: TRuntime, data: TData) { + async completeModuleRegistrations(runtime: TRuntime, data?: TData) { const errors: LocalModuleRegistrationError[] = []; if (this.#registrationStatus === "none" || this.#registrationStatus === "in-progress") { @@ -144,7 +144,7 @@ export function registerLocalModules(runtime: TRuntime, data: TData) { +export function completeLocalModuleRegistrations(runtime: TRuntime, data?: TData) { return localModuleRegistry.completeModuleRegistrations(runtime, data); } diff --git a/packages/firefly/src/AppRouter.tsx b/packages/firefly/src/AppRouter.tsx index 00b32afb1..0f6ddb574 100644 --- a/packages/firefly/src/AppRouter.tsx +++ b/packages/firefly/src/AppRouter.tsx @@ -25,8 +25,9 @@ function useLoadPublicData(areModulesRegistered: boolean, areModulesReady: boole logger.debugOnce("loading-public-data", "[shell] Loading public data."); const abortController = new AbortController(); + const signal = abortController.signal; - const result = onLoadData(abortController.signal); + const result = onLoadData(signal); if (!isPromise(result)) { throw Error("[squide] An AppRouter onLoadPublicData handler must return a promise object."); @@ -38,7 +39,10 @@ function useLoadPublicData(areModulesRegistered: boolean, areModulesReady: boole logger.debugOnce("public-data-loaded", "[shell] Public data has been loaded."); }) .catch(error => { - showBoundary(error); + // Do not handle aborted requests. + if (!signal.aborted) { + showBoundary(error); + } }); return () => { @@ -63,8 +67,9 @@ function useLoadProtectedData(areModulesRegistered: boolean, areModulesReady: bo logger.debugOnce("loading-protected-data", `[shell] Loading protected data as "${location.pathname}" is a protected route.`); const abortController = new AbortController(); + const signal = abortController.signal; - const result = onLoadData(abortController.signal); + const result = onLoadData(signal); if (!isPromise(result)) { throw Error("[squide] An AppRouter onLoadProtectedData handler must return a promise object."); @@ -76,13 +81,15 @@ function useLoadProtectedData(areModulesRegistered: boolean, areModulesReady: bo logger.debugOnce("protected-data-loaded", "[shell] Protected data has been loaded."); }) .catch(error => { - showBoundary(error); + // Do not handle aborted requests. + if (!signal.aborted) { + showBoundary(error); + } }); return () => { abortController.abort(); }; - // } } else { // Prevent logging twice because of React strict mode. logger.debugOnce("is-a-public-route", `[shell] Not loading protected data as "${location.pathname}" is a public route.`); diff --git a/packages/firefly/tests/AppRouter.test.tsx b/packages/firefly/tests/AppRouter.test.tsx index 1fcc9de78..56a39121f 100644 --- a/packages/firefly/tests/AppRouter.test.tsx +++ b/packages/firefly/tests/AppRouter.test.tsx @@ -121,7 +121,7 @@ test("when no data handlers are provided, msw is disabled, modules are registere expect(await screen.findByTestId("loading")).toBeInTheDocument(); }); -test("when a onLoadPublicData handler is provided and the public data is not loaded, render the fallback", async () => { +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. @@ -175,7 +175,7 @@ test("when a onLoadPublicData handler is provided and the public data is loaded, expect(await screen.findByTestId("route")).toBeInTheDocument(); }); -test("when a onLoadProtectedData handler is provided and the protected data is not loaded, render the fallback", async () => { +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. @@ -227,7 +227,7 @@ test("when a onLoadProtectedData handler is provided and the protected data is l expect(await screen.findByTestId("route")).toBeInTheDocument(); }); -test("when msw is enabled and msw is not started, render the fallback", async () => { +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. @@ -288,7 +288,7 @@ test("when a onCompleteRegistrations handler is provided and there's no deferred expect(await screen.findByTestId("route")).toBeInTheDocument(); }); -test("when a onCompleteRegistrations handler is provided and the deferred registrations are not completed, render the fallback", async () => { +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. @@ -349,7 +349,7 @@ test("when a onCompleteRegistrations handler is provided and the deferred regist expect(await screen.findByTestId("deferred-route")).toBeInTheDocument(); }); -test("when a onCompleteRegistrations handler is provided and a onLoadPublicData handler is provided, do not complete the deferred registrations and render the route before the public date is loaded", async () => { +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. @@ -408,7 +408,7 @@ test("when a onCompleteRegistrations handler is provided and a onLoadPublicData expect(await screen.findByTestId("deferred-route")).toBeInTheDocument(); }); -test("when a onCompleteRegistrations handler is provided and a onLoadProtectedData handler is provided, do not complete the deferred registrations and render the route before the protected date is loaded", async () => { +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. @@ -467,7 +467,7 @@ test("when a onCompleteRegistrations handler is provided and a onLoadProtectedDa expect(await screen.findByTestId("deferred-route")).toBeInTheDocument(); }); -test("when a onCompleteRegistrations handler is provided and a onLoadPublicData handler is provided, and a onLoadProtectedData handler is provided, do not complete the deferred registrations and render the route before the public and protected date are loaded", async () => { +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. @@ -533,7 +533,7 @@ test("when a onCompleteRegistrations handler is provided and a onLoadPublicData expect(await screen.findByTestId("deferred-route")).toBeInTheDocument(); }); -test("when an error occurs while loading the public data, show the error element", async () => { +test("when an error occurs while loading the public data, render the error element", async () => { // An error log is expected because it will hit the ErrorBoundary, see: https://github.com/facebook/react/issues/11098. const spy = jest.spyOn(console, "error"); spy.mockImplementation(() => {}); @@ -562,7 +562,7 @@ test("when an error occurs while loading the public data, show the error element spy.mockRestore(); }); -test("when an error occurs while loading the protected data, show the error element", async () => { +test("when an error occurs while loading the protected data, render the error element", async () => { // An error log is expected because it will hit the ErrorBoundary, see: https://github.com/facebook/react/issues/11098. const spy = jest.spyOn(console, "error"); spy.mockImplementation(() => {}); diff --git a/packages/react-router/src/useIsRouteMatchProtected.ts b/packages/react-router/src/useIsRouteMatchProtected.ts index d022d9115..813be08b9 100644 --- a/packages/react-router/src/useIsRouteMatchProtected.ts +++ b/packages/react-router/src/useIsRouteMatchProtected.ts @@ -12,7 +12,12 @@ export function useIsRouteMatchProtected(locationArg: Partial, { throw 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 false; + // 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; } return activeRoute.$visibility === "protected"; diff --git a/packages/webpack-module-federation/src/completeModuleRegistrations.ts b/packages/webpack-module-federation/src/completeModuleRegistrations.ts index 6afd4fbca..60da33dc8 100644 --- a/packages/webpack-module-federation/src/completeModuleRegistrations.ts +++ b/packages/webpack-module-federation/src/completeModuleRegistrations.ts @@ -1,7 +1,7 @@ import { completeLocalModuleRegistrations, getLocalModuleRegistrationStatus, type LocalModuleRegistrationError, type Runtime } from "@squide/core"; import { completeRemoteModuleRegistrations, getRemoteModuleRegistrationStatus, type RemoteModuleRegistrationError } from "./registerRemoteModules.ts"; -export function completeModuleRegistrations(runtime: TRuntime, data: TData) { +export function completeModuleRegistrations(runtime: TRuntime, data?: TData) { const promise: Promise[] = []; if (getLocalModuleRegistrationStatus() !== "none") { diff --git a/packages/webpack-module-federation/src/registerRemoteModules.ts b/packages/webpack-module-federation/src/registerRemoteModules.ts index a2ecda876..05786b21d 100644 --- a/packages/webpack-module-federation/src/registerRemoteModules.ts +++ b/packages/webpack-module-federation/src/registerRemoteModules.ts @@ -120,7 +120,7 @@ export class RemoteModuleRegistry { return errors; } - async completeModuleRegistrations(runtime: TRuntime, data: TData) { + async completeModuleRegistrations(runtime: TRuntime, data?: TData) { const errors: RemoteModuleRegistrationError[] = []; if (this.#registrationStatus === "none" || this.#registrationStatus === "in-progress") { @@ -199,7 +199,7 @@ export function registerRemoteModules(remotes, runtime, options); } -export function completeRemoteModuleRegistrations(runtime: TRuntime, data: TData) { +export function completeRemoteModuleRegistrations(runtime: TRuntime, data?: TData) { return remoteModuleRegistry.completeModuleRegistrations(runtime, data); } diff --git a/samples/endpoints/shell/src/AppRouter.tsx b/samples/endpoints/shell/src/AppRouter.tsx index 14a7fa3d7..1161578af 100644 --- a/samples/endpoints/shell/src/AppRouter.tsx +++ b/samples/endpoints/shell/src/AppRouter.tsx @@ -19,19 +19,13 @@ export interface AppRouterProps { } async function fetchPublicData(setFeatureFlags: (featureFlags: FeatureFlags) => void, signal: AbortSignal, logger: Logger) { - try { - const data = await fetchJson("/api/feature-flags", { - signal - }); + const data = await fetchJson("/api/feature-flags", { + signal + }); - logger.debug("[shell] %cFeature flags are ready%c:", "color: white; background-color: green;", "", data); + logger.debug("[shell] %cFeature flags are ready%c:", "color: white; background-color: green;", "", data); - setFeatureFlags(data); - } catch (error: unknown) { - if (!signal.aborted) { - throw error; - } - } + setFeatureFlags(data); } async function fetchSession(signal: AbortSignal) { @@ -78,14 +72,12 @@ function fetchProtectedData( setSubscription(subscription); }) .catch((error: unknown) => { - if (!signal.aborted) { - if (isApiError(error) && error.status === 401) { - // The authentication boundary will redirect to the login page. - return; - } - - throw error; + if (isApiError(error) && error.status === 401) { + // The authentication boundary will redirect to the login page. + return; } + + throw error; }); }