From b146e8d07afb10915c782a583396f5341d5662f0 Mon Sep 17 00:00:00 2001 From: Greg Brimble Date: Mon, 16 Dec 2024 14:31:52 +0000 Subject: [PATCH] Key the Early Hints off of the asset key rather than request path --- .changeset/cold-turtles-impress.md | 5 + .../__tests__/asset-server/handler.test.ts | 99 ++++++++++++++----- packages/pages-shared/asset-server/handler.ts | 6 +- 3 files changed, 82 insertions(+), 28 deletions(-) create mode 100644 .changeset/cold-turtles-impress.md diff --git a/.changeset/cold-turtles-impress.md b/.changeset/cold-turtles-impress.md new file mode 100644 index 000000000000..fbc015a97fae --- /dev/null +++ b/.changeset/cold-turtles-impress.md @@ -0,0 +1,5 @@ +--- +"@cloudflare/pages-shared": patch +--- + +fix: Key the Early Hints cache entries off of the asset key rather than the request path diff --git a/packages/pages-shared/__tests__/asset-server/handler.test.ts b/packages/pages-shared/__tests__/asset-server/handler.test.ts index 905ffaed1a08..388533f9f4c8 100644 --- a/packages/pages-shared/__tests__/asset-server/handler.test.ts +++ b/packages/pages-shared/__tests__/asset-server/handler.test.ts @@ -435,11 +435,27 @@ describe("asset-server handler", () => { const findAssetEntryForPath = async (path: string) => { if (path === "/index.html") { - return "index.html"; + return "asset-key-index.html"; } return null; }; + const fetchAsset = () => + Promise.resolve( + Object.assign( + new Response(` + + + + + + + + + `), + { contentType: "text/html" } + ) + ); // Create cache storage to reuse between requests const { caches } = createCacheStorage(); @@ -450,22 +466,7 @@ describe("asset-server handler", () => { metadata, findAssetEntryForPath, caches, - fetchAsset: () => - Promise.resolve( - Object.assign( - new Response(` - - - - - - - - - `), - { contentType: "text/html" } - ) - ), + fetchAsset, }); const { response, spies } = await getResponse(); @@ -476,17 +477,20 @@ describe("asset-server handler", () => { await Promise.all(spies.waitUntil); const earlyHintsCache = await caches.open(`eh:${deploymentId}`); - const earlyHintsRes = await earlyHintsCache.match("https://example.com/"); + const earlyHintsRes = await earlyHintsCache.match( + "https://example.com/asset-key-index.html" + ); if (!earlyHintsRes) { throw new Error( - "Did not match early hints cache on https://example.com/" + "Did not match early hints cache on https://example.com/asset-key-index.html" ); } expect(earlyHintsRes.headers.get("link")).toMatchInlineSnapshot( `"; rel="preload"; as=image, ; rel="preload"; as=image, ; rel="modulepreload", ; rel="preconnect""` ); + expect(response.headers.get("link")).toBeNull(); // Do it again, but this time ensure that we didn't write to cache again const { response: response2, spies: spies2 } = await getResponse(); @@ -497,17 +501,54 @@ describe("asset-server handler", () => { await Promise.all(spies2.waitUntil); - const earlyHintsRes2 = await earlyHintsCache.match("https://example.com/"); + const earlyHintsRes2 = await earlyHintsCache.match( + "https://example.com/asset-key-index.html" + ); if (!earlyHintsRes2) { throw new Error( - "Did not match early hints cache on https://example.com/" + "Did not match early hints cache on https://example.com/asset-key-index.html" ); } expect(earlyHintsRes2.headers.get("link")).toMatchInlineSnapshot( `"; rel="preload"; as=image, ; rel="preload"; as=image, ; rel="modulepreload", ; rel="preconnect""` ); + expect(response2.headers.get("link")).toMatchInlineSnapshot( + `"; rel="preload"; as=image, ; rel="preload"; as=image, ; rel="modulepreload", ; rel="preconnect""` + ); + + // Now make sure that requests for other paths which resolve to the same asset share the EH cache result + const { response: response3, spies: spies3 } = await getTestResponse({ + request: new Request("https://example.com/foo"), + metadata, + findAssetEntryForPath, + caches, + fetchAsset, + }); + + expect(response3.status).toBe(200); + // waitUntil should not be called at all (SPA) + expect(spies3.waitUntil.length).toBe(0); + + await Promise.all(spies3.waitUntil); + + const earlyHintsRes3 = await earlyHintsCache.match( + "https://example.com/asset-key-index.html" + ); + + if (!earlyHintsRes3) { + throw new Error( + "Did not match early hints cache on https://example.com/asset-key-index.html" + ); + } + + expect(earlyHintsRes3.headers.get("link")).toMatchInlineSnapshot( + `"; rel="preload"; as=image, ; rel="preload"; as=image, ; rel="modulepreload", ; rel="preconnect""` + ); + expect(response3.headers.get("link")).toMatchInlineSnapshot( + `"; rel="preload"; as=image, ; rel="preload"; as=image, ; rel="modulepreload", ; rel="preconnect""` + ); }); test("early hints should cache empty link headers", async () => { @@ -516,7 +557,7 @@ describe("asset-server handler", () => { const findAssetEntryForPath = async (path: string) => { if (path === "/index.html") { - return "index.html"; + return "asset-key-index.html"; } return null; @@ -554,15 +595,18 @@ describe("asset-server handler", () => { await Promise.all(spies.waitUntil); const earlyHintsCache = await caches.open(`eh:${deploymentId}`); - const earlyHintsRes = await earlyHintsCache.match("https://example.com/"); + const earlyHintsRes = await earlyHintsCache.match( + "https://example.com/asset-key-index.html" + ); if (!earlyHintsRes) { throw new Error( - "Did not match early hints cache on https://example.com/" + "Did not match early hints cache on https://example.com/asset-key-index.html" ); } expect(earlyHintsRes.headers.get("link")).toBeNull(); + expect(response.headers.get("link")).toBeNull(); // Do it again, but this time ensure that we didn't write to cache again const { response: response2, spies: spies2 } = await getResponse(); @@ -573,15 +617,18 @@ describe("asset-server handler", () => { await Promise.all(spies2.waitUntil); - const earlyHintsRes2 = await earlyHintsCache.match("https://example.com/"); + const earlyHintsRes2 = await earlyHintsCache.match( + "https://example.com/asset-key-index.html" + ); if (!earlyHintsRes2) { throw new Error( - "Did not match early hints cache on https://example.com/" + "Did not match early hints cache on https://example.com/asset-key-index.html" ); } expect(earlyHintsRes2.headers.get("link")).toBeNull(); + expect(response2.headers.get("link")).toBeNull(); }); test.todo( diff --git a/packages/pages-shared/asset-server/handler.ts b/packages/pages-shared/asset-server/handler.ts index 7e7db890c2e8..20d9ab35c161 100644 --- a/packages/pages-shared/asset-server/handler.ts +++ b/packages/pages-shared/asset-server/handler.ts @@ -347,6 +347,7 @@ export async function generateHandler< async function attachHeaders(response: Response) { const existingHeaders = new Headers(response.headers); + const eTag = existingHeaders.get("eTag")?.match(/^"(.*)"$/)?.[1]; const extraHeaders = new Headers({ "access-control-allow-origin": "*", @@ -364,13 +365,14 @@ export async function generateHandler< if ( earlyHintsCache && - isHTMLContentType(response.headers.get("Content-Type")) + isHTMLContentType(response.headers.get("Content-Type")) && + eTag ) { const preEarlyHintsHeaders = new Headers(headers); // "Early Hints cache entries are keyed by request URI and ignore query strings." // https://developers.cloudflare.com/cache/about/early-hints/ - const earlyHintsCacheKey = `${protocol}//${host}${pathname}`; + const earlyHintsCacheKey = `${protocol}//${host}/${eTag}`; const earlyHintsResponse = await earlyHintsCache.match(earlyHintsCacheKey); if (earlyHintsResponse) {