starred/vinext

Fork 0

mirror of https://github.com/cloudflare/vinext.git synced 2026-05-09 08:25:34 +02:00

[GH-ISSUE #254] Refactor: Extract shared request handling logic from the four server entry points #64

New issue

Closed

opened 2026-05-06 12:36:56 +02:00 by BreizhHardware · 2 comments

BreizhHardware commented

2026-05-06 12:36:56 +02:00

Owner

Originally created by @southpolesteve on GitHub (Mar 5, 2026).
Original GitHub issue: https://github.com/cloudflare/vinext/issues/254

Problem

Request handling logic is duplicated across four server entry points. AGENTS.md warns about this ("When fixing a bug in any of these files, check whether the same bug exists in the others"), but warnings don't prevent drift. The duplication has already caused behavioral divergence in several areas.

The four entry points:

ADS: server/app-dev-server.ts (App Router dev, generates virtual RSC entry)
DS: server/dev-server.ts (Pages Router dev SSR handler)
PS: server/prod-server.ts (Pages + App Router production)
IX: index.ts configureServer middleware (Pages Router dev) + generateServerEntry() (Pages Router prod entry)

There is also config/config-matchers.ts (CM), which was a good start at extraction, but only prod-server.ts uses it. The same logic is independently re-implemented as generated string code in ADS and IX.

Inventory of duplicated logic

16 pieces of shared logic, with copy counts:

#	Logic	Copies	Identical?	Notes
1	Protocol-relative URL guard (`//` rejection)	5	Yes	Trivial but duplicated in ADS, PS (x2), IX, worker entry
2	Trailing slash normalization	3	Mostly	ADS has extra `.rsc` check
3	BasePath stripping	3	Yes
4	Config redirects	3	3 separate implementations	ADS inline, PS via CM, IX local
5	Config rewrites (3-phase)	3	3 separate implementations	Same pattern as redirects
6	Config headers	3	3 separate implementations	Application order differs (behavioral divergence)
7	External URL detection + proxy	3	Web fetch vs Node HTTP API
8	ReDoS-safe regex compilation	3	3 copies of ~90 lines	ADS generated, IX generated, CM TypeScript
9	Cookie parsing	3-4	Yes	ADS, IX generated, CM, DS (specialized)
10	Request context building	2	Yes	ADS generated, CM TypeScript
11	has/missing condition checking	2	Yes	ADS generated, CM TypeScript
12	Config pattern matching	2	Yes	ADS generated, CM TypeScript
13	Middleware execution	4	Most diverged	Different matchers, different result interpretation, different header unpacking
14	CSRF origin validation	1	N/A	Only in ADS (not duplicated, but only exists as generated code)
15	Route matching (matchRoute + matchPattern)	3	Yes	ADS generated, IX generated, DS imports from routing module
16	Image optimization passthrough	3	No	Dev redirects, prod serves files

Highest-risk divergences

Middleware execution (item 13) is the most fragmented. Four implementations with:

Different matcher regex construction (ADS vs IX-generated)
Different result interpretation
Different x-middleware-request-* header unpacking (ADS uses applyMiddlewareRequestHeaders(), PS iterates manually)

Config headers application order (item 6) has actual behavioral divergence: ADS applies headers after the full request (skipping 3xx redirects), PS applies them early, IX applies them before route matching. This means the same next.config.js headers config produces different behavior depending on which server is running.

Config redirects/rewrites (items 4-5) have three independent implementations of the same pattern matching algorithm. config-matchers.ts is the canonical TypeScript version, but ADS and IX each generate their own as escaped string code.

Proposed approach

Phase 1: Make `config-matchers.ts` the single source of truth

config-matchers.ts already has clean TypeScript implementations of: ReDoS-safe regex, cookie parsing, config pattern matching, has/missing conditions, redirect matching, rewrite matching, header matching, external URL detection, external proxy, and request context building.

The generated code in ADS and IX should import from this module instead of inlining copies. Since the generated entries are virtual modules resolved by Vite, they can import from absolute paths. The generated code already does this for other shared modules (e.g., import { setNavigationContext } from "${absPath}").

This eliminates items 4-12 (config redirects, rewrites, headers, external proxy, ReDoS regex, cookie parsing, request context, has/missing conditions, config pattern matching) as duplicated code.

Phase 2: Extract shared request pipeline utilities

Create a new module (e.g., server/request-pipeline.ts) for the common request lifecycle steps:

// server/request-pipeline.ts
export function guardProtocolRelativeUrl(pathname: string): Response | null
export function stripBasePath(pathname: string, basePath: string): string
export function normalizeTrailingSlash(pathname: string, config: { trailingSlash: boolean, basePath: string }): Response | null

These are small functions, but extracting them ensures the trivial logic (items 1-3) stays consistent.

Phase 3: Unify middleware execution

This is the hardest piece. Create a shared middleware runner:

// server/middleware.ts (extend existing)
export interface MiddlewareResult {
  action: 'continue' | 'redirect' | 'rewrite' | 'respond'
  url?: string
  status?: number
  response?: Response
  requestHeaders?: Headers  // x-middleware-request-* unpacked
  responseHeaders?: Headers
}

export async function executeMiddleware(
  request: Request,
  middlewareFn: Function,
  matchers: MiddlewareMatcher[]
): Promise<MiddlewareResult>

All four entry points would call this and interpret the normalized result. The current divergence in matcher regex construction, result interpretation, and header unpacking would be eliminated.

Phase 4: Express the request pipeline ordering as a shared sequence

All servers follow the same sequence: protocol guard, basePath strip, trailing slash redirect, config redirects, beforeFiles rewrites, middleware, afterFiles rewrites, route match, fallback rewrites, config headers. Rather than each server reimplementing this procedurally, the ordering could be expressed as a composable pipeline. This is optional but would prevent future ordering divergences.

Generated code considerations

The ADS and IX-generated entries run as virtual modules in Vite's module graph. They can import from real modules using absolute paths. The pattern already exists:

// In the generated code template:
import { matchRedirect, matchRewrite, matchHeaders } from "${resolve('config/config-matchers.ts')}"
import { executeMiddleware } from "${resolve('server/middleware.ts')}"

This is how the generated entries already import shims like navigation.ts and headers.ts. The same pattern works for shared request handling logic.

Verification

pnpm test (all Vitest tests pass)
pnpm run test:e2e (all Playwright E2E tests pass)
pnpm run typecheck
pnpm run lint
Verify that config headers application order is consistent across all servers (fix the behavioral divergence in item 6)
Verify that middleware matcher regex produces the same results across all entry points

Relationship to other issues

This pairs with #253 (extract template strings into separate modules). That issue is about file organization (moving templates to their own files). This issue is about deduplication (making the templates import shared logic instead of inlining it). They can be done in either order, but doing #253 first makes this one easier because the templates will be in smaller, focused files.

Originally created by @southpolesteve on GitHub (Mar 5, 2026). Original GitHub issue: https://github.com/cloudflare/vinext/issues/254 ## Problem Request handling logic is duplicated across four server entry points. AGENTS.md warns about this ("When fixing a bug in any of these files, check whether the same bug exists in the others"), but warnings don't prevent drift. The duplication has already caused behavioral divergence in several areas. The four entry points: - **ADS**: `server/app-dev-server.ts` (App Router dev, generates virtual RSC entry) - **DS**: `server/dev-server.ts` (Pages Router dev SSR handler) - **PS**: `server/prod-server.ts` (Pages + App Router production) - **IX**: `index.ts` configureServer middleware (Pages Router dev) + `generateServerEntry()` (Pages Router prod entry) There is also `config/config-matchers.ts` (CM), which was a good start at extraction, but only `prod-server.ts` uses it. The same logic is independently re-implemented as generated string code in ADS and IX. ## Inventory of duplicated logic ### 16 pieces of shared logic, with copy counts: | # | Logic | Copies | Identical? | Notes | |---|-------|--------|------------|-------| | 1 | Protocol-relative URL guard (`//` rejection) | 5 | Yes | Trivial but duplicated in ADS, PS (x2), IX, worker entry | | 2 | Trailing slash normalization | 3 | Mostly | ADS has extra `.rsc` check | | 3 | BasePath stripping | 3 | Yes | | | 4 | Config redirects | 3 | **3 separate implementations** | ADS inline, PS via CM, IX local | | 5 | Config rewrites (3-phase) | 3 | **3 separate implementations** | Same pattern as redirects | | 6 | Config headers | 3 | **3 separate implementations** | Application order differs (behavioral divergence) | | 7 | External URL detection + proxy | 3 | Web fetch vs Node HTTP API | | | 8 | ReDoS-safe regex compilation | 3 | **3 copies of ~90 lines** | ADS generated, IX generated, CM TypeScript | | 9 | Cookie parsing | 3-4 | Yes | ADS, IX generated, CM, DS (specialized) | | 10 | Request context building | 2 | Yes | ADS generated, CM TypeScript | | 11 | has/missing condition checking | 2 | Yes | ADS generated, CM TypeScript | | 12 | Config pattern matching | 2 | Yes | ADS generated, CM TypeScript | | 13 | Middleware execution | **4** | **Most diverged** | Different matchers, different result interpretation, different header unpacking | | 14 | CSRF origin validation | 1 | N/A | Only in ADS (not duplicated, but only exists as generated code) | | 15 | Route matching (matchRoute + matchPattern) | 3 | Yes | ADS generated, IX generated, DS imports from routing module | | 16 | Image optimization passthrough | 3 | No | Dev redirects, prod serves files | ### Highest-risk divergences **Middleware execution (item 13)** is the most fragmented. Four implementations with: - Different matcher regex construction (ADS vs IX-generated) - Different result interpretation - Different `x-middleware-request-*` header unpacking (ADS uses `applyMiddlewareRequestHeaders()`, PS iterates manually) **Config headers application order (item 6)** has actual behavioral divergence: ADS applies headers after the full request (skipping 3xx redirects), PS applies them early, IX applies them before route matching. This means the same `next.config.js` headers config produces different behavior depending on which server is running. **Config redirects/rewrites (items 4-5)** have three independent implementations of the same pattern matching algorithm. `config-matchers.ts` is the canonical TypeScript version, but ADS and IX each generate their own as escaped string code. ## Proposed approach ### Phase 1: Make `config-matchers.ts` the single source of truth `config-matchers.ts` already has clean TypeScript implementations of: ReDoS-safe regex, cookie parsing, config pattern matching, has/missing conditions, redirect matching, rewrite matching, header matching, external URL detection, external proxy, and request context building. The generated code in ADS and IX should `import` from this module instead of inlining copies. Since the generated entries are virtual modules resolved by Vite, they can import from absolute paths. The generated code already does this for other shared modules (e.g., `import { setNavigationContext } from "${absPath}"`). This eliminates items 4-12 (config redirects, rewrites, headers, external proxy, ReDoS regex, cookie parsing, request context, has/missing conditions, config pattern matching) as duplicated code. ### Phase 2: Extract shared request pipeline utilities Create a new module (e.g., `server/request-pipeline.ts`) for the common request lifecycle steps: ```typescript // server/request-pipeline.ts export function guardProtocolRelativeUrl(pathname: string): Response | null export function stripBasePath(pathname: string, basePath: string): string export function normalizeTrailingSlash(pathname: string, config: { trailingSlash: boolean, basePath: string }): Response | null ``` These are small functions, but extracting them ensures the trivial logic (items 1-3) stays consistent. ### Phase 3: Unify middleware execution This is the hardest piece. Create a shared middleware runner: ```typescript // server/middleware.ts (extend existing) export interface MiddlewareResult { action: 'continue' | 'redirect' | 'rewrite' | 'respond' url?: string status?: number response?: Response requestHeaders?: Headers // x-middleware-request-* unpacked responseHeaders?: Headers } export async function executeMiddleware( request: Request, middlewareFn: Function, matchers: MiddlewareMatcher[] ): Promise<MiddlewareResult> ``` All four entry points would call this and interpret the normalized result. The current divergence in matcher regex construction, result interpretation, and header unpacking would be eliminated. ### Phase 4: Express the request pipeline ordering as a shared sequence All servers follow the same sequence: protocol guard, basePath strip, trailing slash redirect, config redirects, beforeFiles rewrites, middleware, afterFiles rewrites, route match, fallback rewrites, config headers. Rather than each server reimplementing this procedurally, the ordering could be expressed as a composable pipeline. This is optional but would prevent future ordering divergences. ### Generated code considerations The ADS and IX-generated entries run as virtual modules in Vite's module graph. They can import from real modules using absolute paths. The pattern already exists: ```typescript // In the generated code template: import { matchRedirect, matchRewrite, matchHeaders } from "${resolve('config/config-matchers.ts')}" import { executeMiddleware } from "${resolve('server/middleware.ts')}" ``` This is how the generated entries already import shims like `navigation.ts` and `headers.ts`. The same pattern works for shared request handling logic. ## Verification - `pnpm test` (all Vitest tests pass) - `pnpm run test:e2e` (all Playwright E2E tests pass) - `pnpm run typecheck` - `pnpm run lint` - Verify that config headers application order is consistent across all servers (fix the behavioral divergence in item 6) - Verify that middleware matcher regex produces the same results across all entry points ## Relationship to other issues This pairs with #253 (extract template strings into separate modules). That issue is about file organization (moving templates to their own files). This issue is about deduplication (making the templates import shared logic instead of inlining it). They can be done in either order, but doing #253 first makes this one easier because the templates will be in smaller, focused files.

BreizhHardware closed this issue

2026-05-06 12:36:57 +02:00

BreizhHardware commented

2026-05-06 12:36:57 +02:00

Author

Owner

@southpolesteve commented on GitHub (Mar 20, 2026):

Reopening. The dynamic detection bug in PR #610 (pipeline stages contaminating handler dynamic usage flags) is a direct consequence of the duplicated request handling logic described in this issue. A shared request handler module would have one place for dynamic scoping instead of repeating it in every entry point.

@southpolesteve commented on GitHub (Mar 20, 2026): Reopening. The dynamic detection bug in PR #610 (pipeline stages contaminating handler dynamic usage flags) is a direct consequence of the duplicated request handling logic described in this issue. A shared request handler module would have one place for dynamic scoping instead of repeating it in every entry point.

BreizhHardware commented

2026-05-06 12:36:58 +02:00

Author

Owner

@southpolesteve commented on GitHub (Mar 20, 2026):

Didn't mean to reopen the PR. Meant to reopen the issue https://github.com/cloudflare/vinext/issues/253

@southpolesteve commented on GitHub (Mar 20, 2026): Didn't mean to reopen the PR. Meant to reopen the issue https://github.com/cloudflare/vinext/issues/253

BreizhHardware referenced this issue

2026-05-06 12:38:57 +02:00

[PR #64] [CLOSED] feat: Core Routing Optimization and Windows Compatibility Improvements #282