[james-elicx]

Summary

Fixes the hover/visible-link RSC prefetch regression introduced after client cache reuse landed.

The bug was that Link-triggered full prefetches were using the dynamic navigation stale window as their fallback freshness. With Next.js defaults (staleTimes.dynamic = 0), a just-prefetched payload could be treated as stale by the visible-link ping/dedupe path, so app-router commits or hover/focus churn could issue extra RSC prefetch requests instead of reusing the existing prefetch entry.

This keeps committed navigation snapshots on dynamic freshness, but restores fetched prefetch entries to prefetch-cache freshness:

• Link prefetches use the static/full prefetch window as their fallback freshness.
• Server-provided stale time headers on fetched prefetch responses are clamped to a 30s minimum, matching Next.js segment-cache behavior (getStaleTimeMs).
• Added regression coverage for visible-link pings with dynamic=0 and for the minimum prefetch stale-time clamp.

Validation

• vp test run tests/link-navigation.test.ts tests/prefetch-cache.test.ts
• vp check packages/vinext/src/shims/link.tsx packages/vinext/src/shims/navigation.ts tests/link-navigation.test.ts tests/prefetch-cache.test.ts

[pkg-pr-new]

Open in StackBlitz

npm i https://pkg.pr.new/@vinext/cloudflare@2396

npm i https://pkg.pr.new/vinext@2396

commit: 4ecfcd5

[github-actions]

Example	Preview	Production	Original
app-router-cloudflare	preview	production
pages-router-cloudflare	preview	production
app-router-playground	preview	production	original
realworld-api-rest	preview	production
nextra-docs-template	preview	production
benchmarks	preview	production
hackernews	preview	production	original
workers-cache	preview	production
web	preview	production

[james-elicx]

/bigbonk review for issues

[github-actions]

Performance benchmarks

Compared 4ecfcd5 against base f0ba7b4 using alternating same-runner rounds. Next.js was unchanged and skipped.

2 improved · 0 regressed · 4 within ±1.5%

Scenario	Framework	Baseline	Current	Change
Client bundle size (gzip)	vinext	123.4 KB	123.4 KB	⚫ +0.0%
Client entry size (gzip)	vinext	118.3 KB	118.4 KB	⚫ +0.0%
Dev server cold start	vinext	2.64 s	2.57 s	🟢 -2.7%
Production build time	vinext	3.20 s	3.14 s	🟢 -1.8%
RSC entry closure size (gzip)	vinext	94.5 KB	94.5 KB	⚫ +0.0%
Server bundle size (gzip)	vinext	160.6 KB	160.6 KB	⚫ +0.0%

View detailed results and traces

_{🟢 improvement · 🔴 regression · ⚫ change below 1.5% · paired base/head}

[ask-bonk]

Review: fix(app-router): avoid repeated hover prefetches

Reviewed the change against Next.js segment-cache behavior. The core fix is correct and the 30s minimum clamp matches Next.js's getStaleTimeMs (Math.max(staleTimeSeconds, 30) * 1000) exactly. Tests pass locally (vitest run tests/link-navigation.test.ts tests/prefetch-cache.test.ts → 83 passed).

A few observations, none blocking:

1. The 30s clamp is not applied to the fallbackTtlMs path (minor parity gap)

In resolvePrefetchedRscResponseExpiresAt (navigation.ts:359), the Math.max(..., MIN_PREFETCH_STALE_TIME_MS) clamp only runs when the server sent a valid dynamicStaleTimeSeconds. When the header is absent, the fallback path returns timestamp + fallbackTtlMs unclamped:

const seconds = cached.dynamicStaleTimeSeconds;
if (!isDynamicStaleTimeSeconds(seconds)) {
  return timestamp + fallbackTtlMs;   // not clamped to MIN_PREFETCH_STALE_TIME_MS
}
return timestamp + Math.max(seconds * 1000, MIN_PREFETCH_STALE_TIME_MS);

Next.js's getStaleTimeMs clamps all prefetch stale times to a 30s floor, including the configured static stale time. With the default config this is harmless (PREFETCH_CACHE_TTL is 300s when the plugin is active, 30s in tests), so the gap only surfaces if a user sets experimental.staleTimes.static below 30s — then vinext would honor a sub-30s prefetch window where Next.js would floor it. Worth either clamping the fallback too (closer parity) or adding a code comment that the fallback is assumed to already be ≥30s.

2. Committed-navigation snapshots inherit the prefetch expiresAt via the consume path (pre-existing seam, worth confirming intent)

The PR body says it "keeps committed navigation snapshots on dynamic freshness, but restores fetched prefetch entries to prefetch-cache freshness." That split holds for the un-consumed prefetch path, but note the consume→commit path:

• consumePrefetchResponse (navigation.ts:888) now always synthesizes expiresAt onto the returned snapshot, since prefetchRscResponse always sets entry.expiresAt.
• app-browser-entry.ts:1878 captures that as navResponseExpiresAt, then spreads it onto the committed snapshot at app-browser-entry.ts:2137.
• storeVisitedResponseSnapshot passes DYNAMIC_NAVIGATION_CACHE_TTL as the fallback (lines 2172 / 2183) intending dynamic freshness, but resolveCachedRscResponseExpiresAt short-circuits on the explicit expiresAt, so the committed snapshot inherits the prefetch-cache window (now clamped/static) rather than the dynamic window.

This propagation predates the PR (introduced in #2251), so it's not a regression this PR creates — but this PR changes the value that flows through it (full/auto prefetches now resolve to the static prefetch window + 30s floor instead of the dynamic window). With staleTimes.dynamic = 0, a navigation that consumes a prefetch will commit a snapshot that stays fresh for ≥30s instead of being immediately revalidatable, which is arguably contrary to the "committed snapshots on dynamic freshness" framing. If that's intended (matching Next.js, where consumed prefetch data carries its prefetch stale time into the committed tree), a one-line comment at app-browser-entry.ts:2137 would prevent a future reader from "fixing" it. If not intended, the commit path should re-resolve expiry against DYNAMIC_NAVIGATION_CACHE_TTL instead of inheriting the prefetch value.

3. Test coverage gap for the consume→commit path

The two new tests cover (a) the visible-link ping not re-prefetching and (b) the 30s clamp in prefetchRscResponse. Neither exercises the consume→commit handoff described in #2 — i.e., that a navigation which consumes a dynamic=0 prefetch commits a visited-response entry with the expected expiry. Adding a test there would lock in whichever behavior is intended and guard the seam.

Nits

• The renamed test "honors an explicit fallback stale window for prefetched responses" (prefetch-cache.test.ts:468) still passes DYNAMIC_NAVIGATION_CACHE_TTL as the explicit fallback. That's fine as a generic "explicit fallback is honored" assertion, but the name now reads as if it no longer ties to the auto-prefetch behavior it originally covered. Consider asserting the auto/full-mode resolution explicitly so the auto-prefetch fallback choice stays covered after the link.tsx conditional was removed.

Overall the change is correct and well-scoped for the stated regression. The main thing to resolve before merge is confirming the intent in #2 (commit-path expiry inheritance) and ideally adding the test in #3.

[ask-bonk]

Review posted to PR #2396. Summary of findings:

The fix is correct and well-scoped. The 30s minimum clamp matches Next.js's getStaleTimeMs (Math.max(staleTimeSeconds, 30) * 1000) exactly, and the targeted tests pass locally (83 passed).

Three non-blocking observations:

1. Clamp not applied to the fallback path — resolvePrefetchedRscResponseExpiresAt only clamps the server-provided dynamicStaleTimeSeconds to 30s, not the fallbackTtlMs path. Harmless with default config (PREFETCH_CACHE_TTL ≥ 30s), but diverges from Next.js if a user sets experimental.staleTimes.static below 30s.

2. Committed-snapshot expiry inheritance — The consume→commit path (app-browser-entry.ts:1878 → :2137) propagates the prefetch entry's expiresAt onto committed snapshots, short-circuiting the DYNAMIC_NAVIGATION_CACHE_TTL fallback that storeVisitedResponseSnapshot passes. This seam predates the PR (fix(app-router): reuse committed client cache payloads #2251), but this PR changes the value flowing through it (auto/full prefetches now resolve to static window + 30s floor instead of dynamic). Worth confirming this matches the PR's "committed snapshots on dynamic freshness" framing, or adding a clarifying comment.

3. Test gap — No test covers the consume→commit handoff for a dynamic=0 prefetch, which is the exact scenario the seam in Published npm package ships sourcemaps pointing to missing source files #2 affects.

Plus a nit on the renamed test name no longer reflecting the auto-prefetch behavior it originally guarded.

github run

[james-elicx]

/bigbonk review for issues

[ask-bonk]

Re-review after 4ecfcd5 (floor prefetch fallback stale time)

Re-reviewed the head of the branch against Next.js segment-cache (packages/next/src/client/components/segment-cache/cache.ts). All three observations from the earlier review have been resolved by 4ecfcd5, and the fix now matches Next.js behavior. No blocking issues.

Prior observations — resolved

1. Fallback path now floored. resolvePrefetchedRscResponseExpiresAt (navigation.ts:368-369) now clamps the no-header fallback to MIN_PREFETCH_STALE_TIME_MS (timestamp + Math.max(fallbackTtlMs, MIN_PREFETCH_STALE_TIME_MS)). This closes the sub-30s staleTimes.static parity gap and matches getStaleTimeMs (Math.max(staleTimeSeconds, 30) * 1000). Note: max(s, 30)*1000 === max(s*1000, 30000), so the seconds-path form is equivalent to Next.js. New test floors explicit fallback stale windows for prefetched responses locks this in.

2. Commit-path inheritance documented. The comment at app-browser-entry.ts:2137 now states the intent explicitly: a navigation that consumes prefetched Flight data keeps the prefetch freshness window on commit, matching Next's segment-cache handoff. This confirms the seam is intentional rather than an accidental divergence from the "committed snapshots on dynamic freshness" framing.

3. Consume→commit coverage added. tests/app-visited-response-cache.test.ts now has inherits the expiry carried by a consumed prefetch snapshot, and the dynamic=0 test in prefetch-cache.test.ts asserts the consumed snapshot carries expiresAt === now + 30_000. The seam is now guarded.

Verified independently

• dynamicStaleTimeSeconds = 0 (valid per isDynamicStaleTimeSeconds, >= 0) correctly hits Math.max(0, 30000) → 30s floor, matching Next's "too-short server stale time" clamp. Covered.
• The link.tsx change (both auto and full prefetches now use PREFETCH_CACHE_TTL as the fallback window) matches Next.js: segment-cache computes staleAt from the same getStaleTimeMs/STATIC_STALETIME_MS path regardless of PrefetchKind. The prior DYNAMIC_NAVIGATION_CACHE_TTL fallback for auto prefetches was the actual divergence this PR removes. The auto-vs-full distinction is about what is cached (shell vs full tree), not the stale window.
• tests/prefetch-cache.test.ts, tests/link-navigation.test.ts, tests/app-visited-response-cache.test.ts: 91 passed.
• Typecheck clean on the three changed source files.

Nit (non-blocking)

• The Next.js no-header default is STATIC_STALETIME_MS (getStaleAt/getStaleAtFromHeader in cache.ts:3408,3451), not a 30s floor — Next only applies the 30s floor when a stale time value (header or config) is present. vinext's fallback now always floors to 30s. In practice PREFETCH_CACHE_TTL is the configured static stale time and is ≥30s by default, so this is effectively equivalent, but it's a slightly different shape from Next's "default = static, floor only the configured value." Worth a one-line note if you want exact parity, but harmless under default config.

LGTM.

[ask-bonk]

Review posted to PR #2396.

Summary

I re-reviewed PR #2396 at its current head (4ecfcd5), which added a second commit since the prior review. No blocking issues.

All three prior observations were resolved by 4ecfcd5:

1. Fallback path now floored — resolvePrefetchedRscResponseExpiresAt clamps the no-header fallback to MIN_PREFETCH_STALE_TIME_MS (30s), closing the sub-30s staleTimes.static parity gap. The clamp form is mathematically equivalent to Next.js's getStaleTimeMs (Math.max(s, 30) * 1000).

2. Commit-path inheritance documented — A comment at app-browser-entry.ts:2137 now states the intentional prefetch→commit freshness handoff, matching Next's segment-cache.

3. Consume→commit coverage added — New test inherits the expiry carried by a consumed prefetch snapshot, plus a consume assertion on the dynamic=0 test.

Verified independently:

• dynamicStaleTimeSeconds = 0 correctly hits the 30s floor (matches Next's too-short-stale-time clamp).
• The link.tsx change (both auto and full prefetches use PREFETCH_CACHE_TTL fallback) matches Next.js — segment-cache computes staleAt the same way regardless of PrefetchKind; the old DYNAMIC_NAVIGATION_CACHE_TTL fallback for auto prefetches was the actual divergence being fixed.
• 91 targeted tests pass; typecheck clean on the three changed source files.

One non-blocking nit: Next's no-header default is STATIC_STALETIME_MS (it floors only when a stale value is present), whereas vinext's fallback always floors to 30s. Effectively equivalent under default config since PREFETCH_CACHE_TTL is ≥30s, but a slightly different shape if exact parity is desired.

github run