cloudflare / cloudflare/vinext

Converge stale-cache freshness policy: make the request-level revalidation mode authoritative for patched fetch

Open
#2,685 0 comments 0 reactions 0 assignees View on GitHub
Dominant language
TypeScript
Stars
8.8k
Forks
406
Avg merge
2d 6h
Merged PRs (30d)
120

Description

Follow-up from the review of #2682.

## Current state

A persistent cache read has one request-level policy question: may this render consume stale data, or must it produce fresh persistent output? vinext answers it from two sources of truth:

- `functionCacheRevalidationMode` (cache-request-state.ts) governs `"use cache"` and `unstable_cache` reads. Default is fail-closed (`"foreground"`); only the App Router RSC handler opts ordinary requests into `"background"`.
- `refreshStaleFetchesInForeground` (fetch-cache.ts) governs patched `fetch()`. Default is fail-open (serve stale, background refetch) on every path; only App Page revalidation enables it, and only when `VINEXT_PRERENDER === "1"` (build-time).

## The inconsistency

During runtime ISR regeneration (page and route-handler revalidation contexts), function caches block for fresh values (`"foreground"`), but patched fetch still serves stale responses and refetches in the background. A regenerated page or route artifact can therefore embed a stale fetch response even though every function-cache dependency was refreshed. Next.js treats a stale fetch as foreground work during static generation so the regenerated entry contains updated data.

## What convergence requires

Making one request-level policy (e.g. `allow-stale` vs `require-fresh`) authoritative for both cache implementations is not a mechanical merge, because the two defaults currently point in opposite directions. Per-path audit:

| Context | Function caches today | Patched fetch today | Converged target |
|---|---|---|---|
| App Router ordinary request (`app-rsc-handler.ts`) | background (SWR) | serve stale + bg refetch | `allow-stale` — no change |
| App page revalidation (`app-page-dispatch.ts`) | foreground | foreground only if `VINEXT_PRERENDER=1` | `require-fresh` — **fixes runtime ISR** |
| Route-handler revalidation (`app-route-handler-dispatch.ts`) | foreground | serve stale + bg refetch | `require-fresh` — **fixes runtime ISR** |
| Pages Router ordinary request (`pages-page-handler.ts` request ctx) | foreground (blocks on stale) | serve stale + bg refetch | `require-fresh` for function caches — already matches Next.js (see below). Fetch caching in Pages Router is a vinext extension with no upstream equivalent; fail-closed is consistent with the function caches |
| Pages Router ISR revalidation (`pages-page-handler.ts` reval ctx) | foreground | serve stale + bg refetch | `require-fresh` |
| Dev server contexts (`dev-server.ts`) | foreground | serve stale + bg refetch | probably `require-fresh` (freshness over latency in dev) |
| Fallback scope (no request context) | foreground | serve stale + bg refetch | `require-fresh` (fail-closed) |

Upstream settles the Pages Router row: in `next@16.2.7` (`dist/server/web/spec-extension/unstable-cache.js`), a stale entry inside a work store serves stale and revalidates in the background unless `workStore.isStaticGeneration`, where it awaits the refresh; without a work store (Pages Router, standalone) it falls through to regeneration and awaits the cache write in the foreground. Current canary (`packages/next/src/server/web/spec-extension/unstable-cache.ts`) has the same structure in both branches. Next.js keys the whole policy on one axis — does this render persist an artifact — which is what the request-level mode encodes. vinext's current Pages Router blocking behavior already matches, so the convergence is confined to the fetch rows plus test coverage. Fetch keeps its specialized refetch machinery (response streams, dedupe timeouts, cloning) either way; only the freshness decision moves to the shared policy.

## Acceptance

- `refreshStaleFetchesInForeground` and its setter are removed; `shouldRefreshStaleFetchInForeground()` reads the request-level policy.
- The field is renamed back to `cacheRevalidationMode` (or similar) once it is authoritative for all persistent cache paths.
- Regression coverage for: runtime ISR page regeneration blocks on stale fetches; ordinary App Router requests still serve stale fetches; whichever Pages Router semantics are chosen.

Contributor guide

Open the contributing guide

Research direction

Start with cache-request-state.ts and fetch-cache.ts to trace how the request-level revalidation mode and patched fetch freshness decisions are set and read. Audit the listed App Router, Pages Router, and dev-server contexts, then add regression coverage showing runtime ISR waits for fresh fetches while ordinary App Router requests still serve stale data; done means the separate refreshStaleFetchesInForeground state and setter are gone.

Written by the indexing model from the issue text.

Assessment

Tech stack
typescript
Domain
backend, performance, testing
Issue type
Bug
Difficulty
4/5
Estimated time
3-5 days
Activity status
Quiet
Clarity
Mostly clear
Newbie friendliness
45/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.