[PR #1086] [MERGED] refactor(routing+server): unify route-match preamble and middleware header merge #1081

New issue

Closed

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

BreizhHardware commented

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

Owner

📋 Pull Request Information

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

Base: main ← Head: refactor/unify-pages-app-shared-utilities

📝 Commits (1)

040f103 refactor(routing): unify route-match preamble across pages and app routers

📊 Changes

3 files changed (+65 additions, -41 deletions)

View changed files

📝 packages/vinext/src/routing/app-router.ts (+3 -20)
📝 packages/vinext/src/routing/pages-router.ts (+4 -21)
➕ packages/vinext/src/routing/route-matching.ts (+58 -0)

📄 Description

Summary

Part A — route-matching preamble (shipped)

Both Pages Router (matchRoute in routing/pages-router.ts) and App Router (matchAppRoute in routing/app-router.ts) had nearly identical preambles around trieMatch: strip query, trailing-slash normalize, run normalizePathnameForRouteMatch, split into url parts, then look up via a per-routes-array WeakMap trie cache. Each side also had its own getOrBuildTrie helper.

Extracted into a new routing/route-matching.ts:

matchRouteWithTrie<R extends { patternParts: string[] }>(url, routes, cache) — generic over the route shape; both Route (Pages) and AppRoute satisfy the constraint.
createRouteTrieCache<R>() — each router still owns its own cache instance, so different route shapes don't share a slot.

matchRoute and matchAppRoute are now thin wrappers; the public API is unchanged.

Part B — pages middleware header merge (deferred)

Originally planned to migrate applyGsspHeaders (in server/pages-page-response.ts) to the shared mergeMiddlewareResponseHeaders helper. After reading both sides carefully, the semantics are non-trivially divergent:

applyGsspHeaders reads from PagesGsspResponse.getHeaders(), which returns Node-style OutgoingHttpHeaders (Record<string, string | number | boolean | string[]>). The shared helper iterates a Web Headers object.
applyGsspHeaders joins non-cookie string[] values with ", " and uses set(). The shared helper has no such path.
applyGsspHeaders does not Vary-merge (it would , -join + set if Vary ever arrived as an array, which collapses RFC 9110 * semantics differently from mergeVaryHeader).
applyGsspHeaders only appends Set-Cookie when the value is an array; a singular string falls through to set(). The shared helper always appends.
It also forces Content-Type: text/html and returns statusCode.

Collapsing those into the shared helper without breaking subtle production semantics needs either a Node-headers adapter or a meaningful API extension. Per the conservative-parity directive, deferring as a separate PR rather than risking silent behavior change here.

Pure refactor; no behavior change. Follow-up to #1058.

Files

packages/vinext/src/routing/route-matching.ts (new) — generic matchRouteWithTrie + createRouteTrieCache.
packages/vinext/src/routing/pages-router.ts — matchRoute delegates to shared helper.
packages/vinext/src/routing/app-router.ts — matchAppRoute delegates to shared helper.

Test plan

pnpm vp test run tests/app-router.test.ts tests/routing.test.ts tests/route-trie.test.ts — 481 passed.
pnpm vp test run tests/pages-router.test.ts — 200 passed (one pre-existing afterAll teardown timeout in the allowedDevOrigins suite, unrelated to this change; reproduces on main).
pnpm fmt --write clean.
pnpm knip clean.

🤖 Generated with Claude Code

_{🔄 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/1086 **Author:** [@james-elicx](https://github.com/james-elicx) **Created:** 5/5/2026 **Status:** ✅ Merged **Merged:** 5/5/2026 **Merged by:** [@james-elicx](https://github.com/james-elicx) **Base:** `main` ← **Head:** `refactor/unify-pages-app-shared-utilities` --- ### 📝 Commits (1) - [`040f103`](https://github.com/cloudflare/vinext/commit/040f10395accff8bf6b3f3d68360cbb2c5e9feaf) refactor(routing): unify route-match preamble across pages and app routers ### 📊 Changes **3 files changed** (+65 additions, -41 deletions) <details> <summary>View changed files</summary> 📝 `packages/vinext/src/routing/app-router.ts` (+3 -20) 📝 `packages/vinext/src/routing/pages-router.ts` (+4 -21) ➕ `packages/vinext/src/routing/route-matching.ts` (+58 -0) </details> ### 📄 Description ## Summary ### Part A — route-matching preamble (shipped) Both Pages Router (`matchRoute` in `routing/pages-router.ts`) and App Router (`matchAppRoute` in `routing/app-router.ts`) had nearly identical preambles around `trieMatch`: strip query, trailing-slash normalize, run `normalizePathnameForRouteMatch`, split into url parts, then look up via a per-routes-array `WeakMap` trie cache. Each side also had its own `getOrBuildTrie` helper. Extracted into a new `routing/route-matching.ts`: - `matchRouteWithTrie<R extends { patternParts: string[] }>(url, routes, cache)` — generic over the route shape; both `Route` (Pages) and `AppRoute` satisfy the constraint. - `createRouteTrieCache<R>()` — each router still owns its own cache instance, so different route shapes don't share a slot. `matchRoute` and `matchAppRoute` are now thin wrappers; the public API is unchanged. ### Part B — pages middleware header merge (deferred) Originally planned to migrate `applyGsspHeaders` (in `server/pages-page-response.ts`) to the shared `mergeMiddlewareResponseHeaders` helper. After reading both sides carefully, the semantics are non-trivially divergent: - `applyGsspHeaders` reads from `PagesGsspResponse.getHeaders()`, which returns Node-style `OutgoingHttpHeaders` (`Record<string, string | number | boolean | string[]>`). The shared helper iterates a Web `Headers` object. - `applyGsspHeaders` joins non-cookie `string[]` values with `", "` and uses `set()`. The shared helper has no such path. - `applyGsspHeaders` does not Vary-merge (it would `, `-join + `set` if Vary ever arrived as an array, which collapses RFC 9110 `*` semantics differently from `mergeVaryHeader`). - `applyGsspHeaders` only appends Set-Cookie when the value is an array; a singular string falls through to `set()`. The shared helper always appends. - It also forces `Content-Type: text/html` and returns `statusCode`. Collapsing those into the shared helper without breaking subtle production semantics needs either a Node-headers adapter or a meaningful API extension. Per the conservative-parity directive, deferring as a separate PR rather than risking silent behavior change here. Pure refactor; no behavior change. Follow-up to #1058. ## Files - `packages/vinext/src/routing/route-matching.ts` (new) — generic `matchRouteWithTrie` + `createRouteTrieCache`. - `packages/vinext/src/routing/pages-router.ts` — `matchRoute` delegates to shared helper. - `packages/vinext/src/routing/app-router.ts` — `matchAppRoute` delegates to shared helper. ## Test plan - [x] `pnpm vp test run tests/app-router.test.ts tests/routing.test.ts tests/route-trie.test.ts` — 481 passed. - [x] `pnpm vp test run tests/pages-router.test.ts` — 200 passed (one pre-existing `afterAll` teardown timeout in the `allowedDevOrigins` suite, unrelated to this change; reproduces on `main`). - [x] `pnpm fmt --write` clean. - [x] `pnpm knip` clean. 🤖 Generated with [Claude Code](https://claude.com/claude-code) --- <sub>🔄 This issue represents a GitHub Pull Request. It cannot be merged through Gitea due to API limitations.</sub>