Skip to content

LTRAC-1499: Add Cloudflare KV adapter for the routing cache - #3170

Draft
jorgemoya wants to merge 1 commit into
jorgemoya/ltrac-1498-wrangler-routes-kv-bindingfrom
jorgemoya/ltrac-1499-cloudflare-kv-adapter
Draft

LTRAC-1499: Add Cloudflare KV adapter for the routing cache#3170
jorgemoya wants to merge 1 commit into
jorgemoya/ltrac-1498-wrangler-routes-kv-bindingfrom
jorgemoya/ltrac-1499-cloudflare-kv-adapter

Conversation

@jorgemoya

Copy link
Copy Markdown
Contributor

Linear: LTRAC-1499
Parent: LTRAC-1019

Stacked PR. Base is jorgemoya/ltrac-1498-wrangler-routes-kv-binding (#3169), so review only the top commit here. Merge #3169 first.

What/Why?

The runtime half of the routing cache work: createKVAdapter gains a Cloudflare branch so proxies/with-routes uses the per-project KV namespace on Native Hosting instead of degrading to MemoryKvAdapter. Vercel stays first in the chain; Upstash and Memory remain the fallbacks.

Three decisions here are non-obvious and are the ones worth reviewing.

1. It reads the context global directly instead of importing @opennextjs/cloudflare

core deliberately does not depend on that package — setupCommerceHosting() injects it only when a project opts into Commerce hosting. Every way of importing it from core is worse, and this was measured rather than assumed:

  • a static import breaks next build outright when the package is absent;
  • a dynamic import() still fails next build's TypeScript step ("Cannot find module … or its corresponding type declarations") on every non-Commerce project;
  • silencing that with an ambient declare module shadows the package's real types when it is installed, which breaks the .bigcommerce/open-next.config.ts the CLI writes (it imports defineCloudflareConfig from the same specifier, and core's tsconfig compiles that directory).

So the adapter reads globalThis[Symbol.for('__cloudflare-context__')], which is exactly what getCloudflareContext() does in sync mode. On any non-Cloudflare runtime the symbol is simply undefined — the "import failed" failure mode stops existing rather than being caught.

That global is an AsyncLocalStorage-backed getter, and the OpenNext worker entrypoint wraps every request handler in cloudflareContextALS.run({ env, ctx, cf }, handler) — which is why a synchronous read works inside middleware.

The tradeoff: that symbol is an internal detail, not exported API, and if it changed the adapter would silently return null and every store would quietly downgrade to an in-process cache with no error. That's why packages/catalyst/src/cli/lib/cloudflare-context-symbol.spec.ts asserts the real getCloudflareContext() still honours this exact key — a version bump that breaks the assumption fails CI instead of degrading production. OPENNEXT_CLOUDFLARE_VERSION is exported so that test can pin what it was verified against.

2. Selection duck-types the binding rather than checking presence

A merchant can define an env var literally named CATALYST_ROUTES_KV, which arrives as a plain string. A truthiness check would accept it and then throw on the first .get(). Narrowing on get/put being functions makes that case fall through to Upstash/Memory cleanly. (Ignition also strips the colliding var server-side — this is defence in depth.)

3. Writes carry an expirationTtl

Workers KV entries are permanent unless written with an expiration, and nothing else deletes routing cache keys. Since kvKey() includes the query string, a crawler walking ?utm_* permutations would otherwise grow a namespace — and its billable write volume — without bound, from unauthenticated requests.

The TTL is 7 days, deliberately much longer than the 30-minute freshness window with-routes enforces via the expiryTime inside each value. The two timers do different jobs: expiryTime decides when to revalidate, the TTL decides when to garbage-collect. Setting them equal would delete each entry exactly as it went stale, turning every stale-while-revalidate hit — which serves instantly and refreshes in the background — into a blocking GraphQL round trip. A test pins the TTL as strictly greater than that window so this can't be "tidied up" into a regression later.

Note the same unbounded-growth gap exists today for Upstash users, which passes opts through but is never given any. Fixing that means touching shared middleware and changing behaviour for existing deployments, so it's intentionally not bundled here.

Reviewer note: new test infrastructure in core

core had no unit-test setup at all — no test script, no vitest/jest, and core/tests/** is entirely Playwright. Covering the selection matrix required introducing vitest: core/vitest.config.ts, a test script, and the vitest devDependency (plus the lockfile line).

Flagging it explicitly because core is the scaffold customers receive, so that config ships into every generated project. Those files are grouped so they're easy to strip if that's unwanted. The vitest include is scoped to lib/**/*.spec.ts so it can't collect Playwright's specs, which share the .spec.ts suffix, and no coverage thresholds were added.

Testing

pnpm vitest run            # in core
✓ lib/kv/adapters/cloudflare-kv.spec.ts (27 tests)
✓ lib/kv/index.spec.ts (9 tests)
Tests  36 passed (36)

pnpm vitest run src/cli/lib/cloudflare-context-symbol.spec.ts   # in packages/catalyst
✓ 3 tests

Selection is covered for: binding present and well-shaped, binding absent, the @opennextjs/cloudflare import itself failing, and the binding present but a string — the last three all asserting it falls through without throwing, since a caching layer must degrade rather than break a request. Also covered: get/put round-trip, per-key error isolation in mget, a failing put being swallowed, and the TTL assertions above.

eslint clean on lib/kv; tsc --noEmit reports no errors in lib/kv (the repo has pre-existing errors elsewhere from missing GraphQL codegen, identical with and without this change).

Migration

None. Additive — no files moved, no breaking changes. A patch changeset is included. Behaviour is unchanged anywhere the binding isn't present, which is every non-Native-Hosting deployment.

with-routes caches redirects and storefront status through lib/kv, which had
no Cloudflare option: on Native Hosting it fell through to MemoryKvAdapter,
an in-process LRU that isn't shared across edge invocations. Add a
CloudflareKvAdapter over the per-project CATALYST_ROUTES_KV namespace and
select it ahead of Upstash, keeping Vercel Runtime Cache first in the chain.

Workers KV has no multi-get, so mget fans out to parallel get(key, 'json')
calls, each guarded so one unreachable key can't blank the batch. set writes
JSON.stringify(value) and swallows failures -- a cache write must not break
the request.

The binding is duck-typed on get/put being callable rather than merely
present. A merchant can define an env var literally named CATALYST_ROUTES_KV,
which arrives as a plain string; a truthiness check would accept it and then
throw on first use. Narrowing lets it fall through to Upstash/Memory instead.

Note this does NOT import @opennextjs/cloudflare, and deliberately so. The
package is absent from core's package.json (the CLI injects it only for
Commerce hosting), and importing it from here fails either way: a dynamic
import breaks `next build`'s TypeScript step on every non-Commerce project
("Cannot find module ... or its corresponding type declarations"), and
silencing that with an ambient `declare module` shadows the package's real
types when it IS installed -- which breaks the .bigcommerce/open-next.config.ts
the CLI writes, since it imports defineCloudflareConfig from the same
specifier and core's tsconfig compiles it. Both were verified empirically.

getCloudflareContext() in sync mode is just a read of
globalThis[Symbol.for('__cloudflare-context__')], set by the OpenNext worker
entrypoint in production and by initOpenNextCloudflareForDev under next dev.
Reading that symbol directly yields the identical value with none of the
above failure modes -- off Cloudflare it is simply undefined.

Verified end to end, not just by inspection: an OpenNext worker built with
the same pipeline and run under `wrangler dev` with the KV binding wired
reports, from inside experimental-edge middleware with no import and no
await, that the symbol is defined, that env.CATALYST_ROUTES_KV exposes
get/put, and that a put/get round trip returns the written value. The value
was then read back by a separate `wrangler kv key get` process, confirming a
genuinely shared store rather than per-invocation memory.

Because that symbol is an internal contract rather than exported API, its
failure mode would otherwise be silent -- a version bump would downgrade
every native-hosted store to an in-process cache with no signal. The key is
now a named exported constant pointing at OPENNEXT_CLOUDFLARE_VERSION, and a
contract test in packages/catalyst (the package that actually has
@opennextjs/cloudflare installed) asserts the real getCloudflareContext still
reads it, and reads it synchronously, so a breaking bump fails CI.

core had no unit test infrastructure (tests/ is Playwright), so this also
adds vitest scoped to lib/**/*.spec.ts, keeping it clear of the Playwright
specs that share the .spec.ts suffix. createKVAdapter is exported for tests
because adapter selection depends on ambient state the memoized kv singleton
hides.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
@changeset-bot

changeset-bot Bot commented Aug 6, 2026

Copy link
Copy Markdown

🦋 Changeset detected

Latest commit: 765d6ac

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 1 package
Name Type
@bigcommerce/catalyst-core Patch

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

@vercel

vercel Bot commented Aug 6, 2026

Copy link
Copy Markdown

The latest updates on your projects. Learn more about Vercel for GitHub.

Project Deployment Actions Updated (UTC)
catalyst Building Building Preview Aug 6, 2026 10:06pm

Request Review

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.

1 participant