[PR #610] [CLOSED] fix: detect dynamic API usage in route handlers to prevent stale ISR cache hits #710

New issue

Closed

opened 2026-05-06 13:09:42 +02:00 by BreizhHardware · 0 comments

BreizhHardware commented

2026-05-06 13:09:42 +02:00

Owner

📋 Pull Request Information

Original PR: https://github.com/cloudflare/vinext/pull/610
Author: @southpolesteve
Created: 3/20/2026
Status: ❌ Closed

Base: main ← Head: fix/route-handler-dynamic-detection

📝 Commits (10+)

efcda09 fix: detect dynamic API usage in route handlers to prevent stale ISR cache hits
057bcda fix: wrap synthetic request in proxy during background regeneration
30b6096 fix: remove TypeScript syntax from generated JavaScript entry
dfd4425 fix: bind proxied Request methods to original target
26b52e5 fix: clear dynamic usage flag before handler execution
72ab864 fix: clear dynamic usage in background regen path too
fff3b10 refactor: extract ISR cache logic into wrapper and use counter-based dynamic detection
39a83d0 fix: update unified-request-context tests for dynamicUsageCount rename
34aac05 fix: reset dynamic counter before handler and fix remaining test assertions
b0302cf fix: use consumeDynamicUsage() instead of counter for handler detection

📊 Changes

7 files changed (+1523 additions, -911 deletions)

View changed files

📝 packages/vinext/src/build/prerender.ts (+1 -1)
📝 packages/vinext/src/entries/app-rsc-entry.ts (+210 -125)
📝 packages/vinext/src/shims/headers.ts (+16 -8)
📝 packages/vinext/src/shims/unified-request-context.ts (+1 -1)
📝 tests/__snapshots__/entry-templates.test.ts.snap (+1260 -750)
📝 tests/app-router.test.ts (+17 -8)
📝 tests/unified-request-context.test.ts (+18 -18)

📄 Description

Summary

Route handlers that access request.headers or request.nextUrl.searchParams are dynamic and should not be served from the ISR cache. This PR adds proper dynamic detection matching Next.js behavior, replacing the query-string-in-cache-key approach from PR #603.

How it works

Three new mechanisms:

__proxyRouteRequest: Wraps the Request passed to route handlers in a Proxy. Accessing .headers or .nextUrl.searchParams calls markDynamicUsage(), which the existing consumeDynamicUsage() check on the write path already respects.
__dynamicRouteHandlers: A module-level Set<string> that remembers route patterns whose handlers used dynamic APIs. On the first request (cache MISS), the handler runs and dynamic usage is detected. On subsequent requests, the cache read path skips the cache for known-dynamic handlers.
Cache read guard: The ISR cache read condition now includes !__dynamicRouteHandlers.has(handler.pattern), preventing stale cache hits for dynamic handlers.

Why not query-string-in-cache-key?

PR #603 took a different approach: including the query string in the ISR cache key. James pointed out that this diverges from Next.js, which uses pathname-only cache keys (source). In Next.js, handlers that read searchParams are detected as dynamic and never cached at all.

This PR matches that behavior: dynamic handlers are never cached, static handlers use pathname-only keys.

Granularity

The Proxy is granular about what triggers dynamic detection:

Access	Dynamic?	Reason
`request.method`	No	Same for all ISR requests
`request.url` (string)	No	Can't track downstream usage
`request.nextUrl.pathname`	No	Pathname is the route
`request.nextUrl.searchParams`	Yes	Varies per request
`request.headers`	Yes	Varies per request

The one gap (same as Next.js): new URL(request.url).searchParams bypasses the Proxy since it operates on the raw URL string. Developers should use request.nextUrl.searchParams instead.

_{🔄 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/610 **Author:** [@southpolesteve](https://github.com/southpolesteve) **Created:** 3/20/2026 **Status:** ❌ Closed **Base:** `main` ← **Head:** `fix/route-handler-dynamic-detection` --- ### 📝 Commits (10+) - [`efcda09`](https://github.com/cloudflare/vinext/commit/efcda0977149fa2532a12f831041e25d820d58a2) fix: detect dynamic API usage in route handlers to prevent stale ISR cache hits - [`057bcda`](https://github.com/cloudflare/vinext/commit/057bcda57732f67115cf8dd568f6cec027a99c5e) fix: wrap synthetic request in proxy during background regeneration - [`30b6096`](https://github.com/cloudflare/vinext/commit/30b60961eeb6f264a11f4b43c44d7abfebf4f55f) fix: remove TypeScript syntax from generated JavaScript entry - [`dfd4425`](https://github.com/cloudflare/vinext/commit/dfd4425c2b322a70cc8760817ccd34d28a291f7c) fix: bind proxied Request methods to original target - [`26b52e5`](https://github.com/cloudflare/vinext/commit/26b52e5d07eb97695278c34e8904bc4afa1118bf) fix: clear dynamic usage flag before handler execution - [`72ab864`](https://github.com/cloudflare/vinext/commit/72ab864a8f8ca7d4a2c9107402d983a30ac0386f) fix: clear dynamic usage in background regen path too - [`fff3b10`](https://github.com/cloudflare/vinext/commit/fff3b108d37afde42acc903827bf100d92ff2a03) refactor: extract ISR cache logic into wrapper and use counter-based dynamic detection - [`39a83d0`](https://github.com/cloudflare/vinext/commit/39a83d0b5a051e85a330a7fb6527cf3e16db0f5b) fix: update unified-request-context tests for dynamicUsageCount rename - [`34aac05`](https://github.com/cloudflare/vinext/commit/34aac053f913c8456b78ddafe682e8f6ff06aa79) fix: reset dynamic counter before handler and fix remaining test assertions - [`b0302cf`](https://github.com/cloudflare/vinext/commit/b0302cf6be9cb0568744e632eca83a57e1a7d880) fix: use consumeDynamicUsage() instead of counter for handler detection ### 📊 Changes **7 files changed** (+1523 additions, -911 deletions) <details> <summary>View changed files</summary> 📝 `packages/vinext/src/build/prerender.ts` (+1 -1) 📝 `packages/vinext/src/entries/app-rsc-entry.ts` (+210 -125) 📝 `packages/vinext/src/shims/headers.ts` (+16 -8) 📝 `packages/vinext/src/shims/unified-request-context.ts` (+1 -1) 📝 `tests/__snapshots__/entry-templates.test.ts.snap` (+1260 -750) 📝 `tests/app-router.test.ts` (+17 -8) 📝 `tests/unified-request-context.test.ts` (+18 -18) </details> ### 📄 Description ## Summary Route handlers that access `request.headers` or `request.nextUrl.searchParams` are dynamic and should not be served from the ISR cache. This PR adds proper dynamic detection matching Next.js behavior, replacing the query-string-in-cache-key approach from PR #603. ## How it works Three new mechanisms: 1. **`__proxyRouteRequest`**: Wraps the `Request` passed to route handlers in a Proxy. Accessing `.headers` or `.nextUrl.searchParams` calls `markDynamicUsage()`, which the existing `consumeDynamicUsage()` check on the write path already respects. 2. **`__dynamicRouteHandlers`**: A module-level `Set<string>` that remembers route patterns whose handlers used dynamic APIs. On the first request (cache MISS), the handler runs and dynamic usage is detected. On subsequent requests, the cache read path skips the cache for known-dynamic handlers. 3. **Cache read guard**: The ISR cache read condition now includes `!__dynamicRouteHandlers.has(handler.pattern)`, preventing stale cache hits for dynamic handlers. ## Why not query-string-in-cache-key? PR #603 took a different approach: including the query string in the ISR cache key. [James pointed out](https://github.com/cloudflare/vinext/pull/603#pullrequestreview-2903462834) that this diverges from Next.js, which uses pathname-only cache keys ([source](https://github.com/vercel/next.js/blob/9f181bd11af5f532c529a6f168fe40505f0a4d75/packages/next/src/build/templates/app-route.ts#L181-L187)). In Next.js, handlers that read searchParams are detected as dynamic and never cached at all. This PR matches that behavior: dynamic handlers are never cached, static handlers use pathname-only keys. ## Granularity The Proxy is granular about what triggers dynamic detection: | Access | Dynamic? | Reason | |---|---|---| | `request.method` | No | Same for all ISR requests | | `request.url` (string) | No | Can't track downstream usage | | `request.nextUrl.pathname` | No | Pathname is the route | | `request.nextUrl.searchParams` | **Yes** | Varies per request | | `request.headers` | **Yes** | Varies per request | The one gap (same as Next.js): `new URL(request.url).searchParams` bypasses the Proxy since it operates on the raw URL string. Developers should use `request.nextUrl.searchParams` instead. --- <sub>🔄 This issue represents a GitHub Pull Request. It cannot be merged through Gitea due to API limitations.</sub>