From dd8a1f39ca7b62e3f825f944244102184b352277 Mon Sep 17 00:00:00 2001 From: Josh Larson Date: Thu, 11 Sep 2025 10:31:21 -0500 Subject: [PATCH 1/4] Use comma-separated list for `p` values in lazy manifest requests --- integration/fog-of-war-test.ts | 6 +++--- packages/react-router/lib/dom/ssr/fog-of-war.ts | 2 +- packages/react-router/lib/rsc/browser.tsx | 2 +- packages/react-router/lib/rsc/server.rsc.ts | 6 +++--- packages/react-router/lib/server-runtime/server.ts | 4 +++- 5 files changed, 11 insertions(+), 9 deletions(-) diff --git a/integration/fog-of-war-test.ts b/integration/fog-of-war-test.ts index e311872374..3d7ffa8c00 100644 --- a/integration/fog-of-war-test.ts +++ b/integration/fog-of-war-test.ts @@ -1233,7 +1233,7 @@ test.describe("Fog of War", () => { await new Promise((resolve) => setTimeout(resolve, 250)); expect(manifestRequests).toEqual([ expect.stringMatching( - /\/__manifest\?p=%2F&p=%2Fa&p=%2Fb&version=[a-z0-9]{8}/, + /\/__manifest\?p=%2F%2C%2Fa%2C%2Fb&version=[a-z0-9]{8}/, ), ]); }); @@ -1275,7 +1275,7 @@ test.describe("Fog of War", () => { await new Promise((resolve) => setTimeout(resolve, 250)); expect(manifestRequests).toEqual([ expect.stringMatching( - /\/__manifest\?p=%2F&p=%2Fa&p=%2Fb&p=%2Fc&p=%2Fd&p=%2Fe&p=%2Ff&p=%2F/, + /\/__manifest\?p=%2F%2C%2Fa%2C%2Fb%2C%2Fc%2C%2Fd%2C%2Fe%2C%2Ff/, ), ]); }); @@ -1439,7 +1439,7 @@ test.describe("Fog of War", () => { ), ).toEqual(["root", "routes/_index", "routes/a"]); expect(manifestRequests).toEqual([ - expect.stringMatching(/\/custom-manifest\?p=%2F&p=%2Fa&version=/), + expect.stringMatching(/\/custom-manifest\?p=%2F%2C%2Fa&version=/), ]); manifestRequests = []; 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 5770fda779..ed46ffd980 100644 --- a/packages/react-router/lib/dom/ssr/fog-of-war.ts +++ b/packages/react-router/lib/dom/ssr/fog-of-war.ts @@ -222,7 +222,7 @@ export async function fetchAndApplyManifestPatches( // https://issues.chromium.org/issues/331406951 // https://github.com/nodejs/node/issues/51518 const searchParams = new URLSearchParams(); - paths.sort().forEach((path) => searchParams.append("p", path)); + searchParams.set("p", paths.sort().join(",")); searchParams.set("version", manifest.version); let url = new URL( getManifestPath(manifestPath, basename), diff --git a/packages/react-router/lib/rsc/browser.tsx b/packages/react-router/lib/rsc/browser.tsx index 4d490f8f45..a88fcf90ca 100644 --- a/packages/react-router/lib/rsc/browser.tsx +++ b/packages/react-router/lib/rsc/browser.tsx @@ -976,7 +976,7 @@ function getManifestUrl(paths: string[]): URL | null { "", ); let url = new URL(`${basename}/.manifest`, window.location.origin); - paths.sort().forEach((path) => url.searchParams.append("p", path)); + url.searchParams.set("p", paths.sort().join(",")); return url; } diff --git a/packages/react-router/lib/rsc/server.rsc.ts b/packages/react-router/lib/rsc/server.rsc.ts index 3f99687e99..f3639ebb1a 100644 --- a/packages/react-router/lib/rsc/server.rsc.ts +++ b/packages/react-router/lib/rsc/server.rsc.ts @@ -472,9 +472,9 @@ async function generateManifestResponse( temporaryReferences: unknown, ) { let url = new URL(request.url); - let pathnameParams = url.searchParams.getAll("p"); - let pathnames = pathnameParams.length - ? pathnameParams + let pathParam = url.searchParams.get("p"); + let pathnames = pathParam + ? pathParam.split(",").filter(Boolean) : [url.pathname.replace(/\.manifest$/, "")]; let routeIds = new Set(); let matchedRoutes = pathnames diff --git a/packages/react-router/lib/server-runtime/server.ts b/packages/react-router/lib/server-runtime/server.ts index 71768f3035..3b533ce669 100644 --- a/packages/react-router/lib/server-runtime/server.ts +++ b/packages/react-router/lib/server-runtime/server.ts @@ -365,7 +365,9 @@ async function handleManifestRequest( // for client side matching if the user routes back up to `/parent`. // This is the same thing we do on initial load in via // `getPartialManifest()` - url.searchParams.getAll("p").forEach((path) => { + let pathParam = url.searchParams.get("p") || ""; + let requestedPaths = pathParam.split(",").filter(Boolean); + requestedPaths.forEach((path) => { if (!path.startsWith("/")) { path = `/${path}`; } From b86a1e882437c3d18b85254b08fbb19391590219 Mon Sep 17 00:00:00 2001 From: Josh Larson Date: Thu, 11 Sep 2025 10:46:15 -0500 Subject: [PATCH 2/4] Sign the CLA --- contributors.yml | 1 + 1 file changed, 1 insertion(+) diff --git a/contributors.yml b/contributors.yml index 06bcb5c118..157eaadfab 100644 --- a/contributors.yml +++ b/contributors.yml @@ -188,6 +188,7 @@ - johnpangalos - jonkoops - joseph0926 +- jplhomer - jrakotoharisoa - jrestall - juanpprieto From ae6d61df43e0bb11f3c54109ab99108e18ae8509 Mon Sep 17 00:00:00 2001 From: Matt Brophy Date: Fri, 12 Sep 2025 12:02:50 -0400 Subject: [PATCH 3/4] Rename p -> paths --- integration/fog-of-war-test.ts | 30 +++++++++---------- .../react-router/lib/dom/ssr/fog-of-war.ts | 2 +- packages/react-router/lib/rsc/browser.tsx | 2 +- packages/react-router/lib/rsc/server.rsc.ts | 2 +- .../react-router/lib/server-runtime/server.ts | 4 +-- 5 files changed, 20 insertions(+), 20 deletions(-) diff --git a/integration/fog-of-war-test.ts b/integration/fog-of-war-test.ts index 3d7ffa8c00..516d0d28e4 100644 --- a/integration/fog-of-war-test.ts +++ b/integration/fog-of-war-test.ts @@ -715,7 +715,7 @@ 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\?p=%2Fparent%2Fchild2&version=/), + expect.stringMatching(/\/__manifest\?paths=%2Fparent%2Fchild2&version=/), ]); }); @@ -783,7 +783,7 @@ test.describe("Fog of War", () => { ), ).toEqual(["root", "routes/_index", "routes/$slug"]); expect(manifestRequests).toEqual([ - expect.stringMatching(/\/__manifest\?p=%2Fsomething&version=/), + expect.stringMatching(/\/__manifest\?paths=%2Fsomething&version=/), ]); manifestRequests = []; @@ -797,7 +797,7 @@ test.describe("Fog of War", () => { await page.waitForSelector("#static"); expect(await app.getHtml("#static")).toMatch("Static"); expect(manifestRequests).toEqual([ - expect.stringMatching(/\/__manifest\?p=%2Fstatic&version=/), + expect.stringMatching(/\/__manifest\?paths=%2Fstatic&version=/), ]); expect( await page.evaluate(() => @@ -870,7 +870,7 @@ test.describe("Fog of War", () => { ), ).toEqual(["root", "routes/_index", "routes/$"]); expect(manifestRequests).toEqual([ - expect.stringMatching(/\/__manifest\?p=%2Fsomething&version=/), + expect.stringMatching(/\/__manifest\?paths=%2Fsomething&version=/), ]); manifestRequests = []; @@ -884,7 +884,7 @@ test.describe("Fog of War", () => { await page.waitForSelector("#static"); expect(await app.getHtml("#static")).toMatch("Static"); expect(manifestRequests).toEqual([ - expect.stringMatching(/\/__manifest\?p=%2Fstatic&version=/), + expect.stringMatching(/\/__manifest\?paths=%2Fstatic&version=/), ]); expect( await page.evaluate(() => @@ -956,7 +956,7 @@ test.describe("Fog of War", () => { await page.waitForSelector("#slug"); expect(await app.getHtml("#slug")).toMatch("Slug: a"); expect(manifestRequests).toEqual([ - expect.stringMatching(/\/__manifest\?p=%2Fa&version=/), + expect.stringMatching(/\/__manifest\?paths=%2Fa&version=/), ]); manifestRequests = []; @@ -977,7 +977,7 @@ test.describe("Fog of War", () => { await page.waitForSelector("#slug"); expect(await app.getHtml("#slug")).toMatch("Slug: b"); expect(manifestRequests).toEqual([ - expect.stringMatching(/\/__manifest\?p=%2Fb&version=/), + expect.stringMatching(/\/__manifest\?paths=%2Fb&version=/), ]); }); @@ -1044,7 +1044,7 @@ test.describe("Fog of War", () => { await page.waitForSelector("#splat"); expect(await app.getHtml("#splat")).toMatch("Splat: a"); expect(manifestRequests).toEqual([ - expect.stringMatching(/\/__manifest\?p=%2Fa&version=/), + expect.stringMatching(/\/__manifest\?paths=%2Fa&version=/), ]); manifestRequests = []; @@ -1065,7 +1065,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\?p=%2Fb%2Fc&version=/), + expect.stringMatching(/\/__manifest\?paths=%2Fb%2Fc&version=/), ]); }); @@ -1137,7 +1137,7 @@ test.describe("Fog of War", () => { await app.clickLink("/not/a/path"); await page.waitForSelector("#error"); expect(manifestRequests).toEqual([ - expect.stringMatching(/\/__manifest\?p=%2Fnot%2Fa%2Fpath&version=/), + expect.stringMatching(/\/__manifest\?paths=%2Fnot%2Fa%2Fpath&version=/), ]); manifestRequests = []; @@ -1145,7 +1145,7 @@ test.describe("Fog of War", () => { await app.clickLink("/something"); await page.waitForSelector("#slug"); expect(manifestRequests).toEqual([ - expect.stringMatching(/\/__manifest\?p=%2Fsomething&version=/), + expect.stringMatching(/\/__manifest\?paths=%2Fsomething&version=/), ]); manifestRequests = []; @@ -1233,7 +1233,7 @@ test.describe("Fog of War", () => { await new Promise((resolve) => setTimeout(resolve, 250)); expect(manifestRequests).toEqual([ expect.stringMatching( - /\/__manifest\?p=%2F%2C%2Fa%2C%2Fb&version=[a-z0-9]{8}/, + /\/__manifest\?paths=%2F%2C%2Fa%2C%2Fb&version=[a-z0-9]{8}/, ), ]); }); @@ -1275,7 +1275,7 @@ test.describe("Fog of War", () => { await new Promise((resolve) => setTimeout(resolve, 250)); expect(manifestRequests).toEqual([ expect.stringMatching( - /\/__manifest\?p=%2F%2C%2Fa%2C%2Fb%2C%2Fc%2C%2Fd%2C%2Fe%2C%2Ff/, + /\/__manifest\?paths=%2F%2C%2Fa%2C%2Fb%2C%2Fc%2C%2Fd%2C%2Fe%2C%2Ff%2C%2Fg/, ), ]); }); @@ -1439,7 +1439,7 @@ test.describe("Fog of War", () => { ), ).toEqual(["root", "routes/_index", "routes/a"]); expect(manifestRequests).toEqual([ - expect.stringMatching(/\/custom-manifest\?p=%2F%2C%2Fa&version=/), + expect.stringMatching(/\/custom-manifest\?paths=%2F%2C%2Fa&version=/), ]); manifestRequests = []; @@ -1449,7 +1449,7 @@ 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\?p=%2Fa%2Fb&version=/), + expect.stringMatching(/\/custom-manifest\?paths=%2Fa%2Fb&version=/), ]); expect(wrongManifestRequests).toEqual([]); 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 ed46ffd980..37ee99d77d 100644 --- a/packages/react-router/lib/dom/ssr/fog-of-war.ts +++ b/packages/react-router/lib/dom/ssr/fog-of-war.ts @@ -222,7 +222,7 @@ export async function fetchAndApplyManifestPatches( // https://issues.chromium.org/issues/331406951 // https://github.com/nodejs/node/issues/51518 const searchParams = new URLSearchParams(); - searchParams.set("p", paths.sort().join(",")); + searchParams.set("paths", paths.sort().join(",")); searchParams.set("version", manifest.version); let url = new URL( getManifestPath(manifestPath, basename), diff --git a/packages/react-router/lib/rsc/browser.tsx b/packages/react-router/lib/rsc/browser.tsx index a88fcf90ca..119db6deef 100644 --- a/packages/react-router/lib/rsc/browser.tsx +++ b/packages/react-router/lib/rsc/browser.tsx @@ -976,7 +976,7 @@ function getManifestUrl(paths: string[]): URL | null { "", ); let url = new URL(`${basename}/.manifest`, window.location.origin); - url.searchParams.set("p", paths.sort().join(",")); + url.searchParams.set("paths", paths.sort().join(",")); return url; } diff --git a/packages/react-router/lib/rsc/server.rsc.ts b/packages/react-router/lib/rsc/server.rsc.ts index f3639ebb1a..d8880506ac 100644 --- a/packages/react-router/lib/rsc/server.rsc.ts +++ b/packages/react-router/lib/rsc/server.rsc.ts @@ -472,7 +472,7 @@ async function generateManifestResponse( temporaryReferences: unknown, ) { let url = new URL(request.url); - let pathParam = url.searchParams.get("p"); + let pathParam = url.searchParams.get("paths"); let pathnames = pathParam ? pathParam.split(",").filter(Boolean) : [url.pathname.replace(/\.manifest$/, "")]; diff --git a/packages/react-router/lib/server-runtime/server.ts b/packages/react-router/lib/server-runtime/server.ts index 3b533ce669..d3b78ae510 100644 --- a/packages/react-router/lib/server-runtime/server.ts +++ b/packages/react-router/lib/server-runtime/server.ts @@ -354,7 +354,7 @@ async function handleManifestRequest( let patches: Record = {}; - if (url.searchParams.has("p")) { + if (url.searchParams.has("paths")) { let paths = new Set(); // In addition to responding with the patches for the requested paths, we @@ -365,7 +365,7 @@ async function handleManifestRequest( // for client side matching if the user routes back up to `/parent`. // This is the same thing we do on initial load in via // `getPartialManifest()` - let pathParam = url.searchParams.get("p") || ""; + let pathParam = url.searchParams.get("paths") || ""; let requestedPaths = pathParam.split(",").filter(Boolean); requestedPaths.forEach((path) => { if (!path.startsWith("/")) { From f05cd10cac2c72cf982056e449f0399520c9c44c Mon Sep 17 00:00:00 2001 From: Matt Brophy Date: Fri, 12 Sep 2025 12:17:25 -0400 Subject: [PATCH 4/4] Add changeset --- .changeset/chilly-needles-taste.md | 8 ++++++++ 1 file changed, 8 insertions(+) create mode 100644 .changeset/chilly-needles-taste.md diff --git a/.changeset/chilly-needles-taste.md b/.changeset/chilly-needles-taste.md new file mode 100644 index 0000000000..e8fa149f18 --- /dev/null +++ b/.changeset/chilly-needles-taste.md @@ -0,0 +1,8 @@ +--- +"react-router": patch +--- + +Update Lazy Route Discovery manifest requests to use a singular comma-separated `paths` query param instead of repeated `p` query params + +- This is because Cloudflare has a hard limit of 100 URL search param key/value pairs when used as a key for caching purposes +- If more that 100 paths were included, the cache key would be incomplete and could produce false-positive cache hits