Cache: route all caching through the cache override - #45
Conversation
commit: |
![devin-ai-integration[bot]](https://avatars.githubusercontent.com/in/811515?s=60&v=4){kind=small}
| if (derivedTags.length > 0) { | ||
| const storedTags = await tagCache.getByPath(key); | ||
| const tagsToWrite = derivedTags.filter((tag) => !storedTags.includes(tag)); | ||
| if (tagsToWrite.length > 0) { | ||
| await writeTags( | ||
| tagsToWrite.map((tag) => ({ | ||
| path: key, | ||
| tag, | ||
| revalidatedAt: 1, | ||
| })), | ||
| tagCache | ||
| ); | ||
| } | ||
| } | ||
| } |
There was a problem hiding this comment.
🔴 Cache tag records are never saved when the cache runs as its own service
Tags derived from a cache entry are recorded (writeTags(...) at packages/core/src/adapters/cache-adapter.ts:231) outside of any request context, and the recording helper silently gives up when no request context exists, so nothing is stored when the cache runs as a separate function.
Impact: With a path-based ("original") tag cache, later tag revalidation finds no entries to invalidate and users keep being served stale pages and fetch data.
Missing OpenNext request context in the cache handler's set path
writeTags (packages/core/src/utils/cache.ts:65-69) starts with const store = globalThis.__openNextAls.getStore(); if (!store || ...) return;. The cache handler module creates a fresh AsyncLocalStorage at load (packages/core/src/adapters/cache-adapter.ts:22) and createGenericHandler never enters a store, and neither does the Cloudflare entrypoint (packages/cloudflare/src/cli/templates/cache-entrypoint.ts:22-26). handleRevalidateTags explicitly wraps its work with runWithOpenNextRequestContext (packages/core/src/adapters/cache-adapter.ts:282) but handleSet does not, so every writeTags call from handleSet returns early whenever the cache handler is invoked out-of-process (the fetch cache client, or the Cloudflare OpenNextCache service binding). The new unit test passes only because runHandler in packages/tests-unit/tests/adapters/cache-adapter.test.ts:72-86 manually installs a store.
Prompt for agents
In packages/core/src/adapters/cache-adapter.ts, handleSet() calls writeTags() to persist derived tag/path pairs, but writeTags (packages/core/src/utils/cache.ts) short-circuits when globalThis.__openNextAls.getStore() is undefined. The cache handler function is not executed inside an OpenNext request context: createGenericHandler does not create one and the Cloudflare OpenNextCache entrypoint only wraps the Cloudflare context. handleRevalidateTags already works around this by wrapping its body with runWithOpenNextRequestContext. Apply the same treatment to the set path (or wrap the whole defaultHandler in a request context) so that tag writes actually happen when the cache handler runs as a separate function/service.
Was this helpful? React with 👍 or 👎 to provide feedback.
| case "FETCH": | ||
| await globalThis.incrementalCache.set(key, data, "fetch"); | ||
| await globalThis.cache.set(key, data, "fetch"); | ||
| break; |
There was a problem hiding this comment.
🔴 Tags attached to cached fetch responses are lost, so revalidating a tag no longer clears fetch data
The tags that Next.js hands over with a cached fetch response are dropped when the entry is stored (globalThis.cache.set(key, data, "fetch") at packages/core/src/adapters/cache.ts:225), so nothing links the stored fetch data to its tags.
Impact: With a path-based ("original") tag cache, revalidating a tag no longer clears the matching cached fetch responses and stale data keeps being served.
ctx.tags dropped between the server cache handler and the cache function
The removed updateTagsOnSet used ctx?.tags ?? data?.data?.tags ?? [] for FETCH entries, because Next.js passes the fetch tags in the set context, not inside the value: see the comment on CachedFetchValue.tags in packages/core/src/types/overrides.ts:87-89 ("tags are only present with file-system-cache, fetch cache stores tags outside of cache entry").
The new flow never forwards ctx: Cache.set sends only the value, and handleSet derives fetch tags from fetchValue.tags ?? data?.tags (packages/core/src/adapters/cache-adapter.ts:218-221), which are absent for the fetch cache. As a result no tag/path rows are written for fetch keys, and handleGet's original-mode check (getLastModified(fetchKey, ...) at packages/core/src/adapters/cache-adapter.ts:183) can never report the entry as revalidated. This compounds with the removal of the soft-tag path fallback previously in getFetchCache, which checked the owning page path when the entry itself had no tags.
Prompt for agents
Fetch cache entries lose their tags in the new cache pipeline. Next.js supplies fetch tags through the set context (ctx.tags), not inside the cached value (see the comment on CachedFetchValue.tags in packages/core/src/types/overrides.ts). The old updateTagsOnSet in packages/core/src/adapters/cache.ts used ctx?.tags ?? data?.data?.tags. Now Cache.set only forwards the value to globalThis.cache.set, and handleSet in packages/core/src/adapters/cache-adapter.ts derives fetch tags from the value alone, so nothing is written for original-mode tag caches and revalidateTag can never invalidate fetch entries. Consider extending the Cache override's set() signature with the additional tags (mirroring what was done for get()), plumbing them through the HTTP layer (fetch/local/service cache clients and the ?tags= query parameter) and merging them into derivedTags in handleSet.
Was this helpful? React with 👍 or 👎 to provide feedback.
| if (result.value && !result.shouldBypassTagCache) { | ||
| let tags: string[] = [...additionalTags]; | ||
|
|
||
| if (cacheType === "cache") { | ||
| tags = [...tags, ...getTagsFromValue(result.value as CacheValue<"cache">)]; | ||
| } else if (cacheType === "fetch") { | ||
| const fetchValue = result.value as CachedFetchValue; | ||
| tags = [...tags, ...(fetchValue.tags ?? []), ...(fetchValue.data?.tags ?? [])]; | ||
| } else if (cacheType === "composable") { | ||
| const composableValue = result.value as StoredComposableCacheEntry; | ||
| tags = [...tags, ...(composableValue.tags ?? [])]; | ||
| } |
There was a problem hiding this comment.
🟡 Internal Next.js cache-tag header can leak into responses sent to browsers
The internal list of cache tags is only stripped from a cached entry when the tag check runs, so entries that skip that check (if (result.value && !result.shouldBypassTagCache) at packages/core/src/adapters/cache-adapter.ts:133) keep the internal header and hand it back with the page.
Impact: Responses served from such cache entries can expose internal cache tag names to end users.
getTagsFromValue also performs the header deletion
getTagsFromValue (packages/core/src/utils/cache.ts:35-47) both reads and deletes value.meta.headers["x-next-cache-tags"]. Previously it was called unconditionally in getIncrementalCache and in cacheInterceptor, so the header was always removed before the value was turned into a response. In the new handler it is only called inside the !result.shouldBypassTagCache branch; when an incremental cache reports shouldBypassTagCache (e.g. a freshly deployed/populated entry), the header survives, is echoed as x-opennext-cache-header-x-next-cache-tags by buildCachedFileResponse (packages/core/src/adapters/cache-adapter.ts:436-442), and ends up in meta.headers of the value returned to Next.js, which forwards it to the client.
Prompt for agents
In handleGet (packages/core/src/adapters/cache-adapter.ts), getTagsFromValue() is only invoked when shouldBypassTagCache is false, but that helper is also what deletes the internal x-next-cache-tags header from the stored value (see packages/core/src/utils/cache.ts). Before this change the deletion happened unconditionally in adapters/cache.ts and cacheInterceptor.ts. Restructure handleGet so the tag extraction/stripping for cacheType === "cache" always runs, and only the revalidation check is skipped when shouldBypassTagCache is set.
Was this helpful? React with 👍 or 👎 to provide feedback.
The incremental cache and the tag cache stop running inside the server function. They only live in the cache handler function now, and the server, the middleware and the composable cache reach them through the `cache` override. - Remove `incrementalCache` and `tagCache` from `OverrideOptions`; they are only configured under `cacheHandler`. `resolveIncrementalCache` and `resolveTagCache` follow. The esbuild resolve plugin keeps its own fields, so adapter `defaultOverrides` are unaffected. - Move tag revalidation - `hasBeenRevalidated`, `writeTags` and CDN invalidation - out of `adapters/cache.ts`, `composable-cache.ts` and `cacheInterceptor.ts` and into the cache handler, which now applies them in `get`, `set` and `revalidateTags`. - `Cache.get` takes the additional tags to check, so the caller no longer needs the tag cache to resolve them. - `defineCloudflareConfig` wires `cache` to the `OpenNextCache` entrypoint and moves the incremental cache, the tag cache and the cdn invalidation to `cacheHandler`; `ensureCloudflareConfig`, `populateCache` and `isPurgeCacheEnabled` read the new location. - Examples move to `cache: "local"` with a `cacheHandler` block. BREAKING CHANGE: `default.override.incrementalCache` and `default.override.tagCache` are replaced by the top level `cacheHandler` option and `default.override.cache`.
c070080 to
dc444fc
Compare
e65c368 to
8b63860
Compare
1 participant
This is the breaking change of the stack, and the only PR that changes runtime wiring for both adapters.
incrementalCacheandtagCachefromOverrideOptions; they are only configured undercacheHandler.resolveIncrementalCacheandresolveTagCachefollow. The esbuild resolve plugin keeps its own fields, so adapterdefaultOverridesare unaffected.hasBeenRevalidated,writeTags, CDN invalidation — out ofadapters/cache.ts,composable-cache.tsandcacheInterceptor.tsinto the cache handler, which now applies them transparently inget,setandrevalidateTags.Cache.gettakes the additional tags to check, so the caller no longer needs the tag cache to resolve them.defineCloudflareConfigwirescacheto theOpenNextCacheentrypoint from PR 2 and moves the incremental cache, tag cache and cdn invalidation tocacheHandler.ensureCloudflareConfig,populateCacheandisPurgeCacheEnabledread the new location.cache: "local"with acacheHandlerblock.Roughly 400 lines of source; the rest is the corresponding rewrite of
cache.test.ts,composable-cache.test.tsandcacheInterceptor.test.ts, plus the newcache-adapter.test.ts.Migration for configurations not created by
defineCloudflareConfig:default: { override: { - incrementalCache: "s3", - tagCache: "dynamodb", + cache: "local", }, }, + cacheHandler: { + incrementalCache: "s3", + tagCache: "dynamodb", + },