Repository navigation
fix(cloudflare): share one Workers Cache entry across hosts - #3924
james-elicx wants to merge 5 commits into
Conversation
@vinext/cloudflare
create-vinext-app
@vinext/types
vinext
@cloudflare/workers-response-store
commit: |
Performance benchmarksCompared 0 improved · 0 regressed · 7 within ±1.5%
View detailed results and traces 🟢 improvement · 🔴 regression · ⚫ change below 1.5% · paired base/head |
|
LGTM! |
a482fef to
f223913
Compare
|
/bigbonk please review this PR at head |
|
LGTM! |
|
/bigbonk please review this PR at head |
|
LGTM! |
|
d6aed61 to
6877554
Compare
|
/bigbonk review for issues |
|
💬 Approved with comments — The host-free Workers Cache identity, the restore header and its fail-closed checks, and the probe and dynamic-marking changes are correct. The only finding is an optional suggestion to document the new identity contract in AGENTS.md. Findings: 1 suggestion The decision is advisory: this comment never approves, blocks or requests changes on the pull request. 🔧 Fix with your agentPaste the following into your coding agent to investigate and fix these issues. 📖 AGENTS.md: 💬 1 suggestionThe new response-stage identity contract isn't mentioned in the agent guidance. Recommendation: 💬 Optional Nits
View full review
✅ All other checks passed🔒 Security: ✅ LGTMNo issues found.
📋 Code Quality: ✅ LGTMNo issues found. This review read the code; it did not run any tests. Worker identity and restore (
App identity (
Probes
E2E
⚡ Performance: ✅ LGTMNo performance concerns. Each request now builds one more JSON header and checks it once. Sharing one cache entry across hosts should cut the number of renders and stored copies. 📝 Documentation: ✅ LGTMThe Reviewed commit: |
Workers Cache keys an entry by its path and query, entrypoint, ctx.props and Worker version, not by host. vinext put the request's full URL into the key it adds and the props it passes, so every host filled its own copy, where Next.js keys its response cache by pathname. The Workers Cache adapter now asks core for a host-free identity, which keeps every dispatch's query. The gateway keys the cached entrypoint by that identity and sends the host-bearing fields the render needs in an internal header, which Workers Cache doesn't key.
…amic in probes Workers Cache warms what its cacheability probe finds cacheable. A probe dispatch bypasses the shared transport, so it never marked a Pages page as shared across hosts, and the render-time getInitialProps veto, which only applied to admission, never ran in it. A page whose getInitialProps an imported wrapper attaches was then classified cacheable, and warming waited for an entry its shared dispatch refuses to store. A probe now marks the page shared across hosts, as its shared dispatch is, without a cache identity, and the veto marks the render dynamic, as Next.js counts a getInitialProps page, in probes and admission alike.
6877554 to
19c6a47
Compare

Summary
Next.js keys its response cache by pathname, so every host an app serves shares one entry. Cloudflare Workers Cache keys an entry by its path and query, entrypoint,
ctx.propsand Worker version, not by host. But vinext put the request's full URL, host included, into its__vinext_cache_keydigest and into thectx.propsit passed the cached entrypoint. So every host filled and stored its own copy of each page.The Workers Cache adapter now declares
responseStageCacheIdentity: "host-free". Core gives it the same host-free identity the Response Store gets (the canonical cache identity origin, or a Pages page's configured i18n domain), but keeps every dispatch's query. Workers Cache keys the dispatched query itself, and the render needs it, so the App page query-free identity stays Response Store only. The manifest-based query-free dispatch for static-candidate App pages is unchanged.On a shared dispatch with an identity:
ctx.propsand the digest it keys by.requestOrigin,requestUrl,requestHost) and the real request URL go in an internal header that Workers Cache doesn't key. The gateway strips that header from public requests.no-store.Cloudflare also keys a few headers that name another host, such as
X-Forwarded-Host. A proxy in front of the Worker that sets one still gets an entry per forwarded host; the docs say so.Workers Cache warms what its cacheability probe finds cacheable. A probe dispatch bypasses the shared transport, so it never marked a Pages page as shared across hosts, and the render-time check that refuses a shared entry for a page whose
getInitialPropsan imported wrapper attaches only ran during admission. Warming then waited forever for an entry that page's shared dispatch never stores. A probe now marks the page shared across hosts, as its shared dispatch is, and that check marks the render dynamic (Next.js doesn't treat agetInitialPropspage as static), in probes and admission alike.The same admission rules as the Response Store apply. A render that reads the host is dynamic and isn't stored. Per-request Pages responses (
getServerSideProps, API routes, pages whosegetInitialPropsruns) keep the host in their keys under an explicit CDN policy, as a CDN in front of Next.js would.docs/caching.mdxnow describes Workers Cache together with the Response Store.Tests
tests/cloudflare-cdn-worker.test.ts):ctx.props, digest and path for two hosts, and each render sees its own host and URL.tests/app-rsc-handler.test.ts: a host-free App page identity keeps the query and selector headers.tests/pages-request-worker-stage.test.ts: the static Pages identity test now runs for both identity kinds.sharedAcrossHostswithout a cache identity. The hybrid bridge marks such agetInitialPropsrender dynamic in admit and probe modes.tests/e2e/cloudflare-workers/cache-prewarm.spec.ts, workers-cache job):pr-<n>-workers-cache) and its Version URL are two hosts of one uploaded version. A new demo route,/api/worker-version, reports the version id.getStaticPropspage and an ISR route handler filled on the alias host are a HIT, with the same render, on the Version URL host.getInitialPropspage that echoes the host always shows each host its own.wrangler devdoesn't emulate Workers Cache, so the deployed job is the e2e proof.