Optimize route matching internals (#15186)

Author: brophdawg11

integration/fog-of-war-test.ts

@@ -715,7 +715,9 @@ test.describe("Fog of War", () => {
     expect(await app.getHtml("#parent")).toMatch(`Parent`);
     expect(await app.getHtml("#child2")).toMatch(`Child 2`);
     expect(manifestRequests).toEqual([
-      expect.stringMatching(/\/__manifest\?paths=%2Fparent%2Fchild2&version=/),
+      expect.stringMatching(
+        /\/__manifest\?paths=%2Fparent%2C%2Fparent%2Fchild2&version=/,
+      ),
     ]);
   });
 
@@ -1065,7 +1067,7 @@ test.describe("Fog of War", () => {
     await page.waitForSelector("#splat");
     expect(await app.getHtml("#splat")).toMatch("Splat: b/c");
     expect(manifestRequests).toEqual([
-      expect.stringMatching(/\/__manifest\?paths=%2Fb%2Fc&version=/),
+      expect.stringMatching(/\/__manifest\?paths=%2Fb%2C%2Fb%2Fc&version=/),
     ]);
   });
 
@@ -1137,7 +1139,9 @@ test.describe("Fog of War", () => {
     await app.clickLink("/not/a/path");
     await page.waitForSelector("#error");
     expect(manifestRequests).toEqual([
-      expect.stringMatching(/\/__manifest\?paths=%2Fnot%2Fa%2Fpath&version=/),
+      expect.stringMatching(
+        /\/__manifest\?paths=%2Fnot%2C%2Fnot%2Fa%2C%2Fnot%2Fa%2Fpath&version=/,
+      ),
     ]);
     manifestRequests = [];
 
@@ -1449,7 +1453,9 @@ test.describe("Fog of War", () => {
     // Wait for eager discovery to kick off
     await new Promise((r) => setTimeout(r, 500));
     expect(manifestRequests).toEqual([
-      expect.stringMatching(/\/custom-manifest\?paths=%2Fa%2Fb&version=/),
+      expect.stringMatching(
+        /\/custom-manifest\?paths=%2Fa%2C%2Fa%2Fb&version=/,
+      ),
     ]);
 
     expect(wrongManifestRequests).toEqual([]);

packages/react-router/.changes/patch.precompute-route-branch-matchers.md

@@ -0,0 +1 @@
+Precompute route branch matchers to avoid recompiling route path regexes during matching

packages/react-router/__tests__/dom/ssr/fog-of-war-test.ts

@@ -0,0 +1,25 @@
+import { getPathsWithAncestors } from "../../../lib/dom/ssr/fog-of-war";
+
+describe("fog of war", () => {
+  describe("getPathsWithAncestors", () => {
+    test("adds parent paths", () => {
+      expect(getPathsWithAncestors(["/a/b/c"])).toEqual([
+        "/a",
+        "/a/b",
+        "/a/b/c",
+      ]);
+    });
+
+    test("dedupes shared parent paths", () => {
+      expect(getPathsWithAncestors(["/a/b", "/a/c"])).toEqual([
+        "/a",
+        "/a/b",
+        "/a/c",
+      ]);
+    });
+
+    test("normalizes paths without leading slashes", () => {
+      expect(getPathsWithAncestors(["a/b"])).toEqual(["/a", "/a/b"]);
+    });
+  });
+});

packages/react-router/__tests__/rsc/server-test.ts

@@ -0,0 +1,43 @@
+import {
+  matchRSCServerRequest,
+  type RSCMatch,
+  type RSCRouteConfigEntry,
+} from "../../lib/rsc/server.rsc";
+import { URL_LIMIT } from "../../lib/dom/ssr/fog-of-war";
+
+describe("RSC server", () => {
+  describe("manifest requests", () => {
+    test("rejects manifest requests over the URL limit", async () => {
+      let path = `/${"a".repeat(URL_LIMIT)}.manifest`;
+
+      let { response, match } = await matchManifestRequest(
+        new Request(`https://remix.run${path}`),
+        [],
+      );
+
+      expect(response.status).toBe(400);
+      expect(match).toBeUndefined();
+    });
+  });
+});
+
+async function matchManifestRequest(
+  request: Request,
+  routes: RSCRouteConfigEntry[],
+) {
+  let match: RSCMatch | undefined;
+  let response = await matchRSCServerRequest({
+    createTemporaryReferenceSet: () => ({}),
+    request,
+    routes,
+    generateResponse(nextMatch) {
+      match = nextMatch;
+      return new Response(null, {
+        status: nextMatch.statusCode,
+        headers: nextMatch.headers,
+      });
+    },
+  });
+
+  return { response, match };
+}

packages/react-router/__tests__/server-runtime/server-test.ts

@@ -6,6 +6,7 @@ import { createContext, type StaticHandlerContext } from "react-router";
 
 import { createRequestHandler } from "../../lib/server-runtime/server";
 import { ServerMode } from "../../lib/server-runtime/mode";
+import { URL_LIMIT } from "../../lib/dom/ssr/fog-of-war";
 import { mockServerBuild } from "./utils";
 
 function spyConsole() {
@@ -2133,7 +2134,7 @@ describe("shared server runtime", () => {
       let handler = createRequestHandler(build, ServerMode.Test);
 
       let request = new Request(
-        `${baseUrl}/__manifest?paths=%2Fa%2Fb&version=${build.assets.version}`,
+        `${baseUrl}/__manifest?paths=%2Fa,%2Fa%2Fb&version=${build.assets.version}`,
       );
 
       let result = await handler(request);
@@ -2165,6 +2166,24 @@ describe("shared server runtime", () => {
       });
     });
 
+    test("rejects manifest requests over the URL limit", async () => {
+      let build = mockServerBuild({
+        root: {
+          default: {},
+        },
+      });
+      let handler = createRequestHandler(build, ServerMode.Test);
+
+      let request = new Request(
+        `${baseUrl}/__manifest?paths=${encodeURIComponent(
+          `/${"a".repeat(URL_LIMIT)}`,
+        )}&version=${build.assets.version}`,
+      );
+
+      let result = await handler(request);
+      expect(result.status).toBe(400);
+    });
+
     test("disabled when route discovery is disabled", async () => {
       let build = mockServerBuild(
         {

packages/react-router/lib/dom/ssr/fog-of-war.ts

@@ -25,6 +25,31 @@ const discoveredPaths = new Set<string>();
 // https://stackoverflow.com/a/417184
 export const URL_LIMIT = 7680;
 
+export function getPathsWithAncestors(paths: string[]): string[] {
+  let result = new Set<string>();
+
+  paths.forEach((path) => {
+    if (!path.startsWith("/")) {
+      path = `/${path}`;
+    }
+    // In addition to the requested path, we need to include patches for each
+    // ancestor path so that we pick up any pathless/index routes below ancestor
+    // segments. So if we get a request for `/parent/child`, we need to look for
+    // a match on `/parent` so that if a `parent._index` route exists we return
+    // it and it's available for client side matching if the user routes back up
+    // to `/parent`. This is the same thing we do on initial load in <Scripts>
+    // via `getPartialManifest()`.
+    for (let i = 1; i < path.length; i++) {
+      if (path[i] === "/") {
+        result.add(path.slice(0, i));
+      }
+    }
+    result.add(path);
+  });
+
+  return Array.from(result);
+}
+
 export function isFogOfWarEnabled(
   routeDiscovery: ServerBuild["routeDiscovery"],
   ssr: boolean,
@@ -228,6 +253,8 @@ export async function fetchAndApplyManifestPatches(
   patchRoutes: DataRouter["patchRoutes"],
   signal?: AbortSignal,
 ): Promise<void> {
+  paths = getPathsWithAncestors(paths);
+
   // NOTE: Intentionally using a standalone `URLSearchParams` instance
   // instead of mutating `url.searchParams`, which is *significantly* slower:
   // https://issues.chromium.org/issues/331406951

packages/react-router/lib/router/utils.ts

@@ -1105,6 +1105,8 @@ interface RouteMeta<RouteObjectType extends RouteObject = RouteObject> {
   caseSensitive: boolean;
   childrenIndex: number;
   route: RouteObjectType;
+  matcher?: RegExp;
+  compiledParams?: CompiledPathParam[];
 }
 
 /**
@@ -1205,9 +1207,21 @@ function flattenRoutes<RouteObjectType extends RouteObject = RouteObject>(
     branches.push({
       path,
       score: computeScore(path, route.index),
-      routesMeta,
+      routesMeta: routesMeta.map((meta, i) => {
+        let [matcher, params] = compilePath(
+          meta.relativePath,
+          meta.caseSensitive,
+          i === routesMeta.length - 1,
+        );
+        return {
+          ...meta,
+          matcher,
+          compiledParams: params,
+        } satisfies RouteMeta<RouteObjectType>;
+      }),
     });
   };
+
   routes.forEach((route, index) => {
     // coarse-grain check for optional params
     if (route.path === "" || !route.path?.includes("?")) {
@@ -1360,10 +1374,21 @@ function matchRouteBranch<
       matchedPathname === "/"
         ? pathname
         : pathname.slice(matchedPathname.length) || "/";
-    let match = matchPath(
-      { path: meta.relativePath, caseSensitive: meta.caseSensitive, end },
-      remainingPathname,
-    );
+    let pattern = {
+      path: meta.relativePath,
+      caseSensitive: meta.caseSensitive,
+      end,
+    };
+    let match =
+      // Use precomputed matcher if it exists
+      meta.matcher && meta.compiledParams
+        ? matchPathImpl(
+            pattern,
+            remainingPathname,
+            meta.matcher,
+            meta.compiledParams,
+          )
+        : matchPath(pattern, remainingPathname);
 
     let route = meta.route;
 
@@ -1546,6 +1571,15 @@ export function matchPath<Path extends string>(
     pattern.end,
   );
 
+  return matchPathImpl(pattern, pathname, matcher, compiledParams);
+}
+
+function matchPathImpl<Path extends string>(
+  pattern: PathPattern<Path>,
+  pathname: string,
+  matcher: RegExp,
+  compiledParams: CompiledPathParam[],
+): PathMatch<ParamParseKey<Path>> | null {
   let match = pathname.match(matcher);
   if (!match) return null;
 

packages/react-router/lib/rsc/browser.tsx

@@ -44,7 +44,7 @@ import {
 import { RSCRouterGlobalErrorBoundary } from "./errorBoundaries";
 import type { RouteModules } from "../dom/ssr/routeModules";
 import { populateRSCRouteModules } from "./route-modules";
-import { URL_LIMIT } from "../dom/ssr/fog-of-war";
+import { URL_LIMIT, getPathsWithAncestors } from "../dom/ssr/fog-of-war";
 
 const defaultManifestPath = "/__manifest";
 
@@ -1044,6 +1044,8 @@ async function fetchAndApplyManifestPatches(
   fetchImplementation: (request: Request) => Promise<Response>,
   signal?: AbortSignal,
 ) {
+  paths = getPathsWithAncestors(paths);
+
   let url = getManifestUrl(paths);
   if (url == null) {
     return;

packages/react-router/lib/rsc/server.rsc.ts

@@ -39,6 +39,7 @@ 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 { throwIfPotentialCSRFAttack } from "../actions";
 import invariant from "../server-runtime/invariant";
 
@@ -532,6 +533,14 @@ async function generateManifestResponse(
   temporaryReferences: unknown,
   routeDiscovery: RouteDiscovery | undefined,
 ) {
+  let url = new URL(request.url);
+  if (url.toString().length > URL_LIMIT) {
+    return new Response(null, {
+      statusText: "Bad Request",
+      status: 400,
+    });
+  }
+
   if (routeDiscovery?.mode === "initial") {
     let payload: RSCManifestPayload = {
       type: "manifest",
@@ -550,7 +559,6 @@ async function generateManifestResponse(
     );
   }
 
-  let url = new URL(request.url);
   let pathParam = url.searchParams.get("paths");
   let pathnames = pathParam
     ? pathParam.split(",").filter(Boolean)
@@ -1193,7 +1201,7 @@ async function getRenderPayload(
           ),
         )
       : getAdditionalRoutePatches(
-          [staticContext.location.pathname],
+          getPathsWithAncestors([staticContext.location.pathname]),
           routes,
           basename,
           staticContext.matches.map((m) => m.route.id),
@@ -1428,33 +1436,18 @@ async function getAdditionalRoutePatches(
   let matchedPaths = new Set<string>();
 
   for (const pathname of pathnames) {
-    let segments = pathname.split("/").filter(Boolean);
-    let paths: string[] = ["/"];
-
-    // We've already matched to the last segment
-    segments.pop();
-
-    // Traverse each path for our parents and match in case they have pathless/index
-    // children we need to include in the initial manifest
-    while (segments.length > 0) {
-      paths.push(`/${segments.join("/")}`);
-      segments.pop();
+    if (matchedPaths.has(pathname)) {
+      continue;
     }
-
-    paths.forEach((path) => {
-      if (matchedPaths.has(path)) {
+    matchedPaths.add(pathname);
+    let matches = matchRoutes(routes, pathname, basename) || [];
+    matches.forEach((m, i) => {
+      if (patchRouteMatches.get(m.route.id)) {
         return;
       }
-      matchedPaths.add(path);
-      let matches = matchRoutes(routes, path, basename) || [];
-      matches.forEach((m, i) => {
-        if (patchRouteMatches.get(m.route.id)) {
-          return;
-        }
-        patchRouteMatches.set(m.route.id, {
-          ...m.route,
-          parentId: matches[i - 1]?.route.id,
-        });
+      patchRouteMatches.set(m.route.id, {
+        ...m.route,
+        parentId: matches[i - 1]?.route.id,
       });
     });
   }