Skip to content

feat: add cache() middleware for server-side response caching#3782

Open
bartlomieju wants to merge 1 commit into
mainfrom
feat/cache-middleware
Open

feat: add cache() middleware for server-side response caching#3782
bartlomieju wants to merge 1 commit into
mainfrom
feat/cache-middleware

Conversation

@bartlomieju
Copy link
Copy Markdown
Contributor

Summary

  • Adds a cache() middleware that uses the Web Cache API for server-side response caching
  • Routes opt in via standard Cache-Control response headers — no new config surface or framework-specific DSL needed
  • Supports stale-while-revalidate for ISR-like behavior (serve stale immediately, regenerate in background)
  • Skips caching for non-GET, private/no-store, Set-Cookie, non-200, and partial requests
  • Configurable via cacheName, methods, and custom shouldCache function
  • Follows existing middleware patterns (staticFiles(), cors(), etc.)

Usage

import { App, cache, staticFiles } from "fresh";

const app = new App()
  .use(staticFiles())
  .use(cache());

Routes opt in:

return page({ data }, {
  headers: { "Cache-Control": "public, max-age=60, stale-while-revalidate=300" },
});

Closes #8

Test plan

  • 13 unit tests covering: miss/hit, skip non-GET, skip private/no-store/Set-Cookie/non-200/partial, separate paths, expiry, stale-while-revalidate, custom shouldCache, custom methods
  • Integration testing with a real Fresh app

Adds a first-party middleware that caches responses using the Web Cache
API. Routes opt in via standard Cache-Control headers — no new config
surface needed. Supports stale-while-revalidate for ISR-like background
revalidation.

Closes #8
@bartlomieju
Copy link
Copy Markdown
Contributor Author

Nice work — the implementation is clean and follows the existing middleware conventions well. A few things that aren't covered here that might be worth considering as follow-ups:

Pluggable cache backend — This uses the Web Cache API exclusively, which works great on Deno Deploy but ties caching to the runtime. A storage option (or similar) that accepts a custom backend (Redis, in-memory LRU, etc.) would make this useful in self-hosted/Docker deployments where the Web Cache API may behave differently or not persist across restarts.

CDN cache coordination — For apps behind a CDN (Cloudflare, Fastly, etc.), it's common to need CDN-Cache-Control or Surrogate-Control headers that differ from the client-facing Cache-Control. The middleware could grow a hook for setting upstream cache headers independently, or at minimum the docs could note how to layer CDN headers alongside this middleware.

Vary header awareness — Right now the cache key is the request URL. If a route returns different content based on Accept, Accept-Language, or Accept-Encoding headers (common for i18n or content negotiation), the same URL would serve the wrong cached response. Respecting Vary in the cache key would prevent subtle bugs.

None of these block the initial merge — the current scope is solid and the SWR via queueMicrotask is a nice touch. Just flagging them as potential next steps.

Copy link
Copy Markdown
Contributor

@lunadogbot lunadogbot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

CI is red — Type check project is failing on all six matrix jobs (the workflow log just prints error: Type checking failed. without the offending file, so worth re-running with verbose output to surface it). That's a hard blocker for approval.

A few concrete things in packages/fresh/src/middlewares/cache.ts worth addressing while you're at it:

  1. Custom shouldCache bypasses the default safety net. Line 104 swaps in the user's predicate wholesale, so status !== 200, Set-Cookie, private/no-store, and the ?fresh-partial skip all silently disappear. The test on line 311 caches a response that has no Cache-Control header at all — almost certainly not what a user expects. Consider AND-ing the user predicate with the built-in checks (or at minimum documenting the trade-off on CacheOptions.shouldCache).
  2. methods: ["POST"|"HEAD"|...] will throw at runtime. Per spec, Cache.put rejects non-GET requests with a TypeError. The custom methods test on line 337 declares ["GET", "HEAD"] but only exercises server.get, so the HEAD path is uncovered — first real HEAD request will explode in store.put. Either restrict methods to GET-only or guard the put.
  3. Body-stream leak on corrupt entries. Line 145's await cached.body?.cancel() lives inside if (cachedAtStr !== null). If a cached Response is missing X-Fresh-Cached-At (older middleware version, manual cache writes), the body never gets canceled before the fall-through ctx.next().
  • nit: the maintainer self-comment already flags Vary, pluggable backend, and CDN headers as follow-ups — agree on all three, especially Vary, which is the most common foot-gun.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Caching of pages

2 participants