diff --git a/integration/fog-of-war-test.ts b/integration/fog-of-war-test.ts index 03b29b05fb..44b2eb1b36 100644 --- a/integration/fog-of-war-test.ts +++ b/integration/fog-of-war-test.ts @@ -85,6 +85,47 @@ function getFiles() { }; } +test("switches to initial discovery above the pathname work limit", async ({ + page, +}) => { + let fixture = await createFixture({ + files: { + ...getFiles(), + "app/routes/$.tsx": js` + export default function Splat() { + return

Sample page

; + } + `, + }, + }); + let appFixture = await createAppFixture(fixture); + let app = new PlaywrightFixture(appFixture, page); + + try { + let response = await app.goto( + `/${Array.from({ length: 60 }, (_, i) => i % 10).join("/")}`, + ); + expect(response.status()).toBe(200); + expect( + await page.evaluate( + () => (window as any).__reactRouterContext.routeDiscovery.mode, + ), + ).toBe("lazy"); + + response = await app.goto( + `/${Array.from({ length: 70 }, (_, i) => i % 10).join("/")}`, + ); + expect(response.status()).toBe(200); + expect( + await page.evaluate( + () => (window as any).__reactRouterContext.routeDiscovery.mode, + ), + ).toBe("initial"); + } finally { + await appFixture.close(); + } +}); + test.describe("Fog of War", () => { let oldConsoleError: typeof console.error; diff --git a/integration/rsc/route-discovery-test.ts b/integration/rsc/route-discovery-test.ts new file mode 100644 index 0000000000..166925975c --- /dev/null +++ b/integration/rsc/route-discovery-test.ts @@ -0,0 +1,84 @@ +import { test, expect } from "@playwright/test"; +import getPort from "get-port"; + +import { implementations, js, setupRscTest } from "./utils"; + +for (let implementation of implementations) { + test(`RSC (${implementation.name}) bounds route discovery for documents and data`, async ({ + page, + }) => { + let pathname = `/${Array.from({ length: 70 }, (_, i) => i % 10).join("/")}`; + let port = await getPort(); + let stop = await setupRscTest({ + implementation, + port, + files: { + "src/routes.ts": js` + export const routes = [{ + id: "root", + path: "", + lazy: () => import("./routes/root"), + children: [ + { id: "other", path: "other", lazy: () => import("./routes/other") }, + { id: "splat", path: "*", lazy: () => import("./routes/sample") }, + ], + }]; + `, + "src/routes/sample.tsx": js` + import { Link } from "react-router"; + export default function Sample() { + return <> +

Sample

+ Sample + Other + ; + } + `, + "src/routes/other.tsx": js` + export default function Other() { + return

Other

; + } + `, + }, + }); + + try { + let manifestRequests: string[] = []; + page.on("request", (request) => { + if (new URL(request.url()).pathname.endsWith(".manifest")) { + manifestRequests.push(request.url()); + } + }); + + let response = await page.goto(`http://localhost:${port}${pathname}`); + expect(response?.status()).toBe(200); + await expect(page.getByRole("heading", { name: "Sample" })).toBeVisible(); + await page.waitForFunction(() => (window as any).__routerInitialized); + let otherResponse = page.waitForResponse( + (response) => new URL(response.url()).pathname === "/other.rsc", + ); + await page.getByRole("link", { name: "Other" }).click(); + expect((await otherResponse).status()).toBe(200); + await expect(page.getByRole("heading", { name: "Other" })).toBeVisible(); + expect(manifestRequests).toEqual([]); + + // Start with the splat already discovered so navigation can request data. + await page.goto(`http://localhost:${port}/sample`); + await page.waitForFunction(() => (window as any).__routerInitialized); + let dataResponse = page.waitForResponse( + (response) => new URL(response.url()).pathname === `${pathname}.rsc`, + ); + await page.getByRole("link", { name: "Sample" }).click(); + let data = await dataResponse; + expect(data.status()).toBe(200); + let payload = await data.text(); + expect(payload).toContain('"routeDiscovery":{"mode":"initial"}'); + expect(payload).toContain('"id":"other"'); + await expect(page.getByRole("heading", { name: "Sample" })).toBeVisible(); + await page.getByRole("link", { name: "Other" }).click(); + await expect(page.getByRole("heading", { name: "Other" })).toBeVisible(); + } finally { + stop(); + } + }); +} diff --git a/packages/react-router/.changes/patch.bound-ssr-manifest-expansion.md b/packages/react-router/.changes/patch.bound-ssr-manifest-expansion.md new file mode 100644 index 0000000000..4002fbaedd --- /dev/null +++ b/packages/react-router/.changes/patch.bound-ssr-manifest-expansion.md @@ -0,0 +1 @@ +Bound route manifest processing during server rendering and RSC document/data requests by falling back to initial route discovery for paths with expensive ancestor expansion, accounting for the number and depth of candidate branches when using regex matching diff --git a/packages/react-router/__tests__/dom/ssr/fog-of-war-test.ts b/packages/react-router/__tests__/dom/ssr/fog-of-war-test.ts index d86c511e29..cb07fd180a 100644 --- a/packages/react-router/__tests__/dom/ssr/fog-of-war-test.ts +++ b/packages/react-router/__tests__/dom/ssr/fog-of-war-test.ts @@ -1,6 +1,60 @@ -import { getPathsWithAncestors } from "../../../lib/dom/ssr/fog-of-war"; +import { + getPathsWithAncestors, + shouldUseInitialRouteDiscovery, + URL_LIMIT, +} from "../../../lib/dom/ssr/fog-of-war"; + +function samplePath(segments: number, segmentLength = 1): string { + return `/${Array.from({ length: segments }, (_, i) => + String(i % 10).repeat(segmentLength), + ).join("/")}`; +} describe("fog of war", () => { + describe("shouldUseInitialRouteDiscovery", () => { + test("keeps the match-cost budget inactive through ten segments", () => { + for (let segments of [0, 1, 10]) { + expect( + shouldUseInitialRouteDiscovery(samplePath(segments), 50_001), + ).toBe(false); + } + expect(shouldUseInitialRouteDiscovery(samplePath(11), 50_001)).toBe(true); + }); + + test("permits exactly the match-cost budget", () => { + expect(shouldUseInitialRouteDiscovery(samplePath(20), 2500)).toBe(false); + expect(shouldUseInitialRouteDiscovery(samplePath(20), 2501)).toBe(true); + }); + + test("bounds pathname work without a match cost", () => { + expect(shouldUseInitialRouteDiscovery(samplePath(62))).toBe(false); + expect(shouldUseInitialRouteDiscovery(samplePath(63))).toBe(true); + }); + + test("bounds pathname work even for shallow paths", () => { + for (let matchCost of [undefined, 50_001]) { + expect( + shouldUseInitialRouteDiscovery( + `${samplePath(1, URL_LIMIT - 4)}/x`, + matchCost, + ), + ).toBe(false); + expect( + shouldUseInitialRouteDiscovery( + `${samplePath(1, URL_LIMIT - 3)}/x`, + matchCost, + ), + ).toBe(true); + } + }); + + test("permits a single segment without ancestor work", () => { + expect(shouldUseInitialRouteDiscovery(samplePath(1, URL_LIMIT))).toBe( + false, + ); + }); + }); + describe("getPathsWithAncestors", () => { test("adds parent paths", () => { expect(getPathsWithAncestors(["/a/b/c"])).toEqual([ diff --git a/packages/react-router/__tests__/rsc/server-test.ts b/packages/react-router/__tests__/rsc/server-test.ts index fdf4d2ab32..2761786d0e 100644 --- a/packages/react-router/__tests__/rsc/server-test.ts +++ b/packages/react-router/__tests__/rsc/server-test.ts @@ -6,6 +6,59 @@ import { import { URL_LIMIT } from "../../lib/dom/ssr/fog-of-war"; describe("RSC server", () => { + test("switches to initial discovery above the pathname work limit", async () => { + let routeDiscovery = Object.freeze({ mode: "lazy" } as const); + let routes: RSCRouteConfigEntry[] = [ + { + id: "root", + path: "", + Component: () => null, + children: [ + { id: "other", path: "other" }, + { id: "splat", path: "*", Component: () => null }, + ], + }, + ]; + + async function getPayload(pathname: string) { + let match: RSCMatch | undefined; + let response = await matchRSCServerRequest({ + createTemporaryReferenceSet: () => ({}), + request: new Request(`https://remix.run${pathname}`), + routes, + routeDiscovery, + generateResponse(nextMatch) { + match = nextMatch; + return new Response(null, { status: nextMatch.statusCode }); + }, + }); + expect(response.status).toBe(200); + if (match?.payload.type !== "render") { + throw new Error("Expected a render payload"); + } + return match.payload; + } + + let pathname = `/${Array.from({ length: 60 }, (_, i) => i % 10).join("/")}`; + let payload = await getPayload(pathname); + expect(payload.routeDiscovery.mode).toBe("lazy"); + expect(await payload.patches).toEqual([]); + + payload = await getPayload(`${pathname}.rsc`); + expect(payload.routeDiscovery.mode).toBe("lazy"); + expect(await payload.patches).toEqual([]); + + pathname = `/${Array.from({ length: 70 }, (_, i) => i % 10).join("/")}`; + payload = await getPayload(pathname); + expect(payload.routeDiscovery.mode).toBe("initial"); + expect((await payload.patches).map((patch) => patch.id)).toEqual(["other"]); + + payload = await getPayload(`${pathname}.rsc`); + expect(payload.routeDiscovery.mode).toBe("initial"); + expect((await payload.patches).map((patch) => patch.id)).toEqual(["other"]); + expect(routeDiscovery.mode).toBe("lazy"); + }); + describe("manifest requests", () => { test("rejects manifest requests over the URL limit", async () => { let path = `/${"a".repeat(URL_LIMIT)}.manifest`; 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 265a4606a2..545acbe60f 100644 --- a/packages/react-router/lib/dom/ssr/fog-of-war.ts +++ b/packages/react-router/lib/dom/ssr/fog-of-war.ts @@ -57,6 +57,27 @@ export function isFogOfWarEnabled( return routeDiscovery.mode === "lazy" && ssr === true; } +const MANIFEST_MATCH_COST_LIMIT = 50_000; +const MANIFEST_MATCH_COST_SEGMENT_THRESHOLD = 10; + +export function shouldUseInitialRouteDiscovery( + pathname: string, + matchCost = 0, +): boolean { + let segmentCount = pathname.split("/").length - 1; + let ancestorCount = Math.max(segmentCount - 1, 0); + + // Each ancestor is shorter than the incoming pathname. Use an upper bound on + // their cumulative length, plus the root path, to avoid expanding every prefix. + // Regex matching scans candidate branches. Bound its estimated metadata + // visits for deep paths while keeping typical shallow paths lazy. + return ( + ancestorCount * pathname.length + 1 > URL_LIMIT || + (segmentCount > MANIFEST_MATCH_COST_SEGMENT_THRESHOLD && + segmentCount * matchCost > MANIFEST_MATCH_COST_LIMIT) + ); +} + export function getPartialManifest( { sri, ...manifest }: AssetsManifest, router: DataRouter, diff --git a/packages/react-router/lib/router/router.ts b/packages/react-router/lib/router/router.ts index 610b1d4167..55fec05896 100644 --- a/packages/react-router/lib/router/router.ts +++ b/packages/react-router/lib/router/router.ts @@ -485,6 +485,8 @@ export interface StaticHandler { * The set of data routes managed by this handler */ dataRoutes: DataRouteObject[]; + /** @private Estimated route metadata visits per regex match. */ + readonly _matchCost?: number; /** * @private * PRIVATE - DO NOT USE @@ -4112,6 +4114,12 @@ export function createStaticHandler( // Pre-compute flattened/ranked route branches when the flag is enabled. // Skipped in development mode because routes can be added dynamically (HMR). let routeBranches = flattenAndRankRoutes(dataRoutes); + // Upper bound on metadata visited per match, including optional segment + // expansion and pathless layouts within each candidate branch. + let matchCost = routeBranches.reduce( + (total, branch) => total + branch.routesMeta.length, + 0, + ); /** * The query() method is intended for document requests, in which we want to @@ -5029,6 +5037,7 @@ export function createStaticHandler( return { dataRoutes, + _matchCost: matchCost, _internalRouteBranches: routeBranches, query, queryRoute, diff --git a/packages/react-router/lib/rsc/server.rsc.ts b/packages/react-router/lib/rsc/server.rsc.ts index 834b27aa7d..a1af25480a 100644 --- a/packages/react-router/lib/rsc/server.rsc.ts +++ b/packages/react-router/lib/rsc/server.rsc.ts @@ -39,7 +39,11 @@ import { } from "../router/utils"; import { getDocumentHeadersImpl } from "../server-runtime/headers"; import { SINGLE_FETCH_REDIRECT_STATUS } from "../dom/ssr/single-fetch"; -import { URL_LIMIT, getPathsWithAncestors } from "../dom/ssr/fog-of-war"; +import { + URL_LIMIT, + getPathsWithAncestors, + shouldUseInitialRouteDiscovery, +} from "../dom/ssr/fog-of-war"; import { throwIfPotentialCSRFAttack } from "../actions"; import invariant from "../server-runtime/invariant"; @@ -958,6 +962,7 @@ async function generateRenderResponse( skipRevalidation, ctx.redirect?.headers, routeDiscovery, + staticHandler._matchCost, ); }, }), @@ -1056,6 +1061,7 @@ async function generateStaticContextResponse( skipRevalidation: boolean, sideEffectRedirectHeaders: Headers | undefined, routeDiscovery: RouteDiscovery | undefined, + matchCost: number | undefined, ): Promise { statusCode = staticContext.statusCode ?? statusCode; @@ -1122,6 +1128,7 @@ async function generateStaticContextResponse( isDataRequest, staticContext, routeDiscovery, + matchCost, ); let payload: RSCRenderPayload | RSCActionPayload; @@ -1164,6 +1171,7 @@ async function getRenderPayload( isDataRequest: boolean, staticContext: StaticHandlerContext, routeDiscovery: RouteDiscovery | undefined, + matchCost: number | undefined, ) { // Figure out how deep we want to render server components based on any // triggered error boundaries and/or `routeIdsToLoad` @@ -1198,23 +1206,32 @@ async function getRenderPayload( }), ); - let patches = - routeDiscovery?.mode === "initial" && !isDataRequest - ? getAllRoutePatches(routes, basename).then((patches) => - patches.filter( - (patch) => - !staticContext.matches.some((m) => m.route.id === patch.id), - ), - ) - : getAdditionalRoutePatches( - getPathsWithAncestors([staticContext.location.pathname]), - routes, - basename, - staticContext.matches.map((m) => m.route.id), - ); + let pathname = staticContext.location.pathname; + let useInitialRouteDiscovery = + (routeDiscovery?.mode === "initial" && !isDataRequest) || + shouldUseInitialRouteDiscovery(pathname, matchCost); + let patches: Promise; + if (useInitialRouteDiscovery) { + patches = getAllRoutePatches(routes, basename).then((patches) => + patches.filter( + (patch) => !staticContext.matches.some((m) => m.route.id === patch.id), + ), + ); + } else { + patches = getAdditionalRoutePatches( + // The full pathname is last and its matches are already in the payload. + getPathsWithAncestors([pathname]).slice(0, -1), + routes, + basename, + staticContext.matches.map((m) => m.route.id), + ); + } return { ...baseRenderPayload, + routeDiscovery: useInitialRouteDiscovery + ? ({ mode: "initial" } satisfies RouteDiscovery) + : baseRenderPayload.routeDiscovery, matches: await matchesPromise, patches, }; diff --git a/packages/react-router/lib/server-runtime/server.ts b/packages/react-router/lib/server-runtime/server.ts index 00bd4ff98d..f4c974d273 100644 --- a/packages/react-router/lib/server-runtime/server.ts +++ b/packages/react-router/lib/server-runtime/server.ts @@ -40,7 +40,12 @@ import { import { getDocumentHeaders } from "./headers"; import type { EntryRoute } from "../dom/ssr/routes"; import type { MiddlewareEnabled } from "../types/future"; -import { URL_LIMIT, getManifestPath } from "../dom/ssr/fog-of-war"; +import { + URL_LIMIT, + getManifestPath, + isFogOfWarEnabled, + shouldUseInitialRouteDiscovery, +} from "../dom/ssr/fog-of-war"; import type { InstrumentRequestHandlerFunction } from "../router/instrumentation"; import { instrumentHandler } from "../router/instrumentation"; import { throwIfPotentialCSRFAttack } from "../actions"; @@ -534,10 +539,22 @@ async function handleDocumentRequest( actionData: context.actionData, errors: context.errors, }; + let routeDiscovery = build.routeDiscovery; + if ( + isFogOfWarEnabled(routeDiscovery, build.ssr) && + shouldUseInitialRouteDiscovery( + context.location.pathname, + staticHandler._matchCost, + ) + ) { + // Keep this response and its client router on the existing full-manifest + // path without changing discovery for other requests using this build. + routeDiscovery = { ...routeDiscovery, mode: "initial" }; + } let baseServerHandoff: ServerHandoff = { basename: build.basename, future: build.future, - routeDiscovery: build.routeDiscovery, + routeDiscovery, ssr: build.ssr, isSpaMode, }; @@ -560,7 +577,7 @@ async function handleDocumentRequest( renderMeta: {}, future: build.future, ssr: build.ssr, - routeDiscovery: build.routeDiscovery, + routeDiscovery, isSpaMode, serializeError: (err) => serializeError(err, serverMode), };