Skip to content

Commit a1941cd

Browse files
committed
Handle difference between max-age and s-maxage
1 parent 22f0d02 commit a1941cd

22 files changed

Lines changed: 527 additions & 792 deletions

CLAUDE.md

Lines changed: 0 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -78,13 +78,3 @@ Uses strict TypeScript configuration with:
7878
- Module: preserve (for bundler compatibility)
7979
- Strict mode with additional safety checks (`noUncheckedIndexedAccess`, `noImplicitOverride`)
8080
- Library-focused settings (declaration files, declaration maps)
81-
82-
## Use Specialized Agents for Complex Tasks
83-
84-
ALWAYS use the appropriate specialized agents for complex work:
85-
86-
- **technical-architect**: For designing system architecture, evaluating technical approaches, planning major features
87-
- **code-reviewer**: For comprehensive code review after implementing significant code changes
88-
- **test-engineer**: For analyzing test failures, creating new tests, and enhancing test coverage. Should NOT fix application code - only creates/updates test files
89-
- **docs-author**: For creating or updating documentation, READMEs, changesets, or PR descriptions
90-
- **package-installer**: For installing npm packages with proper dependency management

packages/cache-handlers/src/types.ts

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -271,6 +271,11 @@ export interface ParsedCacheHeaders {
271271
* Whether ETag should be generated if not present
272272
*/
273273
shouldGenerateETag?: boolean;
274+
275+
/**
276+
* Filtered cache-control header value after removing used directives
277+
*/
278+
filteredCacheControl?: string;
274279
}
275280

276281
/**

packages/cache-handlers/src/utils.ts

Lines changed: 86 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -125,32 +125,63 @@ export function parseResponseHeaders<TRequest, TResponse>(
125125
? headers.get("cdn-cache-control")
126126
: null;
127127

128-
const finalCacheControl = cdnCacheControlHeader || cacheControlHeader;
129-
130128
if (cdnCacheControlHeader) {
131129
result.headersToRemove.push("cdn-cache-control");
132130
}
133131

134-
if (finalCacheControl) {
135-
const directives = parseCacheControl(finalCacheControl);
136-
result.isPrivate = !!directives.private;
137-
result.noCache = !!directives["no-cache"];
138-
result.noStore = !!directives["no-store"];
132+
// Parse cdn-cache-control first (if present)
133+
if (cdnCacheControlHeader) {
134+
const cdnDirectives = parseCacheControl(cdnCacheControlHeader);
135+
result.isPrivate = !!cdnDirectives.private;
136+
result.noCache = !!cdnDirectives["no-cache"];
137+
result.noStore = !!cdnDirectives["no-store"];
138+
139+
// cdn-cache-control uses max-age
140+
if (typeof cdnDirectives["max-age"] === "number") {
141+
result.ttl = cdnDirectives["max-age"];
142+
}
143+
144+
if (typeof cdnDirectives["stale-while-revalidate"] === "number") {
145+
result.staleWhileRevalidate = cdnDirectives["stale-while-revalidate"];
146+
}
147+
}
139148

140-
if (typeof directives["max-age"] === "number") {
141-
result.ttl = directives["max-age"];
149+
// Parse regular cache-control (fallback for properties not set by cdn-cache-control)
150+
if (cacheControlHeader) {
151+
const directives = parseCacheControl(cacheControlHeader);
152+
153+
// Only use cache-control for properties not already set by cdn-cache-control
154+
if (!cdnCacheControlHeader) {
155+
result.isPrivate = !!directives.private;
156+
result.noCache = !!directives["no-cache"];
157+
result.noStore = !!directives["no-store"];
142158
}
143159

144-
if (typeof directives["stale-while-revalidate"] === "number") {
160+
// cache-control only uses s-maxage for TTL (ignore max-age)
161+
if (!result.ttl && typeof directives["s-maxage"] === "number") {
162+
result.ttl = directives["s-maxage"];
163+
}
164+
165+
if (!result.staleWhileRevalidate && typeof directives["stale-while-revalidate"] === "number") {
145166
result.staleWhileRevalidate = directives["stale-while-revalidate"];
146167
}
168+
169+
// Filter out used directives from cache-control
170+
const filteredCacheControl = filterCacheControlDirectives(cacheControlHeader);
171+
if (filteredCacheControl !== cacheControlHeader && filteredCacheControl.trim()) {
172+
// Only modify cache-control if we have remaining directives
173+
result.filteredCacheControl = filteredCacheControl;
174+
} else if (filteredCacheControl !== cacheControlHeader) {
175+
// If we filtered everything out, remove the header entirely
176+
result.headersToRemove.push("cache-control");
177+
}
147178
}
148179

149180
if (features.cacheTags !== false) {
150181
const cacheTag = headers.get("cache-tag");
151182
if (cacheTag) {
152183
result.tags = parseCacheTags(cacheTag);
153-
result.headersToRemove.push("cache-tag");
184+
// Cache tags should be preserved for clients, not removed
154185
}
155186
}
156187

@@ -188,7 +219,7 @@ export function parseResponseHeaders<TRequest, TResponse>(
188219
}
189220

190221
// Cache only when explicitly allowed by headers (no implicit caching)
191-
const hasExplicitCacheHeaders = !!finalCacheControl ||
222+
const hasExplicitCacheHeaders = !!cacheControlHeader || !!cdnCacheControlHeader ||
192223
!!headers.get("cache-tag") ||
193224
!!headers.get("expires");
194225
result.shouldCache = hasExplicitCacheHeaders &&
@@ -317,11 +348,44 @@ function getCookieValue(
317348
return null;
318349
}
319350

351+
/**
352+
* Remove cache-control directives that were processed by cache-handlers
353+
*/
354+
export function filterCacheControlDirectives(
355+
cacheControlValue: string,
356+
): string {
357+
const directives = parseCacheControl(cacheControlValue);
358+
359+
// Remove directives that cache-handlers processes
360+
const usedDirectives = [
361+
"s-maxage", // CDN-specific, we've processed it
362+
"stale-while-revalidate" // Our SWR implementation
363+
];
364+
365+
// Remove used directives
366+
for (const directive of usedDirectives) {
367+
delete directives[directive];
368+
}
369+
370+
// Rebuild cache-control header from remaining directives
371+
const remaining = Object.entries(directives)
372+
.map(([key, value]) => {
373+
if (value === true) {
374+
return key;
375+
}
376+
return `${key}=${value}`;
377+
})
378+
.filter(Boolean);
379+
380+
return remaining.join(", ");
381+
}
382+
320383
export function removeHeaders(
321384
response: Response,
322385
headersToRemove: string[],
386+
filteredCacheControl?: string,
323387
): Response {
324-
if (headersToRemove.length === 0) {
388+
if (headersToRemove.length === 0 && !filteredCacheControl) {
325389
return response;
326390
}
327391

@@ -330,6 +394,15 @@ export function removeHeaders(
330394
newHeaders.delete(headerName);
331395
}
332396

397+
// Apply filtered cache-control if provided
398+
if (filteredCacheControl !== undefined) {
399+
if (filteredCacheControl.trim()) {
400+
newHeaders.set("cache-control", filteredCacheControl);
401+
} else {
402+
newHeaders.delete("cache-control");
403+
}
404+
}
405+
333406
return new Response(response.body, {
334407
status: response.status,
335408
statusText: response.statusText,

packages/cache-handlers/src/write.ts

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -36,7 +36,7 @@ export async function writeToCache(
3636
noCache: cacheInfo.noCache,
3737
noStore: cacheInfo.noStore,
3838
});
39-
return removeHeaders(response, cacheInfo.headersToRemove);
39+
return removeHeaders(response, cacheInfo.headersToRemove, cacheInfo.filteredCacheControl);
4040
}
4141
const cacheKey = await getCacheKey(request, cacheInfo.vary);
4242
debug.logCacheWrite(request.url, cacheInfo.ttl, cacheInfo.tags);
@@ -83,5 +83,5 @@ export async function writeToCache(
8383
cacheInfo.vary,
8484
);
8585
}
86-
return removeHeaders(response, cacheInfo.headersToRemove);
86+
return removeHeaders(response, cacheInfo.headersToRemove, cacheInfo.filteredCacheControl);
8787
}

packages/cache-handlers/test/deno/conditional.test.ts

Lines changed: 16 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
import { assertEquals, assertExists } from "jsr:@std/assert";
1+
import { assert, assertEquals, assertExists } from "jsr:@std/assert";
22
import {
33
compareETags,
44
create304Response,
@@ -20,8 +20,8 @@ Deno.test("Conditional Requests - ETag generation", async () => {
2020

2121
assertExists(etag);
2222
assertEquals(typeof etag, "string");
23-
assertEquals(etag.startsWith('"'), true);
24-
assertEquals(etag.endsWith('"'), true);
23+
assert(etag.startsWith('"'));
24+
assert(etag.endsWith('"'));
2525
});
2626

2727
Deno.test("Conditional Requests - ETag parsing", () => {
@@ -33,7 +33,7 @@ Deno.test("Conditional Requests - ETag parsing", () => {
3333
// Weak ETag
3434
const weakETag = parseETag('W/"abc123"');
3535
assertEquals(weakETag.value, "abc123");
36-
assertEquals(weakETag.weak, true);
36+
assert(weakETag.weak);
3737

3838
// Empty ETag
3939
const emptyETag = parseETag("");
@@ -61,13 +61,13 @@ Deno.test("Conditional Requests - ETag comparison", () => {
6161
Deno.test("Conditional Requests - If-None-Match parsing", () => {
6262
// Single ETag
6363
const single = parseIfNoneMatch('"abc123"');
64-
assertEquals(Array.isArray(single), true);
64+
assert(Array.isArray(single));
6565
assertEquals((single as string[]).length, 1);
6666
assertEquals((single as string[])[0], '"abc123"');
6767

6868
// Multiple ETags
6969
const multiple = parseIfNoneMatch('"abc123", "def456", W/"ghi789"');
70-
assertEquals(Array.isArray(multiple), true);
70+
assert(Array.isArray(multiple));
7171
assertEquals((multiple as string[]).length, 3);
7272

7373
// Wildcard
@@ -76,14 +76,14 @@ Deno.test("Conditional Requests - If-None-Match parsing", () => {
7676

7777
// Empty
7878
const empty = parseIfNoneMatch("");
79-
assertEquals(Array.isArray(empty), true);
79+
assert(Array.isArray(empty));
8080
assertEquals((empty as string[]).length, 0);
8181
});
8282

8383
Deno.test("Conditional Requests - HTTP date parsing", () => {
8484
const validDate = parseHttpDate("Wed, 21 Oct 2015 07:28:00 GMT");
8585
assertExists(validDate);
86-
assertEquals(validDate instanceof Date, true);
86+
assert(validDate instanceof Date);
8787

8888
const invalidDate = parseHttpDate("invalid date");
8989
assertEquals(invalidDate, null);
@@ -108,8 +108,8 @@ Deno.test("Conditional Requests - validateConditionalRequest with ETag", () => {
108108

109109
const result = validateConditionalRequest(request, cachedResponse);
110110

111-
assertEquals(result.matches, true);
112-
assertEquals(result.shouldReturn304, true);
111+
assert(result.matches);
112+
assert(result.shouldReturn304);
113113
assertEquals(result.matchedValidator, "etag");
114114
});
115115

@@ -134,8 +134,8 @@ Deno.test(
134134

135135
const result = validateConditionalRequest(request, cachedResponse);
136136

137-
assertEquals(result.matches, true);
138-
assertEquals(result.shouldReturn304, true);
137+
assert(result.matches);
138+
assert(result.shouldReturn304);
139139
assertEquals(result.matchedValidator, "last-modified");
140140
},
141141
);
@@ -250,7 +250,7 @@ Deno.test("Conditional Requests - unified handler ETag generation", async () =>
250250
Promise.resolve(
251251
new Response("etag-body", {
252252
headers: {
253-
"cache-control": "public, max-age=3600",
253+
"cache-control": "public, s-maxage=3600",
254254
"content-type": "application/json",
255255
},
256256
}),
@@ -267,9 +267,9 @@ Deno.test("Conditional Requests - unified handler ETag generation", async () =>
267267
Deno.test("Conditional Requests - Default configuration", () => {
268268
const config = getDefaultConditionalConfig();
269269

270-
assertEquals(config.etag, true);
271-
assertEquals(config.lastModified, true);
272-
assertEquals(config.weakValidation, true);
270+
assert(config.etag);
271+
assert(config.lastModified);
272+
assert(config.weakValidation);
273273
});
274274

275275
Deno.test("Conditional Requests - disabled returns full response", async () => {

0 commit comments

Comments
 (0)