[PR #1035] [MERGED] refactor(app-rsc): extract response finalisation into server/app-rsc-response-finalizer #1036

New issue

Closed

opened 2026-05-06 13:11:41 +02:00 by BreizhHardware · 0 comments

BreizhHardware commented

2026-05-06 13:11:41 +02:00

Owner

📋 Pull Request Information

Original PR: https://github.com/cloudflare/vinext/pull/1035
Author: @NathanDrake2406
Created: 5/3/2026
Status: ✅ Merged
Merged: 5/3/2026
Merged by: @james-elicx

Base: main ← Head: refactor/app-rsc-response-finalizer

📝 Commits (2)

7037c10 refactor(app-rsc): introduce app-rsc-request-normalization module
d5e9b24 refactor(app-rsc): extract response finalisation into typed helper

📊 Changes

4 files changed (+279 additions, -22 deletions)

View changed files

📝 packages/vinext/src/entries/app-rsc-entry.ts (+12 -21)
➕ packages/vinext/src/server/app-rsc-response-finalizer.ts (+70 -0)
📝 tests/app-router.test.ts (+1 -1)
➕ tests/app-rsc-response-finalizer.test.ts (+196 -0)

📄 Description

Based on #1034

What this changes

Extracts the App Router response finalisation step from an inline template string in the generated handler() function into a dedicated typed module: server/app-rsc-response-finalizer.ts.

The extracted module owns:

Skipping 3xx redirect responses. Response.redirect() creates immutable Headers that throw TypeError: Cannot modify immutable headers on write. Next.js also excludes config headers from redirects.
Segment-wise pathname normalisation before pattern matching (decode + collapse).
basePath stripping at a segment boundary before matching config header sources.
Applying matched config headers via applyConfigHeadersToResponse without overwriting middleware-set headers.

The generated entry is reduced to a single delegation call.

Why

Every response variant (page, route handler, server action, metadata, not-found) passes through handler(). The shared finalisation logic lived in a template string with no direct tests and no reuse path. Per the architecture rule: codegen describes app shape; normal modules implement behaviour.

Bug fixed in the same change: the inline basePath strip used pathname.startsWith(basePath), which is a string-prefix match. /app2/page with basePath=/app would be incorrectly stripped to /2/page, potentially causing wrong config headers to apply. stripBasePath() enforces a path-separator boundary.

Refs #1020.

Approach

New module imports from existing typed helpers only:

applyConfigHeadersToResponse from server/request-pipeline.ts (source)
stripBasePath from utils/base-path.ts for segment-boundary correctness
normalizePath + normalizePathnameForRouteMatch for pathname normalisation

Generated entry drops two imports (applyConfigHeadersToResponse, normalizePathnameForRouteMatch) that are now internal to the helper.

Validation

New test file tests/app-rsc-response-finalizer.test.ts — 10 tests covering:

Header applied to 200 response
configHeaders: [] is a no-op (returns same response object)
Source pattern miss leaves response untouched
Immutable 307 (Response.redirect()) does not throw
Mutable 308 response does not get config headers applied
basePath stripped before source matching
basePath segment-boundary correctness (regression for the startsWith bug)
Nested basePath stripped correctly
has-condition match applies header
has-condition miss skips header

Entry template snapshot updated automatically by pre-commit hook. vp check clean.

Risks / follow-ups

mergeMiddlewareResponseHeaders in the plain-text 404 path inside _handleRequest is not yet moved; that path is simpler and lower risk. Can be a follow-up if desired.
The module currently has one caller (the generated App Router entry). The Pages Router production server (server/prod-server.ts) has its own header finalisation path that is separately out of scope for this PR.

_{🔄 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/1035 **Author:** [@NathanDrake2406](https://github.com/NathanDrake2406) **Created:** 5/3/2026 **Status:** ✅ Merged **Merged:** 5/3/2026 **Merged by:** [@james-elicx](https://github.com/james-elicx) **Base:** `main` ← **Head:** `refactor/app-rsc-response-finalizer` --- ### 📝 Commits (2) - [`7037c10`](https://github.com/cloudflare/vinext/commit/7037c10fc3c6929bd4680c4ede91df63091ea5b9) refactor(app-rsc): introduce app-rsc-request-normalization module - [`d5e9b24`](https://github.com/cloudflare/vinext/commit/d5e9b244ba6e5ed009dc5c8b6325871a5dd29448) refactor(app-rsc): extract response finalisation into typed helper ### 📊 Changes **4 files changed** (+279 additions, -22 deletions) <details> <summary>View changed files</summary> 📝 `packages/vinext/src/entries/app-rsc-entry.ts` (+12 -21) ➕ `packages/vinext/src/server/app-rsc-response-finalizer.ts` (+70 -0) 📝 `tests/app-router.test.ts` (+1 -1) ➕ `tests/app-rsc-response-finalizer.test.ts` (+196 -0) </details> ### 📄 Description Based on #1034 ## What this changes Extracts the App Router response finalisation step from an inline template string in the generated `handler()` function into a dedicated typed module: `server/app-rsc-response-finalizer.ts`. The extracted module owns: - Skipping 3xx redirect responses. [`Response.redirect()`](https://developer.mozilla.org/en-US/docs/Web/API/Response/redirect_static) creates immutable `Headers` that throw `TypeError: Cannot modify immutable headers` on write. [Next.js also excludes config headers from redirects](https://github.com/vercel/next.js/blob/canary/packages/next/src/server/app-render/app-render.tsx). - Segment-wise pathname normalisation before pattern matching (decode + collapse). - basePath stripping at a **segment boundary** before matching config header sources. - Applying matched config headers via `applyConfigHeadersToResponse` without overwriting middleware-set headers. The generated entry is reduced to a single delegation call. ## Why Every response variant (page, route handler, server action, metadata, not-found) passes through `handler()`. The shared finalisation logic lived in a template string with no direct tests and no reuse path. Per the architecture rule: codegen describes app shape; normal modules implement behaviour. **Bug fixed in the same change:** the inline basePath strip used `pathname.startsWith(basePath)`, which is a string-prefix match. `/app2/page` with `basePath=/app` would be incorrectly stripped to `/2/page`, potentially causing wrong config headers to apply. `stripBasePath()` enforces a path-separator boundary. Refs #1020. ## Approach New module imports from existing typed helpers only: - `applyConfigHeadersToResponse` from `server/request-pipeline.ts` ([source](https://github.com/NathanDrake2406/vinext/blob/main/packages/vinext/src/server/request-pipeline.ts#L166)) - `stripBasePath` from `utils/base-path.ts` for segment-boundary correctness - `normalizePath` + `normalizePathnameForRouteMatch` for pathname normalisation Generated entry drops two imports (`applyConfigHeadersToResponse`, `normalizePathnameForRouteMatch`) that are now internal to the helper. ## Validation New test file `tests/app-rsc-response-finalizer.test.ts` — 10 tests covering: - Header applied to 200 response - `configHeaders: []` is a no-op (returns same response object) - Source pattern miss leaves response untouched - Immutable 307 (`Response.redirect()`) does not throw - Mutable 308 response does not get config headers applied - basePath stripped before source matching - basePath segment-boundary correctness (regression for the `startsWith` bug) - Nested basePath stripped correctly - `has`-condition match applies header - `has`-condition miss skips header Entry template snapshot updated automatically by pre-commit hook. `vp check` clean. ## Risks / follow-ups - `mergeMiddlewareResponseHeaders` in the plain-text 404 path inside `_handleRequest` is not yet moved; that path is simpler and lower risk. Can be a follow-up if desired. - The module currently has one caller (the generated App Router entry). The Pages Router production server (`server/prod-server.ts`) has its own header finalisation path that is separately out of scope for this PR. --- <sub>🔄 This issue represents a GitHub Pull Request. It cannot be merged through Gitea due to API limitations.</sub>