Skip to content

Commit 147ab7d

Browse files
GregBrimbleGregBrimble
authored
Key the Early Hints off of the asset key rather than request path (#7564)
* Refactor ETag handling * Check if-none-match before fulfilling preservation cache * Skip parsing Early Hints for known empty results * Key the Early Hints off of the asset key rather than request path * Fix rebased changesets
1 parent fe1e344 commit 147ab7d

File tree

3 files changed

+82
-28
lines changed

3 files changed

+82
-28
lines changed

‎.changeset/cold-turtles-impress.md

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
---
2+
"@cloudflare/pages-shared": patch
3+
---
4+
5+
fix: Key the Early Hints cache entries off of the asset key rather than the request path

‎packages/pages-shared/__tests__/asset-server/handler.test.ts

Lines changed: 73 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -435,11 +435,27 @@ describe("asset-server handler", () => {
435435

436436
const findAssetEntryForPath = async (path: string) => {
437437
if (path === "/index.html") {
438-
return "index.html";
438+
return "asset-key-index.html";
439439
}
440440

441441
return null;
442442
};
443+
const fetchAsset = () =>
444+
Promise.resolve(
445+
Object.assign(
446+
new Response(`
447+
<!DOCTYPE html>
448+
<html>
449+
<body>
450+
<link rel="preload" as="image" href="/a.png" />
451+
<link rel="preload" as="image" href="/b.png" />
452+
<link rel="modulepreload" href="lib.js" />
453+
<link rel="preconnect" href="cloudflare.com" />
454+
</body>
455+
</html>`),
456+
{ contentType: "text/html" }
457+
)
458+
);
443459

444460
// Create cache storage to reuse between requests
445461
const { caches } = createCacheStorage();
@@ -450,22 +466,7 @@ describe("asset-server handler", () => {
450466
metadata,
451467
findAssetEntryForPath,
452468
caches,
453-
fetchAsset: () =>
454-
Promise.resolve(
455-
Object.assign(
456-
new Response(`
457-
<!DOCTYPE html>
458-
<html>
459-
<body>
460-
<link rel="preload" as="image" href="/a.png" />
461-
<link rel="preload" as="image" href="/b.png" />
462-
<link rel="modulepreload" href="lib.js" />
463-
<link rel="preconnect" href="cloudflare.com" />
464-
</body>
465-
</html>`),
466-
{ contentType: "text/html" }
467-
)
468-
),
469+
fetchAsset,
469470
});
470471

471472
const { response, spies } = await getResponse();
@@ -476,17 +477,20 @@ describe("asset-server handler", () => {
476477
await Promise.all(spies.waitUntil);
477478

478479
const earlyHintsCache = await caches.open(`eh:${deploymentId}`);
479-
const earlyHintsRes = await earlyHintsCache.match("https://example.com/");
480+
const earlyHintsRes = await earlyHintsCache.match(
481+
"https://example.com/asset-key-index.html"
482+
);
480483

481484
if (!earlyHintsRes) {
482485
throw new Error(
483-
"Did not match early hints cache on https://example.com/"
486+
"Did not match early hints cache on https://example.com/asset-key-index.html"
484487
);
485488
}
486489

487490
expect(earlyHintsRes.headers.get("link")).toMatchInlineSnapshot(
488491
`"</a.png>; rel="preload"; as=image, </b.png>; rel="preload"; as=image, <lib.js>; rel="modulepreload", <cloudflare.com>; rel="preconnect""`
489492
);
493+
expect(response.headers.get("link")).toBeNull();
490494

491495
// Do it again, but this time ensure that we didn't write to cache again
492496
const { response: response2, spies: spies2 } = await getResponse();
@@ -497,17 +501,54 @@ describe("asset-server handler", () => {
497501

498502
await Promise.all(spies2.waitUntil);
499503

500-
const earlyHintsRes2 = await earlyHintsCache.match("https://example.com/");
504+
const earlyHintsRes2 = await earlyHintsCache.match(
505+
"https://example.com/asset-key-index.html"
506+
);
501507

502508
if (!earlyHintsRes2) {
503509
throw new Error(
504-
"Did not match early hints cache on https://example.com/"
510+
"Did not match early hints cache on https://example.com/asset-key-index.html"
505511
);
506512
}
507513

508514
expect(earlyHintsRes2.headers.get("link")).toMatchInlineSnapshot(
509515
`"</a.png>; rel="preload"; as=image, </b.png>; rel="preload"; as=image, <lib.js>; rel="modulepreload", <cloudflare.com>; rel="preconnect""`
510516
);
517+
expect(response2.headers.get("link")).toMatchInlineSnapshot(
518+
`"</a.png>; rel="preload"; as=image, </b.png>; rel="preload"; as=image, <lib.js>; rel="modulepreload", <cloudflare.com>; rel="preconnect""`
519+
);
520+
521+
// Now make sure that requests for other paths which resolve to the same asset share the EH cache result
522+
const { response: response3, spies: spies3 } = await getTestResponse({
523+
request: new Request("https://example.com/foo"),
524+
metadata,
525+
findAssetEntryForPath,
526+
caches,
527+
fetchAsset,
528+
});
529+
530+
expect(response3.status).toBe(200);
531+
// waitUntil should not be called at all (SPA)
532+
expect(spies3.waitUntil.length).toBe(0);
533+
534+
await Promise.all(spies3.waitUntil);
535+
536+
const earlyHintsRes3 = await earlyHintsCache.match(
537+
"https://example.com/asset-key-index.html"
538+
);
539+
540+
if (!earlyHintsRes3) {
541+
throw new Error(
542+
"Did not match early hints cache on https://example.com/asset-key-index.html"
543+
);
544+
}
545+
546+
expect(earlyHintsRes3.headers.get("link")).toMatchInlineSnapshot(
547+
`"</a.png>; rel="preload"; as=image, </b.png>; rel="preload"; as=image, <lib.js>; rel="modulepreload", <cloudflare.com>; rel="preconnect""`
548+
);
549+
expect(response3.headers.get("link")).toMatchInlineSnapshot(
550+
`"</a.png>; rel="preload"; as=image, </b.png>; rel="preload"; as=image, <lib.js>; rel="modulepreload", <cloudflare.com>; rel="preconnect""`
551+
);
511552
});
512553

513554
test("early hints should cache empty link headers", async () => {
@@ -516,7 +557,7 @@ describe("asset-server handler", () => {
516557

517558
const findAssetEntryForPath = async (path: string) => {
518559
if (path === "/index.html") {
519-
return "index.html";
560+
return "asset-key-index.html";
520561
}
521562

522563
return null;
@@ -554,15 +595,18 @@ describe("asset-server handler", () => {
554595
await Promise.all(spies.waitUntil);
555596

556597
const earlyHintsCache = await caches.open(`eh:${deploymentId}`);
557-
const earlyHintsRes = await earlyHintsCache.match("https://example.com/");
598+
const earlyHintsRes = await earlyHintsCache.match(
599+
"https://example.com/asset-key-index.html"
600+
);
558601

559602
if (!earlyHintsRes) {
560603
throw new Error(
561-
"Did not match early hints cache on https://example.com/"
604+
"Did not match early hints cache on https://example.com/asset-key-index.html"
562605
);
563606
}
564607

565608
expect(earlyHintsRes.headers.get("link")).toBeNull();
609+
expect(response.headers.get("link")).toBeNull();
566610

567611
// Do it again, but this time ensure that we didn't write to cache again
568612
const { response: response2, spies: spies2 } = await getResponse();
@@ -573,15 +617,18 @@ describe("asset-server handler", () => {
573617

574618
await Promise.all(spies2.waitUntil);
575619

576-
const earlyHintsRes2 = await earlyHintsCache.match("https://example.com/");
620+
const earlyHintsRes2 = await earlyHintsCache.match(
621+
"https://example.com/asset-key-index.html"
622+
);
577623

578624
if (!earlyHintsRes2) {
579625
throw new Error(
580-
"Did not match early hints cache on https://example.com/"
626+
"Did not match early hints cache on https://example.com/asset-key-index.html"
581627
);
582628
}
583629

584630
expect(earlyHintsRes2.headers.get("link")).toBeNull();
631+
expect(response2.headers.get("link")).toBeNull();
585632
});
586633

587634
test.todo(

‎packages/pages-shared/asset-server/handler.ts

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -347,6 +347,7 @@ export async function generateHandler<
347347

348348
async function attachHeaders(response: Response) {
349349
const existingHeaders = new Headers(response.headers);
350+
const eTag = existingHeaders.get("eTag")?.match(/^"(.*)"$/)?.[1];
350351

351352
const extraHeaders = new Headers({
352353
"access-control-allow-origin": "*",
@@ -364,13 +365,14 @@ export async function generateHandler<
364365

365366
if (
366367
earlyHintsCache &&
367-
isHTMLContentType(response.headers.get("Content-Type"))
368+
isHTMLContentType(response.headers.get("Content-Type")) &&
369+
eTag
368370
) {
369371
const preEarlyHintsHeaders = new Headers(headers);
370372

371373
// "Early Hints cache entries are keyed by request URI and ignore query strings."
372374
// https://developers.cloudflare.com/cache/about/early-hints/
373-
const earlyHintsCacheKey = `${protocol}//${host}${pathname}`;
375+
const earlyHintsCacheKey = `${protocol}//${host}/${eTag}`;
374376
const earlyHintsResponse =
375377
await earlyHintsCache.match(earlyHintsCacheKey);
376378
if (earlyHintsResponse) {

0 commit comments

Comments
 (0)