[PR #1072] [MERGED] refactor(shims): dedupe cookie value serialization #1069

New issue

Closed

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

BreizhHardware commented

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

Owner

📋 Pull Request Information

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

Base: main ← Head: refactor/dedupe-cookie-serialization

📝 Commits (2)

8960d16 refactor(shims): dedupe cookie value serialization
2fd7dfe fix(shims): drop unused export of SerializeSetCookieOptions

📊 Changes

3 files changed (+92 additions, -83 deletions)

View changed files

📝 packages/vinext/src/shims/headers.ts (+6 -46)
➕ packages/vinext/src/shims/internal/cookie-serialize.ts (+83 -0)
📝 packages/vinext/src/shims/server.ts (+3 -37)

📄 Description

Summary

Extract the shared Set-Cookie serialization from the next/headers and next/server shims into a single helper at packages/vinext/src/shims/internal/cookie-serialize.ts. Follow-up to #1049.

The two call sites — cookies().set() in headers.ts and ResponseCookies.set() in server.ts — were producing byte-for-byte identical Set-Cookie strings via copy-pasted code (same encodeURIComponent(value), same default Path=/, same attribute order: Path → Domain → Max-Age → Expires → HttpOnly → Secure → SameSite, same validators). Pure refactor — no behavior change.

The third candidate site, RequestCookies._serialize in server.ts:574, builds a request Cookie: header (name=value; name=value, no attributes) — different format, intentionally left alone.

What moved

packages/vinext/src/shims/internal/cookie-serialize.ts (new):

serializeSetCookie(name, value, options) — builds the full Set-Cookie value string
validateCookieName(name) — RFC 6265 §4.1.1 token validator
validateCookieAttributeValue(value, attr) — control-char/semicolon guard

The validators were also duplicated verbatim across both files; consolidating them avoids subtle drift.

Why a separate module instead of importing across shims

server.ts deliberately avoids static-importing headers.ts because of a Vite "use cache" transform issue (existing comment in server.ts:18-30). The new helper lives under internal/ alongside parse-cookie-header.ts, mirroring the existing pattern.

Files changed

packages/vinext/src/shims/internal/cookie-serialize.ts (new, 84 lines)
packages/vinext/src/shims/headers.ts (-43 lines, unchanged behavior)
packages/vinext/src/shims/server.ts (-37 lines, unchanged behavior)

Net: 9 insertions, 79 deletions across the two existing files.

Test plan

pnpm vp test run tests/app-router.test.ts — 308 tests pass
pnpm vp test run tests/pages-router.test.ts — 200 tests pass (one pre-existing afterAll timeout in the unrelated allowedDevOrigins suite, reproduces on main without these changes)
pnpm vp test run tests/shims.test.ts tests/app-route-handler-execution.test.ts tests/app-page-execution.test.ts tests/api-handler.test.ts — 912 tests pass
pnpm tsc --noEmit (in packages/vinext) — clean
pnpm fmt --write on touched files

🤖 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/1072 **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/dedupe-cookie-serialization` --- ### 📝 Commits (2) - [`8960d16`](https://github.com/cloudflare/vinext/commit/8960d1684e0fb00c530194dbd73eaf78e828fcc6) refactor(shims): dedupe cookie value serialization - [`2fd7dfe`](https://github.com/cloudflare/vinext/commit/2fd7dfe87cd031737b7637ee8e29ebf542f634a3) fix(shims): drop unused export of SerializeSetCookieOptions ### 📊 Changes **3 files changed** (+92 additions, -83 deletions) <details> <summary>View changed files</summary> 📝 `packages/vinext/src/shims/headers.ts` (+6 -46) ➕ `packages/vinext/src/shims/internal/cookie-serialize.ts` (+83 -0) 📝 `packages/vinext/src/shims/server.ts` (+3 -37) </details> ### 📄 Description ## Summary Extract the shared Set-Cookie serialization from the next/headers and next/server shims into a single helper at `packages/vinext/src/shims/internal/cookie-serialize.ts`. Follow-up to #1049. The two call sites — `cookies().set()` in `headers.ts` and `ResponseCookies.set()` in `server.ts` — were producing byte-for-byte identical Set-Cookie strings via copy-pasted code (same `encodeURIComponent(value)`, same default `Path=/`, same attribute order: Path → Domain → Max-Age → Expires → HttpOnly → Secure → SameSite, same validators). Pure refactor — no behavior change. The third candidate site, `RequestCookies._serialize` in `server.ts:574`, builds a request `Cookie:` header (`name=value; name=value`, no attributes) — different format, intentionally left alone. ## What moved `packages/vinext/src/shims/internal/cookie-serialize.ts` (new): - `serializeSetCookie(name, value, options)` — builds the full Set-Cookie value string - `validateCookieName(name)` — RFC 6265 §4.1.1 token validator - `validateCookieAttributeValue(value, attr)` — control-char/semicolon guard The validators were also duplicated verbatim across both files; consolidating them avoids subtle drift. ## Why a separate module instead of importing across shims `server.ts` deliberately avoids static-importing `headers.ts` because of a Vite "use cache" transform issue (existing comment in `server.ts:18-30`). The new helper lives under `internal/` alongside `parse-cookie-header.ts`, mirroring the existing pattern. ## Files changed - `packages/vinext/src/shims/internal/cookie-serialize.ts` (new, 84 lines) - `packages/vinext/src/shims/headers.ts` (-43 lines, unchanged behavior) - `packages/vinext/src/shims/server.ts` (-37 lines, unchanged behavior) Net: 9 insertions, 79 deletions across the two existing files. ## Test plan - [x] `pnpm vp test run tests/app-router.test.ts` — 308 tests pass - [x] `pnpm vp test run tests/pages-router.test.ts` — 200 tests pass (one pre-existing `afterAll` timeout in the unrelated `allowedDevOrigins` suite, reproduces on `main` without these changes) - [x] `pnpm vp test run tests/shims.test.ts tests/app-route-handler-execution.test.ts tests/app-page-execution.test.ts tests/api-handler.test.ts` — 912 tests pass - [x] `pnpm tsc --noEmit` (in `packages/vinext`) — clean - [x] `pnpm fmt --write` on touched files 🤖 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>