[PR #359] [CLOSED] fix(app-router): implement ISR caching gated behind production NODE_ENV #509

New issue

Closed

opened 2026-05-06 13:08:27 +02:00 by BreizhHardware · 0 comments

BreizhHardware commented

2026-05-06 13:08:27 +02:00

Owner

📋 Pull Request Information

Original PR: https://github.com/cloudflare/vinext/pull/359
Author: @james-elicx
Created: 3/8/2026
Status: ❌ Closed

Base: main ← Head: fix/app-router-isr-production-guard

📝 Commits (10+)

8b076c9 fix(app-router): implement ISR caching gated behind production NODE_ENV
919591e Merge remote-tracking branch 'origin/main' into fix/app-router-isr-production-guard
38d7495 test: update entry template snapshots for App Router ISR changes
aee9cc0 fix(isr): complete RSC caching, fix nav context, add X-Vinext-Cache: MISS
5be8559 test(e2e): add app-router-prod Playwright project with production ISR lifecycle tests
40fee93 ci: add app-router-prod to E2E matrix
40054a7 fix(isr): thread fetch tags into isrSet so revalidateTag invalidates page cache
2df32da perf(isr): move cache check before render, stream MISS response, fix CodeQL ReDoS
9138a1c fix(isr): add path tags to isrSet, cache RSC MISS, fix STALE regen font timing
d7714a2 perf(dev): cache loadModule() result to eliminate per-request Vite IPC overhead

📊 Changes

10 files changed (+3303 additions, -582 deletions)

View changed files

📝 .github/workflows/ci.yml (+1 -0)
📝 packages/vinext/src/cloudflare/kv-cache-handler.ts (+27 -3)
📝 packages/vinext/src/index.ts (+1 -0)
📝 packages/vinext/src/server/app-dev-server.ts (+438 -82)
📝 packages/vinext/src/server/app-router-entry.ts (+2 -2)
📝 packages/vinext/src/server/isr-cache.ts (+16 -0)
📝 packages/vinext/src/shims/cache.ts (+17 -8)
📝 playwright.config.ts (+12 -0)
📝 tests/__snapshots__/entry-templates.test.ts.snap (+2548 -487)
➕ tests/e2e/app-router-prod/isr.spec.ts (+241 -0)

📄 Description

Summary

App Router ISR now works in production: pages with export const revalidate = N read from/write to the pluggable cache handler (set via setCacheHandler), serving HIT/STALE/MISS responses — matching Pages Router behavior
RSC flight data cached alongside HTML: client-side navigations (RSC requests) are served from cache without a re-render, matching Next.js's rscData cache entry field
X-Vinext-Cache on every ISR response: HIT, STALE, and MISS all emit the header for observability parity
Cache-Control headers still emitted in all environments: dev and test still get s-maxage=N, stale-while-revalidate on ISR pages; only the cache handler interaction is gated behind NODE_ENV === "production"
activeHandler moved to globalThis: shims/cache.ts now stores the active cache handler on globalThis[Symbol.for("vinext.activeHandler")] so all module instances in the same process (host + Vite RSC environment) share the same reference

How it works

The generated RSC entry gains full ISR logic wrapped in if (process.env.NODE_ENV === "production"). Vite's define config statically replaces process.env.NODE_ENV at transform time ("development" or "test" in non-production), so the cache handler interaction is dead-code-eliminated in dev/test — every request re-renders fresh.

HTML requests:

HIT: returns cached HTML with X-Vinext-Cache: HIT
STALE: returns cached HTML with X-Vinext-Cache: STALE + triggers background regeneration (uses _frozenNavContext captured before request context is cleared, so usePathname() etc. work correctly in background renders)
MISS: tees rscStream before handleSsr, then collects HTML and RSC flight bytes concurrently (concurrent collection avoids a backpressure deadlock — SSR produces HTML by consuming the RSC stream, so draining them in series could stall), stores both in cache, returns with X-Vinext-Cache: MISS
Dev/test: falls through directly to htmlStream response with Cache-Control header still set

RSC requests (client-side navigation):

HIT/STALE: serves cached RSC flight bytes directly as a stream — no re-render; STALE triggers background regeneration of both HTML and RSC data
MISS: falls through to serve the live RSC stream; the HTML path is responsible for populating the cache with rscData

Fixes addressed from review

Issue	Fix
`renderFreshAndCache` called `_getNavigationContext()` after context was already nulled	Capture `_frozenNavContext` before nulling; use it in all background regen lambdas
Missing `X-Vinext-Cache: MISS` on cache miss	Added to HTML MISS response
`triggerBackgroundRegeneration` type mismatch (`Promise<string>` vs `() => Promise<void>`)	Wrapped in `async () => { await renderFreshAndCache(); }`
RSC requests bypass cache	Full RSC HIT/STALE/MISS path added; `rscStream` teed on HTML MISS to populate `rscData`
`collectStreamAsArrayBuffer` used but not defined	Added as a top-level helper in the generated entry
Cached RSC stream was never returned (structural bug)	Fixed: HIT/STALE branch now has its own `return new Response(cachedRscStream, ...)`

_{🔄 This issue represents a GitHub Pull Request. It cannot be merged through Gitea due to API limitations.}

## 📋 Pull Request Information **Original PR:** https://github.com/cloudflare/vinext/pull/359 **Author:** [@james-elicx](https://github.com/james-elicx) **Created:** 3/8/2026 **Status:** ❌ Closed **Base:** `main` ← **Head:** `fix/app-router-isr-production-guard` --- ### 📝 Commits (10+) - [`8b076c9`](https://github.com/cloudflare/vinext/commit/8b076c9c061363a6a3693cf3f4cde27201bba813) fix(app-router): implement ISR caching gated behind production NODE_ENV - [`919591e`](https://github.com/cloudflare/vinext/commit/919591e3bfc3a159ad8d229e0cbfa3dffc0b7e0e) Merge remote-tracking branch 'origin/main' into fix/app-router-isr-production-guard - [`38d7495`](https://github.com/cloudflare/vinext/commit/38d749513edcf871d4b0e44ab01c237e6f269fd9) test: update entry template snapshots for App Router ISR changes - [`aee9cc0`](https://github.com/cloudflare/vinext/commit/aee9cc05419fd98a71df22f80240d6a1899422ef) fix(isr): complete RSC caching, fix nav context, add X-Vinext-Cache: MISS - [`5be8559`](https://github.com/cloudflare/vinext/commit/5be855908a94abdeda8c99d73b68d1ebe8aef5cd) test(e2e): add app-router-prod Playwright project with production ISR lifecycle tests - [`40fee93`](https://github.com/cloudflare/vinext/commit/40fee932c4d34abf57c45069709f2d741e1926d1) ci: add app-router-prod to E2E matrix - [`40054a7`](https://github.com/cloudflare/vinext/commit/40054a733e4f992674d0b8660a4c9256c163956c) fix(isr): thread fetch tags into isrSet so revalidateTag invalidates page cache - [`2df32da`](https://github.com/cloudflare/vinext/commit/2df32daf95d3b6774d2a25a9d74b5ccef965d82a) perf(isr): move cache check before render, stream MISS response, fix CodeQL ReDoS - [`9138a1c`](https://github.com/cloudflare/vinext/commit/9138a1c696e131af6ef556fafc60bef183656f63) fix(isr): add path tags to isrSet, cache RSC MISS, fix STALE regen font timing - [`d7714a2`](https://github.com/cloudflare/vinext/commit/d7714a2d3081e1faaf64ef56b580b087bbd5f70a) perf(dev): cache loadModule() result to eliminate per-request Vite IPC overhead ### 📊 Changes **10 files changed** (+3303 additions, -582 deletions) <details> <summary>View changed files</summary> 📝 `.github/workflows/ci.yml` (+1 -0) 📝 `packages/vinext/src/cloudflare/kv-cache-handler.ts` (+27 -3) 📝 `packages/vinext/src/index.ts` (+1 -0) 📝 `packages/vinext/src/server/app-dev-server.ts` (+438 -82) 📝 `packages/vinext/src/server/app-router-entry.ts` (+2 -2) 📝 `packages/vinext/src/server/isr-cache.ts` (+16 -0) 📝 `packages/vinext/src/shims/cache.ts` (+17 -8) 📝 `playwright.config.ts` (+12 -0) 📝 `tests/__snapshots__/entry-templates.test.ts.snap` (+2548 -487) ➕ `tests/e2e/app-router-prod/isr.spec.ts` (+241 -0) </details> ### 📄 Description ## Summary - **App Router ISR now works in production**: pages with `export const revalidate = N` read from/write to the pluggable cache handler (set via `setCacheHandler`), serving HIT/STALE/MISS responses — matching Pages Router behavior - **RSC flight data cached alongside HTML**: client-side navigations (RSC requests) are served from cache without a re-render, matching Next.js's `rscData` cache entry field - **`X-Vinext-Cache` on every ISR response**: HIT, STALE, and MISS all emit the header for observability parity - **Cache-Control headers still emitted in all environments**: dev and test still get `s-maxage=N, stale-while-revalidate` on ISR pages; only the cache handler interaction is gated behind `NODE_ENV === "production"` - **`activeHandler` moved to `globalThis`**: `shims/cache.ts` now stores the active cache handler on `globalThis[Symbol.for("vinext.activeHandler")]` so all module instances in the same process (host + Vite RSC environment) share the same reference ## How it works The generated RSC entry gains full ISR logic wrapped in `if (process.env.NODE_ENV === "production")`. Vite's `define` config statically replaces `process.env.NODE_ENV` at transform time (`"development"` or `"test"` in non-production), so the cache handler interaction is dead-code-eliminated in dev/test — every request re-renders fresh. **HTML requests:** - **HIT**: returns cached HTML with `X-Vinext-Cache: HIT` - **STALE**: returns cached HTML with `X-Vinext-Cache: STALE` + triggers background regeneration (uses `_frozenNavContext` captured before request context is cleared, so `usePathname()` etc. work correctly in background renders) - **MISS**: tees `rscStream` before `handleSsr`, then collects HTML and RSC flight bytes concurrently (concurrent collection avoids a backpressure deadlock — SSR produces HTML by consuming the RSC stream, so draining them in series could stall), stores both in cache, returns with `X-Vinext-Cache: MISS` - **Dev/test**: falls through directly to `htmlStream` response with `Cache-Control` header still set **RSC requests (client-side navigation):** - **HIT/STALE**: serves cached RSC flight bytes directly as a stream — no re-render; STALE triggers background regeneration of both HTML and RSC data - **MISS**: falls through to serve the live RSC stream; the HTML path is responsible for populating the cache with `rscData` ## Fixes addressed from review | Issue | Fix | |-------|-----| | `renderFreshAndCache` called `_getNavigationContext()` after context was already nulled | Capture `_frozenNavContext` before nulling; use it in all background regen lambdas | | Missing `X-Vinext-Cache: MISS` on cache miss | Added to HTML MISS response | | `triggerBackgroundRegeneration` type mismatch (`Promise<string>` vs `() => Promise<void>`) | Wrapped in `async () => { await renderFreshAndCache(); }` | | RSC requests bypass cache | Full RSC HIT/STALE/MISS path added; `rscStream` teed on HTML MISS to populate `rscData` | | `collectStreamAsArrayBuffer` used but not defined | Added as a top-level helper in the generated entry | | Cached RSC stream was never returned (structural bug) | Fixed: HIT/STALE branch now has its own `return new Response(cachedRscStream, ...)` | --- <sub>🔄 This issue represents a GitHub Pull Request. It cannot be merged through Gitea due to API limitations.</sub>