feat(app-router): implement RSC segment cache prefetch protocol#1420
feat(app-router): implement RSC segment cache prefetch protocol#1420yunus25jmi1 wants to merge 4 commits into
Conversation
yunus25jmi1
force-pushed
the
feat/segment-cache-prefetch-protocol
branch
from
88e9125 to
6d947cb
Compare
commit: |
yunus25jmi1
force-pushed
the
feat/segment-cache-prefetch-protocol
branch
from
6d947cb to
55b9be7
Compare
|
/bigbonk review |
![ask-bonk[bot]](https://avatars.githubusercontent.com/in/2454580?s=60&v=4){kind=small}
There was a problem hiding this comment.
Review: RSC segment cache prefetch protocol (Phase 1)
The normalizer and normalization pipeline changes are clean. The URL pattern matching is correct, tests are thorough, and the architecture decision to return full-page RSC (falling through to NavigationFlightResponse) is a pragmatic Phase 1. A few things to address before merging.
Issues
-
Double regex execution —
matchSegmentPrefetchRsc+extractSegmentPrefetchRscrun the same regex twice. Since the extract function already returnsnullfor non-matches, thematchguard is redundant. -
Inline comment removal — The inline step comments (
// Step 2:,// Step 3:, etc.) innormalizeRscRequestwere useful — they tied the code to the numbered steps in the function-level doc block, making it easy to audit security-sensitive ordering. The doc block steps were renumbered, but the inline anchors are gone. Please keep them. -
Doc-comment removal on
NormalizedRscRequestfields — The existing per-field JSDoc comments were removed (/** Parsed URL. Callers may mutate... */,/** True when the request targets... */, etc.). These were useful for type hover docs. Please keep them alongside the newsegmentPrefetchPathfield. -
segmentPrefetchPathis threaded but unused — The value propagates throughDispatchMatchedPageOptions→ generated entry destructure → ... and is never consumed. This is fine for Phase 1 plumbing, but worth noting that the type contract now has a field no consumer reads. -
Missing negative test: percent-encoded
.segments— What happens when.segmentsis percent-encoded (e.g.,%2Esegments)? The strict decoder runs before segment-prefetch detection, so%2Edecodes to., meaning the regex would match. This might be fine (same behavior as.rscsuffix detection), but it should have a test documenting the expected behavior.
| segmentPrefetchPath = extracted.segmentPath; | ||
| pathname = extracted.originalPathname; | ||
| } | ||
| } |
There was a problem hiding this comment.
The matchSegmentPrefetchRsc guard is redundant — extractSegmentPrefetchRsc already returns null for non-matches, so the if (extracted) check handles it. Running the regex twice on every segment-prefetch request is wasteful on a hot path.
| } | |
| let segmentPrefetchPath: string | null = null; | |
| const extracted = extractSegmentPrefetchRsc(pathname); | |
| if (extracted) { | |
| segmentPrefetchPath = extracted.segmentPath; | |
| pathname = extracted.originalPathname; | |
| } |
| /** Parsed URL. Callers may mutate `url.search` after middleware runs. */ | ||
| url: URL; | ||
| /** Normalized pathname with basePath stripped. Used for all internal routing. */ | ||
| pathname: string; |
There was a problem hiding this comment.
The per-field JSDoc comments on NormalizedRscRequest were removed in this PR. These were useful for IDE hover docs:
/** Parsed URL. Callers may mutate `url.search` after middleware runs. */
url: URL;
/** Normalized pathname with basePath stripped. Used for all internal routing. */
pathname: string;
etc. Please keep them — the new segmentPrefetchPath doc is good, just don't remove the existing ones.
| // Step 2: Guard against protocol-relative open redirects on the raw pathname. | ||
| // normalizePath (step 4) would collapse //evil.com to /evil.com, causing the | ||
| // guard to miss it. Raw pathname must be checked first. | ||
| const protoGuard = guardProtocolRelativeUrl(url.pathname); |
There was a problem hiding this comment.
The inline step comments (// Step 2: Guard against protocol-relative..., // Step 3: Strict segment-wise percent-decode..., etc.) were removed. These are valuable — they tie each code block to the numbered steps in the function-level doc block, making it easy to audit that the security-sensitive ordering hasn't drifted. The doc block steps were correctly renumbered; please keep the inline anchors too.
For example:
// Step 2: Guard against protocol-relative open redirects on the raw pathname.
const protoGuard = guardProtocolRelativeUrl(url.pathname);
| const RSC_SEGMENT_SUFFIX = ".segment.rsc"; | ||
|
|
||
| const SEGMENT_PREFETCH_PATTERN = new RegExp( | ||
| `^(/.*)${escapeRegex(RSC_SEGMENTS_DIR_SUFFIX)}(/.*)${escapeRegex(RSC_SEGMENT_SUFFIX)}$`, |
There was a problem hiding this comment.
The first capture group (/.*) is greedy, which means for a path like /foo.segments/bar.segments/_tree.segment.rsc, the first group would capture /foo.segments/bar (consuming up to the last .segments). This is probably the correct behavior (the last .segments is the real protocol delimiter), but it's worth a brief comment explaining this is intentional, and ideally a test case for a path containing a literal .segments in the page portion.
Also — minor style nit: escapeRegex is called at module scope before its definition (line 24). This works because of hoisting, but reads oddly. Consider moving the helper above its first use.
| expect( | ||
| matchSegmentPrefetchRsc("/dashboard/settings.segments/tab/profile/__PAGE__.segment.rsc"), | ||
| ).toBe(true); | ||
| }); |
There was a problem hiding this comment.
Consider adding a test for percent-encoded .segments (e.g., /dashboard%2Esegments/_tree.segment.rsc). In the normalization pipeline, normalizePathnameForRouteMatchStrict decodes %2E to . before the segment-prefetch check runs, so this URL would match. That's probably fine (consistent with how .rsc suffix detection works post-decode), but it should be documented with a test.
|
Review posted. Here's a summary of the feedback: Overall: The normalizer and pipeline changes are solid. The URL pattern matching is correct, tests are thorough (22 unit + 7 integration), and the Phase 1 strategy of returning full-page RSC payloads is pragmatic. Key issues raised:
|
…dflare#1335) vinext currently has no endpoint for the Next.js 15+ RSC segment cache prefetch protocol. The client-side router expects to make segment-level prefetch requests (e.g. `/_tree`, `/dashboard/__PAGE__`) to URLs like `/page.segments/_tree.segment.rsc` and receive segment-specific RSC payloads. Without this endpoint, ~66 deploy suite tests time out waiting for segment-prefetch requests that never fire. This PR implements Phase 1 of the protocol — the server-side URL handling and request routing. When a request arrives at `.segments/*.segment.rsc`, the normalizer rewrites it back to the original page path and marks it as a segment-prefetch RSC request. The page renders normally and returns a valid RSC response that the client segment cache can process. Implementation: - New `app-segment-prefetch-normalizer.ts` — matches `.segments/*.segment.rsc` URLs and extracts the original page path + segment path - Updated `normalizeRscRequest()` — detects segment-prefetch URLs in the normalization pipeline, rewrites the pathname back to the page path, sets `segmentPrefetchPath` on the normalized result - Updated `app-rsc-handler.ts` — passes `segmentPrefetchPath` through `DispatchMatchedPageOptions` so downstream handlers can access it - Updated `app-rsc-entry.ts` — accepts `segmentPrefetchPath` in the generated `dispatchMatchedPage` function signature - No response format changes yet — segment-prefetch requests receive a full page RSC response (same as a regular `.rsc` prefetch), which the client's non-PPR code path handles gracefully Why no `Next-Did-Postpone: 2` header: Without PPR, setting this header would cause the client to attempt parsing the response as a `RootTreePrefetch` structure. Returning the response without it lets the client fall through to the NavigationFlightResponse code path, which correctly handles the full-page RSC payload. Trade-offs: - This is intentionally minimal — the endpoint exists, responds with valid RSC content, and doesn't error. Full segment-level response generation (proper `RootTreePrefetch` / `SegmentPrefetchResponse` payloads) is deferred to a follow-up. - Per-segment caching and the client-side segment cache scheduler are not implemented. The server returns the full page RSC for all segment prefetch requests, which is functionally correct but doesn't provide the granular caching benefits of the full protocol. - Complex segment-cache E2E tests can be skipped with `adapter-api-e2e` annotations until Phase 2+ is complete. Testing: 22 unit tests for the URL normalizer (match + extract), 7 integration tests for the request normalization pipeline. All existing tests pass (251 across 6 core test files, 313 app-router, 292 features).
…s, add percent-encode tests
yunus25jmi1
force-pushed
the
feat/segment-cache-prefetch-protocol
branch
from
b8961c6 to
c0eb026
Compare
2 participants
Problem
vinext does not implement the Next.js 15+ RSC segment cache prefetch protocol. The client-side router expects to make segment-level prefetch requests (with
Next-Router-Segment-Prefetchheader) to URLs in the format/page-path.segments/_tree.segment.rscand receive segment-specific RSC payloads.Without this endpoint, the "Next.js Deploy Suite" (#1328) tests time out waiting for segment-prefetch requests that never fire. Estimated impact: ~66 test failures across the deploy suite.
Fixes #1335.
What this PR implements (Phase 1)
This adds the server-side URL handling for the segment cache prefetch protocol. When a request arrives at
.segments/*.segment.rsc, the handler:/dashboard.segments/_tree.segment.rsc-> pathname=/dashboard, segmentPath=/_tree)isRscRequest=true,segmentPrefetchPathset)The client segment cache receives the full-page RSC response and falls through to the
NavigationFlightResponsecode path, which correctly handles it.Files changed
packages/vinext/src/server/app-segment-prefetch-normalizer.ts.segments/*.segment.rscURLs, extracts original path + segment pathpackages/vinext/src/server/app-rsc-request-normalization.tspackages/vinext/src/server/app-rsc-handler.tssegmentPrefetchPaththroughDispatchMatchedPageOptionspackages/vinext/src/entries/app-rsc-entry.tssegmentPrefetchPathin generateddispatchMatchedPagetests/app-segment-prefetch-normalizer.test.tstests/app-rsc-request-normalization.test.tsNot in this PR (follow-up)
RootTreePrefetch/SegmentPrefetchResponsepayloads requires rendering the page and walking the RSC output tree to extract per-segment data. This is deferred as Phase 2.Next-Did-Postpone: 2header: Intentionally omitted. Without it, the client falls through to the non-PPR code path that correctly processes full-page RSC payloads.collectSegmentData. vinext can do the same once the runtime protocol is solid.Trade-offs
text/x-componentcontent, and doesn't error. This is sufficient for the deploy suite tests that only check for request initiation.Next-Did-Postpone: 2: Setting this would cause the client to attemptRootTreePrefetchparsing. Omitting it allows graceful fallback to the existing prefetch format.adapter-api-e2eannotations until Phase 2+ is ready.Testing
Closes: #1335
Related: #1328