From 2f851067fa2cc052925e674c2c4a5dbb2f3236ad Mon Sep 17 00:00:00 2001 From: Matt Brophy Date: Thu, 30 Jan 2025 11:19:13 -0500 Subject: [PATCH] Properly handle interrupted manifest requests --- .changeset/young-stingrays-begin.md | 5 ++ integration/fog-of-war-test.ts | 55 +++++++++++++++++++ .../react-router/lib/dom/ssr/fog-of-war.ts | 28 ++++++---- packages/react-router/lib/router/router.ts | 1 + packages/react-router/lib/router/utils.ts | 1 + 5 files changed, 80 insertions(+), 10 deletions(-) create mode 100644 .changeset/young-stingrays-begin.md diff --git a/.changeset/young-stingrays-begin.md b/.changeset/young-stingrays-begin.md new file mode 100644 index 0000000000..43c3510d53 --- /dev/null +++ b/.changeset/young-stingrays-begin.md @@ -0,0 +1,5 @@ +--- +"react-router": patch +--- + +Properly handle interrupted manifest requests in lazy route discovery diff --git a/integration/fog-of-war-test.ts b/integration/fog-of-war-test.ts index f422e42a1a..9c05bfbab8 100644 --- a/integration/fog-of-war-test.ts +++ b/integration/fog-of-war-test.ts @@ -1273,4 +1273,59 @@ test.describe("Fog of War", () => { ), ]); }); + + test("handles interruptions from back to back navigations", async ({ + page, + }) => { + let fixture = await createFixture({ + files: { + ...getFiles(), + "app/routes/a.tsx": js` + import { Link, Outlet, useLoaderData, useNavigate } from "react-router"; + export function loader({ request }) { + return { message: "A LOADER" }; + } + export default function Index() { + let data = useLoaderData(); + let navigate = useNavigate(); + return ( + <> +

A: {data.message}

+ + + + ) + } + `, + }, + }); + let appFixture = await createAppFixture(fixture); + let app = new PlaywrightFixture(appFixture, page); + + await app.goto("/a", true); + expect( + await page.evaluate(() => + Object.keys((window as any).__reactRouterManifest.routes) + ) + ).toEqual(["root", "routes/a", "routes/_index"]); + + // /a/b gets discovered on click + await app.clickElement("[data-link]"); + await new Promise((resolve) => setTimeout(resolve, 1000)); + expect(await (await page.$("body"))?.textContent()).not.toContain( + "Not Found" + ); + await page.waitForSelector("#b"); + + expect( + await page.evaluate(() => + Object.keys((window as any).__reactRouterManifest.routes) + ) + ).toEqual(["root", "routes/a", "routes/_index", "routes/a.b"]); + }); }); diff --git a/packages/react-router/lib/dom/ssr/fog-of-war.ts b/packages/react-router/lib/dom/ssr/fog-of-war.ts index 9993d4a4cf..efa969392e 100644 --- a/packages/react-router/lib/dom/ssr/fog-of-war.ts +++ b/packages/react-router/lib/dom/ssr/fog-of-war.ts @@ -77,7 +77,7 @@ export function getPatchRoutesOnNavigationFunction( return undefined; } - return async ({ path, patch }) => { + return async ({ path, patch, signal }) => { if (discoveredPaths.has(path)) { return; } @@ -87,7 +87,8 @@ export function getPatchRoutesOnNavigationFunction( routeModules, isSpaMode, basename, - patch + patch, + signal ); }; } @@ -185,7 +186,8 @@ export async function fetchAndApplyManifestPatches( routeModules: RouteModules, isSpaMode: boolean, basename: string | undefined, - patchRoutes: DataRouter["patchRoutes"] + patchRoutes: DataRouter["patchRoutes"], + signal?: AbortSignal ): Promise { let manifestPath = `${basename != null ? basename : "/"}/__manifest`.replace( /\/+/g, @@ -203,15 +205,21 @@ export async function fetchAndApplyManifestPatches( return; } - let res = await fetch(url); + let serverPatches: AssetsManifest["routes"]; + try { + let res = await fetch(url, { signal }); - if (!res.ok) { - throw new Error(`${res.status} ${res.statusText}`); - } else if (res.status >= 400) { - throw new Error(await res.text()); - } + if (!res.ok) { + throw new Error(`${res.status} ${res.statusText}`); + } else if (res.status >= 400) { + throw new Error(await res.text()); + } - let serverPatches = (await res.json()) as AssetsManifest["routes"]; + serverPatches = (await res.json()) as AssetsManifest["routes"]; + } catch (e) { + if (signal?.aborted) return; + throw e; + } // Patch routes we don't know about yet into the manifest let knownRoutes = new Set(Object.keys(manifest.routes)); diff --git a/packages/react-router/lib/router/router.ts b/packages/react-router/lib/router/router.ts index be0fe6ec48..e5b95c02c7 100644 --- a/packages/react-router/lib/router/router.ts +++ b/packages/react-router/lib/router/router.ts @@ -3180,6 +3180,7 @@ export function createRouter(init: RouterInit): Router { let localManifest = manifest; try { await patchRoutesOnNavigationImpl({ + signal, path: pathname, matches: partialMatches, patch: (routeId, children) => { diff --git a/packages/react-router/lib/router/utils.ts b/packages/react-router/lib/router/utils.ts index 3a23e8315e..f89664d1d7 100644 --- a/packages/react-router/lib/router/utils.ts +++ b/packages/react-router/lib/router/utils.ts @@ -224,6 +224,7 @@ export type AgnosticPatchRoutesOnNavigationFunctionArgs< O extends AgnosticRouteObject = AgnosticRouteObject, M extends AgnosticRouteMatch = AgnosticRouteMatch > = { + signal: AbortSignal; path: string; matches: M[]; patch: (routeId: string | null, children: O[]) => void;