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 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 diff --git a/integration/fog-of-war-test.ts b/integration/fog-of-war-test.ts index e311872374..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&p=%2Fa&p=%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&p=%2Fa&p=%2Fb&p=%2Fc&p=%2Fd&p=%2Fe&p=%2Ff&p=%2F/, + /\/__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&p=%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 5770fda779..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(); - paths.sort().forEach((path) => searchParams.append("p", path)); + 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 4d490f8f45..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); - paths.sort().forEach((path) => url.searchParams.append("p", path)); + 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 3f99687e99..d8880506ac 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("paths"); + 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..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,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("paths") || ""; + let requestedPaths = pathParam.split(",").filter(Boolean); + requestedPaths.forEach((path) => { if (!path.startsWith("/")) { path = `/${path}`; }