fix(app-router): send the request's params and path on core RSC cache hits - #3460
james-elicx wants to merge 6 commits into
Conversation
|
/bigbonk review for issues Please review exact head |
@vinext/cloudflare
create-vinext-app
@vinext/types
vinext
@cloudflare/workers-response-store
commit: |
![ask-bonk[bot]](https://avatars.githubusercontent.com/in/2454580?s=60&v=4){kind=small}
| middlewareHeaders: options.middlewareContext.headers, | ||
| middlewareStatus: options.middlewareContext.status, | ||
| mountedSlotsHeader: options.mountedSlotsHeader, | ||
| params: options.params, |
There was a problem hiding this comment.
Use the resolved navigation params here rather than the raw matched-route params. Fresh RSC responses pass navigationParams to buildAppPageRscResponse (app-page-render.ts:1090); those include active parallel-slot and interception params added by resolveAppPageNavigationParams. This cache-hit return happens before that resolution and now emits only options.params, so useParams() loses slot-local/intercepted params specifically on core HIT/STALE responses. Resolve the equivalent navigation params before serving the cache entry, or defer composing this header until they are known.
There was a problem hiding this comment.
Fixed in fff0257. A hit now resolves the same render target that the entry's regeneration uses: resolveAppPageInterceptionRerenderTarget, then resolveAppPageNavigationParams, which now sit in one shared helper. So it sends the navigation params with active slot and interception params, as a fresh render does. The dispatch test now uses a route with an active slot that contributes a param, and it fails without the fix.
|
Posted 1 actionable inline finding. Focused tests pass. |
Performance benchmarksCompared 0 improved · 0 regressed · 6 within ±1.5%
View detailed results and traces 🟢 improvement · 🔴 regression · ⚫ change below 1.5% · paired base/head |
297ad57 to
fff0257
Compare
|
/bigbonk review for issues Please review exact head |
| // the slot param merge. resolveAppPageNavigationParams preserves all | ||
| // base keys and overlays active slot params on top; when narrowed | ||
| // params were used here, non-slot extras were silently dropped. | ||
| const mergedNavigationParams = resolveAppPageNavigationParams( |
There was a problem hiding this comment.
Hydrate revalidationTarget.route before resolving these navigation params. For a source-route interception, getSourceRoute() returns the lazy route table entry; its non-intercepted parallel slots still have page/default unset until ensureRouteLoaded runs. resolveAppPageNavigationParams therefore skips those active slots and a core HIT returns an incomplete X-Vinext-Params header. The existing ensureRouteLoaded in renderFreshPageForCache runs only after this helper and is never reached on a HIT. Moving that await into this helper before this call preserves the same slot params as the fresh intercept path.
There was a problem hiding this comment.
Fixed in 3e1ad4f. The shared render-target helper now awaits ensureRouteLoaded(revalidationTarget.route) before resolving navigation params, so a hit sees the source route's lazy slots. The regeneration path reuses it. The new dispatch test "hydrates an intercepting source route before resolving a cached RSC hit's params" fails without the fix.
|
Posted 1 actionable inline finding. Focused tests and checks pass. |
|
fff0257 to
3e1ad4f
Compare
|
/bigbonk review for issues Please review exact head |
|
LGTM! |
3e1ad4f to
02276d2
Compare
|
/bigbonk review for issues Please review exact head |
| middlewareStatus: options.middlewareContext.status, | ||
| mountedSlotsHeader: options.mountedSlotsHeader, | ||
| params: options.isRscRequest | ||
| ? (await resolveCacheRenderTarget()).navigationParams |
There was a problem hiding this comment.
The resolved navigation params set here are overwritten in split response-stage deployments. After this response returns, app-rsc-handler.ts:1183-1195 unconditionally rebuilds X-Vinext-Params from props.params, which contains only the matched route params. Active-slot and source-interception params added by resolveCacheRenderTarget() are therefore still lost on the default transported cache path. Please pass the resolved params through the response-stage envelope or avoid replacing this header with the raw params.
There was a problem hiding this comment.
This one is outside this PR. In split response-stage deployments, the request stage has rebuilt X-Vinext-Params from props.params (matched-route params only) since 2978637 on main (2026-09-07). That applies to fresh renders as much as to hits, so active-slot and interception params already fail to reach the client there. This PR fixes the core path, where the hit is composed in dispatch. Carrying navigation params through the response-stage envelope would change that transport contract, so I've noted it as a follow-up rather than widening this PR.
|
Posted 1 actionable inline finding. Focused tests and checks pass. |
02276d2 to
904e1ce
Compare
|
/bigbonk review for issues Please review exact head |
| middlewareStatus: options.middlewareContext.status, | ||
| mountedSlotsHeader: options.mountedSlotsHeader, | ||
| params: options.isRscRequest | ||
| ? (await resolveCacheRenderTarget()).navigationParams |
There was a problem hiding this comment.
Resolve this target only after the cache helper has confirmed a serveable HIT/STALE entry. Because this argument is awaited before readAppPageCacheResponse starts, every cache-eligible RSC request now runs interception loading and ensureRouteLoaded before the cache lookup and, importantly, before validateAppPageDynamicParams below. An invalid generateStaticParams request that should return the route’s 404 can therefore evaluate a lazy intercepted source module first (and become a 500 if that import throws). Please make params resolution lazy at the point a cached response is actually built, or otherwise defer source-route hydration until a usable entry is known.
There was a problem hiding this comment.
Fixed in 652def2. readAppPageCacheResponse now takes resolveParams and calls it only while building a HIT/STALE response for an RSC entry with rscData. A miss reaches validateAppPageDynamicParams without loading the intercepted source.
|
Posted 1 actionable inline finding. Focused tests and checks pass. |
904e1ce to
652def2
Compare
|
/bigbonk review for issues Please review exact head |
|
LGTM! |
38ca6a6 to
eca6016
Compare
eca6016 to
43927a7
Compare
43927a7 to
ffc6f09
Compare
26830ad to
a94d8c6
Compare
a94d8c6 to
0000e2a
Compare
0000e2a to
8a724cb
Compare
|
/bigbonk review for issues Please review exact head |
|
LGTM! |
… hits
A core RSC cache hit sent neither X-Vinext-Params nor
X-Vinext-Rendered-Path-And-Search. The browser reads the params for
useParams() after a navigation and falls back to {} when the header is
missing, and the prefetch cache aliases entries by the rendered path. A
fresh render sends both, and the response-stage transport composes them
above the adapter for its hits.
Hits and stale hits now compose both from the current request, since they
describe the request rather than the shared RSC bytes.
A cache hit sent the matched route's params, while a fresh render sends the navigation params, which add active parallel-slot and interception params. Hits now resolve the same render target the entry's regeneration uses.
…g hit params A source route's slots are lazy until the route is loaded, so a cache hit for a source-route interception skipped its active slots' params.
…ntry Resolving a hit's navigation params loads interception source routes, and it ran before the cache lookup and the generated-params check. A request outside generateStaticParams could load route modules before its 404. The cache helper now resolves them only when building a HIT or STALE RSC response.
…ponses, as on fresh ones
…ilure Resolving a cached RSC response's params can load interception and source route modules. Its failure was caught as an ISR read error and turned into a MISS, so the fresh-render path retried the load and the current request never observed the failure. Rethrow it past the cache read's catch on HIT and STALE.
8a724cb to
3da2dd7
Compare
1 participant
Stacked on #3459. Second part of plan PR 3: RSC hits on the core cache path.
Problem
The browser reads route params from
X-Vinext-Params(falling back to{}) and the rendered path fromX-Vinext-Rendered-Path-And-Search. A fresh RSC render sets both, and the Response Store and Workers Cache paths compose them at the request stage. But an RSC response served from core's cache (memory/KV) had neither. A soft navigation served from a core HIT or STALE entry therefore lost its dynamic params, and lost the rewritten path and query.Change
buildAppPageCachedResponsenow composes both headers on RSC HIT and STALE responses from the current request's params and rendered path and search. They aren't read from the stored entry, which every query shares.X-Vinext-Paramsis only sent when there are params, as on a fresh render. HTML responses are unchanged.Tests
tests/app-page-cache.test.ts: HIT and STALE RSC responses carry both headers, HTML doesn't, and empty params omitX-Vinext-Params.tests/app-page-dispatch.test.ts: a cached RSC hit sends the request's params and path. It fails without the change.