Repository navigation
Cache: route all caching through the cache override - #45
Conversation
…y into core adapter feat: enhance build process with additional server bundle customization options chore: update package.json scripts for improved build and testing workflow test: add unit tests for adapter build process and server bundle generation fix: ensure proper handling of external dependencies and edge configuration in server bundle
…ests for resolve plugin
…specific overrides
…udflare specific overrides" This reverts commit 37c4d90.
…ad of throwing - Extract ValidateConfigResult type with success/message/shouldThrow/level - Convert validateFunctionOptions and validateSplittedFunctionOptions to return result objects - Remove logger dependency from validateConfig.ts - Preserve compatibilityMatrix, TODO comment, @ts-expect-error pragmas - Add 5 characterization tests in validateConfig.spec.ts - No caller impact: compileConfig.ts is the sole importer (updated in T3)
- Export OpenNextOutput interface (was internal) - Extract buildOpenNextOutput(buildOpts) for construction-only (no fs write) - Keep legacy generateOutput as thin wrapper (construction + file write) - Preserve all construction logic verbatim, including @ts-expect-error - Add 3 characterization tests in generateOutput.spec.ts - Backward compatible: byte-equivalent output to today
- Replace bare validateConfig(config) call with result-handling block - Throw on shouldThrow:true (bad routes — preserves existing behavior) - Log at appropriate level on shouldThrow:false (level field from T1) - All 3 export signatures and edge-runtime detection block unchanged - Direct callers (aws/build.ts, cloudflare/utils.ts) unaffected
…OpenNextAdapterOptions - Make OpenNextAdapterOptions<T = OpenNextOutput> and buildAdapter<T> generic - Add validateConfig override hook (runs after callback in modifyConfig) - Add generateOutput override hook (returns T, gated by skipGenerateOutput) - buildAdapter serializes override return via fs.writeFileSync (override never touches fs) - Default path uses buildOpenNextOutput (extracted in T2) - Add 5 new tests covering override behaviors + default path + skipGenerateOutput - All 16 existing adapter tests preserved; AWS/Cloudflare adapters compile with default T
When an adapter config specifies full package-specifier paths (e.g., @opennextjs/aws/overrides/wrappers/aws-lambda.js), esbuild cannot resolve them during bundling. Use createRequire(args.path).resolve() in the openNextResolvePlugin to convert package specifiers to filesystem-relative paths at build time, falling back to the original value if resolution fails. This fixes the openbuild:local build error: ERROR: Could not resolve "@opennextjs/aws/overrides/wrappers/aws-lambda.js" ERROR: Could not resolve "@opennextjs/aws/overrides/tagCache/dynamodb.js" Added test I verifying resolution of a mock package in node_modules.
…solution and improve path handling
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.
Add a standalone cache handler function that owns the incremental cache, the tag cache and CDN invalidation, and a `cache` override that other functions use to reach it. - `adapters/cache-adapter.ts` exposes `GET/PUT/DELETE /cache/*` and `POST /cache/revalidate-tags` through `createGenericHandler`. - `build/createCacheBundle.ts` bundles it to `.open-next/cache-function`, wired into `buildAdapter` behind the new `skipCache` option and reported as `cacheFunction` in the OpenNext output. - New top level `cacheHandler` config option carries the incremental cache, tag cache and cdn invalidation overrides for that function. - New `cache` override with `fetch`, `local` and `dummy` implementations, and a shared wire format in `utils/cache-get.ts` that splits the entry metadata into headers and the payload into the body. Nothing is removed here: the existing in-process `incrementalCache` and `tagCache` overrides keep working unchanged.
Build the OpenNext cache handler function as part of the worker and expose it through a named entrypoint, so the cache can later run behind a service binding - in this worker or in one of its own. - `service-cache.ts` is a `cache` override that speaks the cache handler HTTP API over the `NEXT_CACHE_SERVICE` binding. - `buildCacheFunction` emits the bundle from `beforeServerBundle`, because the worker imports the entrypoint; core's own cache step is skipped. - `runWithCloudflareContext` initialises the Cloudflare context outside of a request, for entrypoints invoked over RPC. The origin is now populated separately, on the first call coming from the fetch handler. - `withoutSelfEntrypointServices` strips the self referencing binding before handing the config to `getPlatformProxy`, which cannot resolve a named entrypoint of the worker it is configuring. - Drop `compile-cache-assets-manifest.ts`, unreferenced. `defineCloudflareConfig` is untouched: the entrypoint is built and exported but nothing routes to it yet.
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
|
rebase + fixes /cc @conico974 Additional Commit Details
|
There was a problem hiding this comment.
Copilot review overview
🟡 Changes recommended
Critical AWS cache wiring and moderate cache-handler correctness issues remain unresolved.
Get a fresh assessment by requesting another Copilot review.
Review effort: Lite
Findings: 1
Open (2)
What changed in this PR
This breaking PR routes caching and tag revalidation through the dedicated cache override and cache handler.
Changes:
- Moves cache providers under
cacheHandler. - Adds tag propagation through cache clients.
- Rewires AWS and Cloudflare adapters.
- Updates builds, tests, and examples.
| File | Summary |
|---|---|
packages/tests-unit/tests/overrides/cache/local.test.ts |
Tests local tag forwarding. |
packages/tests-unit/tests/overrides/cache/fetch.test.ts |
Tests fetch tag forwarding. |
packages/tests-unit/tests/core/routing/cacheInterceptor.test.ts |
Updates interceptor tests. |
packages/tests-unit/tests/adapters/composable-cache.test.ts |
Updates composable cache tests. |
packages/tests-unit/tests/adapters/cache.test.ts |
Updates cache adapter tests. |
packages/tests-unit/tests/adapters/cache-adapter.test.ts |
Adds cache-handler coverage. |
packages/tests-unit/tests/adapters/aws-adapter.test.ts |
Tests AWS cache defaults. |
packages/core/src/utils/cache.ts |
Supports explicit tag-cache dependencies. |
packages/core/src/types/overrides.ts |
Extends cache APIs with tags. |
packages/core/src/types/open-next.ts |
Moves cache provider configuration. |
packages/core/src/types/global.ts |
Updates cache globals. |
packages/core/src/plugins/resolve.ts |
Preserves provider bundle resolution. |
packages/core/src/overrides/cache/local.ts |
Forwards tags locally. |
packages/core/src/overrides/cache/fetch.ts |
Forwards tags over HTTP. |
packages/core/src/core/routing/cacheInterceptor.ts |
Routes interception through the cache client. |
packages/core/src/core/resolve.ts |
Resolves cache-handler providers; nit (1 vote): public fallback documentation is stale. |
packages/core/src/core/createMainHandler.ts |
Initializes cache client wiring. |
packages/core/src/build/middleware/buildNodeMiddleware.ts |
Removes legacy middleware providers. |
packages/core/src/build/generateOutput.ts |
Reports cache provider metadata. |
packages/core/src/build/generateOutput.spec.ts |
Tests generated metadata. |
packages/core/src/build/edge/createEdgeBundle.ts |
Removes legacy edge providers. |
packages/core/src/build/createCacheBundle.ts |
Bundles cache-handler providers. |
packages/core/src/build/createCacheBundle.spec.ts |
Tests provider bundling. |
packages/core/src/adapters/middleware.ts |
Uses the cache client in middleware. |
packages/core/src/adapters/composable-cache.ts |
Routes composable operations through cache. |
packages/core/src/adapters/cache.ts |
Routes incremental operations through cache. |
packages/core/src/adapters/cache-handler.ts |
Centralizes cache and tag handling; moderate findings (3 and 1 votes): preserves empty-tag invalidation checks and merges both tag fields. |
packages/cloudflare/src/cli/commands/populate-cache.ts |
Reads cache providers from cacheHandler. |
packages/cloudflare/src/cli/commands/populate-cache.spec.ts |
Updates populate-cache tests. |
packages/cloudflare/src/cli/build/utils/ensure-cf-config.ts |
Validates Cloudflare wiring; nit (1 vote): error text references removed fields. |
packages/cloudflare/src/cli/build/utils/ensure-cf-config.spec.ts |
Updates validation tests. |
packages/cloudflare/src/api/overrides/internal.ts |
Reads CDN invalidation configuration. |
packages/cloudflare/src/api/overrides/cache/service-cache.ts |
Forwards tags through service bindings. |
packages/cloudflare/src/api/overrides/cache/service-cache.spec.ts |
Tests service-cache tag forwarding. |
packages/cloudflare/src/api/config.ts |
Wires Cloudflare cache and cache handler. |
packages/cloudflare/src/api/config.spec.ts |
Updates Cloudflare configuration tests. |
packages/aws/src/adapter.ts |
Adds AWS cache defaults; critical (1 vote): the default cache URL is not wired to a deployed endpoint. |
examples/pages-router/open-next.config.ts |
Migrates example cache configuration. |
examples/experimental/open-next.config.ts |
Migrates experimental cache configuration. |
examples/app-router/open-next.config.ts |
Migrates app-router cache configuration. |
examples/app-pages-router/open-next.config.ts |
Migrates combined-router configuration. |
.changeset/cache-consolidate-override.md |
Documents the breaking migration. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
|
@conico974 the last 2 commits address your review feedback. |
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", + },