mirror of
https://github.com/cloudflare/vinext.git
synced 2026-09-14 19:04:59 +08:00
codex/cacheability-platform-io
12 Commits
| Author | SHA1 | Message | Date | |
|---|---|---|---|---|
 Nathan Nguyen
|
73f56ef3f8 |
fix(app-router): honor cacheLife stale on the client router (#2708)
* fix(app-router): honor cacheLife stale on the client router
`cacheLife` profiles carry three independent numbers, and `stale` is the
client-router dimension: how long the browser may reuse cached route output
without asking the server. vinext aggregated it correctly (`resolveCacheLife`
min-reduces all three; the request scope accumulates min-wins) and then
projected it away at every consumer, so the only staleness a browser ever saw
was `dynamicStaleTimeSeconds` from `experimental.staleTimes` — a build-time
constant unrelated to the cached subtrees that produced the render. A subtree
declaring `cacheLife("seconds")` (stale: 30) was held for the full 5-minute
visited-response TTL.
Carry the resolved `stale` to the client on `x-nextjs-stale-time` (matching
Next.js's `NEXT_ROUTER_STALE_TIME_HEADER`) and combine it with the config
value by taking the minimum, so neither min-wins lattice overrides the other.
The 30s prefetch floor applies to the new value too.
Two normalization rules, both deliberate:
- An absent `stale` is never synthesized from `revalidate`/`expire`. The
`default` profile has no `stale` and an `expire` of ~136 years, so deriving
one would license session-long reuse without a refresh.
- The three numbers are not assumed to be ordered — `seconds` is
`{ stale: 30, revalidate: 1, expire: 60 }` — so `revalidate` never
constrains `stale`. Only the hard `expire` ceiling does.
* fix(app-router): source the client stale time from completed renders
The previous commit derived `x-nextjs-stale-time` from `peekRequestCacheLife()`
at response-construction time. That read happens after `probeAppPageBeforeRender`
but before the RSC stream is consumed, and the probe only awaits the page
component's own async result — it does not render the returned tree. Any
`use cache` scope in a child Server Component registers later, during stream
consumption, so the header described the probe rather than the output.
Because `cacheLife` aggregation is minimum-wins, missing one late scope is
enough to make the value wrong, and "it can only shorten" did not hold: with no
`dynamicStaleTimeSeconds` present (the usual case for `use cache` pages, which
are not dynamic renders), a peeked `stale: 300` widened the prefetch window from
`PREFETCH_CACHE_TTL` (30s) to 300s — 10x longer than today, in the direction the
change set out to fix.
Carry the value on the cache entry instead. The ISR write path reads the
request-scoped accumulation via the consuming `getRequestCacheLife()` inside the
cache-write closure, which runs after the captured stream has drained, so it
observes the completed render's minimum. `CacheControlMetadata` gains `stale`,
`isrSet` persists it, and `buildAppPageCachedResponse` re-advertises it on hits.
Prerender seeds carry it through `VINEXT_PRERENDER_CACHE_LIFE_HEADER` and the
prerender manifest so a seeded entry makes the same claim a runtime render would.
Two links that would otherwise silently drop the claim are closed with it: the
`use cache` entry write now persists `stale`, and `recordRequestScopedCacheControl`
re-registers it on a data-cache hit — without both, a page's advertised freshness
would depend on data-cache temperature rather than on what it declared.
Streaming responses now advertise nothing and leave the client on its configured
`experimental.staleTimes`, which is the honest answer for a value that is not yet
known when headers are committed. Covering fresh streaming renders needs a
render-completion signal the streaming RSC path does not have today; Next.js
solves it by streaming an `AsyncIterable<number>` in the RSC payload
(`app-render.tsx` `baseResponse.s`, closed after the render settles), which is
the natural follow-up.
The client-side combination logic is unchanged: both stale signals are still
min-reduced, absent `stale` is never synthesized from `revalidate`/`expire`,
`revalidate` never clamps `stale`, and `expire` still caps it.
* fix(app-router): deliver the resolved client stale time on fresh renders
The previous round persisted the completed render's cacheLife stale onto
the ISR entry and replayed it on cache hits, which left the value missing
or drifting wherever the entry's lifetime diverged from the render's:
- Fresh streaming renders advertised nothing, so the first visitor of any
route (and every dev render, since dev writes no ISR entry) stayed on
the configured staleTimes despite a declared cacheLife. The done-script
emitted by the RSC embed transform's finalize() runs only after the
full RSC stream has drained, so it can carry the completed render's
minimum where streaming headers cannot. Emit the peeked request-scoped
cacheLife there and seed the hydration visited-response entry from it.
This also makes the client's min-combination of the config and
cacheLife signals reachable: a dynamic render with use cache subtrees
now legitimately carries both.
- The expire clamp bounded the value but not the elapsed window: an
entry of { stale: 30, expire: 60 } hit at age 59s replayed a 30s reuse
window reaching 29s past expire. Age the serve-time clamp with the
entry's lastModified so only the remaining window is advertised.
- The two client caches applied different floors: prefetch entries
floored a cacheLife stale at 30s while cold navigations honored it
verbatim, so the same declaration produced two behaviors keyed on
whether a prefetch fired first. Floor the cacheLife signal once in the
shared resolver, mirroring Next.js getStaleTimeMs, before the min so
it can never raise the config-derived bound.
Mechanically, the app-page cache setter's six-position signature
(declared identically in four modules) collapses into one exported
AppPageCacheSetter taking an AppPageCacheWritePolicy object, with
isrSetAppPage adapting to the shared positional isrSet, which stays
unchanged for the Pages Router and route handlers.
* docs(app-router): pin the cold RSC stale contract and expire precedence
The cold RSC fetch gap is a design decision, not a pending follow-up:
Next.js's AsyncIterable stale transport only works under staged rendering
(cacheComponents), where cache scopes settle before the render task queue
drains — in vinext's lazy streaming model the iterable could never close.
Next.js's plain-mode mechanism is a blocking cold render, which #961
deliberately rejected to keep ISR page streams unblocked. Assert the
resulting no-header contract on the streaming-response test.
Also reword the write-policy expire comment: a cacheLife-declared expire
replaces the config expireTime fallback (Next.js precedence), it is not
min-merged with a route-level ceiling — no such ceiling exists outside
cacheLife.
* fix(app-router): carry stale through regen and bound cold responses
Three fixes from review round 3:
Background regeneration dropped the regenerating render's cacheLife stale:
renderAppPageCacheArtifacts returned only { revalidate, expire }, so
resolveRegeneratedAppPageCachePolicy could never receive the stale it was
built to preserve and the first regen silently widened client reuse back
to the configured fallback. The producer now carries it, with a
real-producer regression test the mocked-cacheControl tests could not
provide.
The age-aware expire clamp is removed. Composed with the client's 30s
floor it delivered neither contract (a clamped 1 re-floored to 30), and
cached HTML replayed the unclamped done-script value regardless. Next.js
stores the stale header at generation and replays it verbatim on every
hit; vinext now does the same, keeping expire a serve-side ceiling.
Cold cacheable RSC responses stream before their cacheLife resolves
(#961), which left them on the 300s client fallback — reproducing the
headline bug for the first request of every entry epoch. They now carry
X-Vinext-Stale-Time-Pending, and both client caches bound such responses
at the 30s floor: the unresolved claim, once floored, could never
license less.
* fix(app-router): bound pending-stale responses by the dynamic stale time
The pending marker meant 'capture was attempted', but the client read it
as 'a cacheLife claim exists'. Capture eligibility is decided before the
lazy stream runs, so a late request-API read can make the completed
render dynamic — the finalizer skips the ISR write and no claim ever
exists, yet the marker granted 30 seconds of reuse even under
staleTimes.dynamic: 0.
Pending responses now carry the configured dynamic stale time (including
0) and the client takes the minimum, so an unresolved response never
receives a wider window than the dynamic bound.
* fix(app-router): keep the pending cap independent of staleTimes.dynamic
Pairing the pending marker with the configured dynamic stale time broke
the segment-cache-client-params compat test: dynamic-param routes
prefetch with no minimum TTL, so the paired 0 default made every cold
prefetch entry expire instantly and navigations refetched routes that
Next.js serves entirely from a static prefetch.
A cold stream cannot distinguish a render that will resolve static from
one that turns dynamic mid-stream, and bounding both by staleTimes.dynamic
sacrifices the guaranteed-correct case for the ambiguous one. Pending
responses go back to the 30s floor cap; the late-dynamic exposure
(one epoch-cold response, at most 30s) is documented as the price of
non-blocking cold renders (#961).
* fix: carry the client stale claim through KV, the prerender index, and nested cache hits
- writePrerenderIndex now copies `stale` into vinext-prerender.json so
seedMemoryCacheFromPrerender actually receives it
- KVCacheHandler.set() and buildPrerenderKVPairs persist cacheControl.stale
so warm hits on the Cloudflare KV backend replay the producing render's
claim; validateCacheEntry accepts the field
- a nested use cache HIT pushes its stored lifetime into the enclosing
cache context's lifeConfigs, mirroring the MISS path, so the outer entry
keeps the child's stale claim once the outer goes warm
* perf: trim shipped comment bytes in the stale-time plumbing
The added modules ride in every consumer build environment; the review-grade
rationale lives in the PR body, the code keeps one-line constraints.
* refactor(app-router): model the cached client stale claim as one state
CachedRscResponse carried staleTimePending and staleTimeSeconds as two
independent optionals, so the cached form admitted a state the wire never
produces (pending and resolved at once) and every consumer had to encode
the precedence rule. Replace both with a discriminated serverStaleTime
(pending | resolved), collapsed once at the header-parse boundary.
* refactor(isr): give isrSet a cache-metadata write policy
The generic setter had grown to six positional arguments, the last an
App-Router-only `stale` value, with isrSetAppPage as a policy-object
wrapper that unpacked straight back into it. Take { cacheControl, tags }
instead: routers construct the metadata they actually claim, the wrapper
and its type disappear, and the shared isrCacheControl builder replaces
the cache-control literal duplicated across writers.
* fix(app-router): keep the dynamic bound on captured dynamic renders
The done-script metadata reused the RSC header's rule of dropping the
config-derived dynamic stale time while the speculative ISR capture was
armed. That rule only holds on the header path, which substitutes the
pending marker and its 30s floor; the done script emits the resolved
cacheLife instead, so a production render that turned dynamic shipped the
cacheLife claim as its only bound and let the hydration-seeded entry reuse
dynamic output for its full duration. Dev never took the capture path, so
the two diverged.
Also migrates the pages-basic seed fixture, the last positional isrSet
caller, which typecheck does not cover.
* ci: re-run after unrelated dev-overlay canary flake
* fix(app-router): preserve completed client stale metadata
* fix(app-router): strip completion metadata during HMR
* fix(app-router): stream completion metadata safely
* fix(app-router): validate completed stale metadata
* docs(cache): clarify zero stale client claim
---------
Co-authored-by: James <james@eli.cx>
|
||
|
James Anderson
|
71dbb45ca8 |
fix(pages): isolate on-demand revalidation requests (#2495)
* fix(pages): pin revalidate loopback origin * test(pages): cover production revalidation origin * fix(pages): preserve internal revalidation boundaries * fix(pages): preserve on-demand revalidation boundaries * fix(pages): dispatch Worker revalidation internally * fix(pages): authenticate revalidation request context * fix(pages): isolate revalidation transport headers * test(pages): account for generated route table size * fix(pages): align on-demand revalidation semantics * fix(pages): align revalidation response cache parity * fix(pages): preserve regenerated ISR representations * fix(pages): align ISR cache representations * fix(pages): preserve canonical ISR representations * fix(pages): align cached response parity * fix(pages): preserve custom App render props * fix(pages): preserve optional App page props * fix(pages): preserve optional App props on the client * fix(pages): normalize client App page props * fix(pages): preserve App data merge semantics * fix(pages): keep redirect status helper internal * fix(pages): match Next.js terminal ISR behavior * fix(pages): preserve custom app error envelopes * fix(pages): match Next.js dev revalidation semantics * test(pages): align dev revalidation parity coverage * chore(pages): remove stale cache helper exports * fix(cache): preserve explicit no-store context |
||
|
James Anderson
|
69e0051cf7 | fix(pages): expose parsed cookies on request objects (#2401) | ||
|
Nathan Nguyen
|
3253aafd77 |
fix(pages-router): pass rewrite URL to edge API requests (#1998)
* fix(pages-router): pass rewrite URL to edge API requests * fix(pages-router): preserve edge API request URL parity * fix(pages-router): configure edge API NextRequest * fix(pages-router): preserve edge API locale config * fix(pages-router): preserve edge request metadata * fix(pages-router): align edge API URL formatting * fix(pages-router): match API locale casing * fix(pages-router): match domain locales by locale * docs(pages-router): clarify edge API basePath parity * test(deploy): expect metadata-preserving URL clone * docs(pages-router): note basePath rewrite parity --------- Co-authored-by: James <james@eli.cx> |
||
|
James Anderson
|
9596b72495 |
fix(pages-router): pass revalidateReason "on-demand" to gsp/gssp (#1462) (#1856)
* fix(pages-router): pass revalidateReason "on-demand" to getStaticProps (#1462) Implements res.revalidate() for Pages Router API routes and wires the on-demand revalidation trigger through to getStaticProps/getServerSideProps context as revalidateReason: "on-demand". res.revalidate(urlPath) issues an internal HEAD request to the target path carrying the x-prerender-revalidate header (mirroring Next.js's api-resolver). The Pages render path (dev-server and prod pages-page-data) detects that header, bypasses the fresh/stale cache-hit short-circuits, and regenerates the entry synchronously with reason "on-demand". The build and stale reasons already worked; this finishes the on-demand case. Closes #1462 * fix(pages-router): authenticate on-demand revalidation with a secret (#1462) The on-demand `revalidateReason` detection treated `x-prerender-revalidate` as an unauthenticated presence check and sent the literal value "1". Any external client could send `x-prerender-revalidate: 1` to force synchronous regeneration of any ISR page, bypassing the fresh/stale cache short-circuits — a cache-stampede/DoS vector. Mirror Next.js's `checkIsOnDemandRevalidate`: a request is only treated as on-demand revalidation when the `x-prerender-revalidate` header value EQUALS a secret (the vinext analog of `previewModeId`). Introduce a per-process revalidate secret (`getRevalidateSecret`, 256-bit random hex via Web Crypto) that `res.revalidate()` attaches to its internal loopback request, and a constant-time `isOnDemandRevalidateRequest` equality check on the receiving dev-server and Pages page handler. Header presence alone is never honored. Also fix the test's HTML-reason extraction to avoid CodeQL's incomplete-sanitization / bad-HTML-regex findings, and add a negative test asserting forged `x-prerender-revalidate` values ("1", "", a guess) are rejected while the real secret is accepted. * refactor(pages-router): share res.revalidate() impl; support unstable_onlyGenerated (#1462) Address non-blocking bonk review findings: - Extract the two near-identical `res.revalidate()` implementations (`api-handler.ts`, `pages-node-compat.ts`) into a shared `performOnDemandRevalidate` helper so the secret wiring and success detection cannot drift between dev and prod. - Align success detection with Next.js's api-resolver: accept `x-nextjs-cache: REVALIDATED`, status 200, or a 404 when the caller passed `unstable_onlyGenerated`. Add `unstable_onlyGenerated` support (`x-prerender-revalidate-if-generated` header). * docs(pages-router): note x-nextjs-cache REVALIDATED branch is parity-only (#1462) * fix(pages-router): make on-demand revalidate secret build-time (shared across isolates) The on-demand ISR revalidation secret was a per-process random value. On Cloudflare Workers, res.revalidate()'s loopback fetch() can land on a different isolate than the sender, so a per-process secret mismatches across isolates and false-rejects legitimate revalidations. Mirror Next.js's previewModeId: generate one 256-bit secret at build time (in the vinext build CLI, once per build, shared across plugin instances like __VINEXT_SHARED_BUILD_ID) and bake it SERVER-ONLY into every server bundle via a Vite define (__VINEXT_REVALIDATE_SECRET). It is byte-for-byte identical in every isolate and never reaches the client bundle. getRevalidateSecret() reads the baked constant at runtime and falls back to a process-shared random secret in dev (single-process, no regression). Also persist revalidateSecret in vinext-server.json for diagnostics/tooling parity with prerenderSecret. Constant-time safeEqual validation and reject-arrays/empty/absent behavior are unchanged. Refs #1462 * test(pages-router): cover server-only revalidate secret; drop dead manifest reader Address ask-bonk review of the build-time revalidate secret: - Add tests pinning the security-critical invariant that the __VINEXT_REVALIDATE_SECRET define is baked into server environments (rsc/ssr) but NEVER into the client bundle (the client env returns null outright). A leak would ship the secret to every browser and re-open the cache-stampede vector the equality check prevents. Also harden the existing "no-ops when defineServer unset" test to explicitly clear the env var instead of relying on it happening to be unset. - Drop the unused readRevalidateSecret export and the orphaned revalidateSecret manifest field — the runtime authenticates against the baked define (both Workers isolates and the Node prod-server run the bundle), so the on-disk copy had no consumer (knip-flagged dead code). - Hoist the client early-return in vinext:compiler-define-server so the server-only guarantee is structural (removes the redundant name check), and reword the dev-fallback comments ("absent unless set during vinext build"). Refs #1462 |
||
|
James Anderson
|
15fbe66c47 |
test(pages-router): regression coverage for edge runtime & OG image API routes (#1338) (#1648)
Adds Pages Router fixtures and dev + production integration tests for the
two scenarios reported as failing in the Next.js Deploy Suite:
- `export const config = { runtime: 'edge' }` API route returns a Web
Response (issue #1338 case 1 — edge API routes were observed as 500).
- `next/og` `ImageResponse` with `runtime: 'edge'` inside a Pages Router
API route returns `image/png` (issue #1338 case 2 — OG routes were
observed as 404).
Ports route handlers from `next.js/test/e2e/edge-pages-support/app/pages/api/hello.js`
and `next.js/test/e2e/og-api/app/pages/api/og.js`. The existing
implementation already wires both code paths correctly via
`handlePagesApiRoute` and the `vinext:og-inline-fetch-assets` plugin; this
PR locks in coverage so a Pages Router-only build cannot silently regress
either path.
Refs #1338
|
||
|
Jared Stowell
|
f360380112 |
fix: Pages API body parser for invalid JSON and repeated form keys (#446)
* Fix Pages API parsing parity * Fix Pages API body parsing parity * Fix empty form body parity * Regenerate entry-templates snapshots after merge * fix: propagate statusText through prod server sendCompressed non-compressed path The sendCompressed function had a statusText parameter but only used it in the compressed response path. The non-compressed else branch called res.writeHead directly without forwarding statusText, so short error responses (like 'Invalid JSON' at 12 bytes, well below COMPRESS_THRESHOLD) lost their custom reason phrase and fell back to the default 'Bad Request'. Fix: replace the direct res.writeHead call in the else branch with the writeHead closure that already handles the statusText conditional. Also set statusText on the ApiBodyParseError response in the pages server entry template so the value is present for prod-server to forward. --------- Co-authored-by: James <james@eli.cx> |
||
|
Jared Stowell
|
3b982066d0 |
fix: align Pages API body parsing and res.send(Buffer) (#428)
* Fix Buffer handling in Pages API res * fix: skip reporting handled Pages API parse errors * address review: blank line nit, content-length parity for Buffer, fix duplicate Content-Length in sendCompressed * fix: set content-length for Buffer in production res.send() to match dev parity --------- Co-authored-by: James <james@eli.cx> |
||
|
James Anderson
|
c4c67fa5f4 |
fix: pages router instrumentation error reporting for api routes (#334)
* test: pages router instrumentation * fix error handling * add pages route to app dir * fix api route type |
||
|
Steve Faulkner
|
9ab08ff49f |
Default API route Content-Type to application/octet-stream (#285)
* Default API route Content-Type to application/octet-stream When an API route handler calls res.end() without setting a Content-Type header, the production server was defaulting to text/html. This could cause browsers to render response bodies as HTML when the developer intended to return plain data. Change the fallback for API routes to application/octet-stream, which is the safe default for arbitrary binary/text content. The SSR page rendering path still correctly defaults to text/html. * Improve test comment for Content-Type assertion Explain why we use a negative assertion (not text/html) rather than a positive one: when the handler passes a string to res.end(), the Response constructor auto-sets text/plain, so the octet-stream fallback only triggers for non-string bodies. |
||
|
Tiago Linares
|
0d5b42c79e |
fix: preserve binary API response bytes in prod-server (#30)
Updated the production server to handle binary responses correctly by using Buffer.from with arrayBuffer instead of text(). Added a test to ensure binary data integrity and created a new fixture for testing binary API responses. |
||
|
Steve Faulkner
|
12fea722b6 | Initial public release of vinext |