fix: bind iterator methods on request headers proxy

[Divkix]

Summary

• bind non-mutating request header proxy members, including Symbol.iterator, to the live Headers instance
• add shim regression coverage for iterator-based headers() access, ported from the Next.js headers adapter tests
• add a live App Router route handler regression that exercises Array.from(headers()), entries(), keys(), values(), and Object.fromEntries()

Testing

• pnpm test tests/shims.test.ts
• pnpm test tests/nextjs-compat/request-apis.test.ts
• pnpm run lint
• pnpm run fmt -- packages/vinext/src/shims/headers.ts tests/shims.test.ts tests/nextjs-compat/request-apis.test.ts tests/fixtures/app-basic/app/nextjs-compat/api/headers-iterate/route.ts

Refs #443

[pkg-pr-new]

Open in StackBlitz

npm i https://pkg.pr.new/vinext@480

commit: dccfba9

[Copilot]

Pull request overview

This PR fixes iterator-based access on the next/headers shim by ensuring request header proxy members (including Symbol.iterator) are bound to the current/live Headers source, and adds regression coverage to prevent iterator reads from breaking in route handlers and shim-level contexts (refs #443).

Changes:

• Bind non-mutating Headers proxy members (including symbol-based iterator access) to the active backing Headers instance.
• Add shim-level tests covering Array.from(headers), entries(), keys(), values(), and Object.fromEntries() behavior before copy-on-write.
• Add an App Router fixture route + compat test to validate iterator-based headers() usage in real route handlers.

Reviewed changes

Copilot reviewed 4 out of 4 changed files in this pull request and generated no comments.

File	Description
packages/vinext/src/shims/headers.ts	Updates the request headers proxy get trap to bind non-mutating members (incl. symbols) to the current backing Headers.
tests/shims.test.ts	Adds regression tests for iterator-based reads on the headers context proxy and on headers() (sync + awaited).
tests/nextjs-compat/request-apis.test.ts	Adds an integration/compat test verifying iterator-based headers() access in a route handler.
tests/fixtures/app-basic/app/nextjs-compat/api/headers-iterate/route.ts	Introduces a route handler fixture that exercises iterator-based headers() APIs and returns results for assertions.

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[james-elicx]

/bigbonk review

[james-elicx]

Thanks

[ask-bonk]

Clean fix with thorough test coverage. The root cause and fix are both correct.

Analysis

The old code early-returned symbol properties via Reflect.get(src, prop, receiver), where receiver was the Proxy itself. Native Headers methods like Symbol.iterator, entries(), keys(), and values() need this to be the actual Headers instance — passing the proxy as receiver causes the native method's implicit this binding to be wrong, breaking iteration.

The fix removes the early-return branch for symbols and lets them fall through to the existing Reflect.get(src, prop, src) + .bind(src) path, which correctly binds methods to the live Headers instance. This is consistent with how _sealHeaders (line 343) already handles the same scenario.

Test coverage

All three layers are tested:

1. Unit (shims.test.ts): headersContextFromRequest proxy iteration, plus headers() sync/awaited iteration through runWithHeadersContext
2. Integration (request-apis.test.ts): Live route handler exercising Array.from(), entries(), keys(), values(), Object.fromEntries()
3. Fixture (headers-iterate/route.ts): Both sync and awaited headers() paths

LGTM.

[ask-bonk]

Nit: values isn't filtered by prefix here (unlike keys on line 14), so the integration test asserts with expect.arrayContaining to compensate. This works but makes the values assertion weaker than the others — it would pass even if the custom headers were missing, as long as "alpha" and "beta" appeared in any header value.

Not blocking since the other assertions collectively prove iteration works, but filtering values the same way would make this assertion precise:

Suggested change

	const values = Array.from(awaitedHeaders.values());
	const values = Array.from(awaitedHeaders.entries())
	.filter(([key]) => key.startsWith(PREFIX))
	.map(([, val]) => val);

Then the test expectation could use toEqual(["alpha", "beta"]) instead of arrayContaining.

[ask-bonk]

Review submitted. Here's a summary:

Approved — the fix is correct and well-tested.

Root cause: The old Proxy get trap in headersContextFromRequest early-returned symbol properties with Reflect.get(src, prop, receiver), where receiver was the Proxy. Native Headers methods like Symbol.iterator need this to be the real Headers instance, so iteration via Array.from(headers()), for...of, Object.fromEntries(), etc. broke.

The fix: Removes the symbol early-return branch and lets all property access (string and symbol) flow through the Reflect.get(src, prop, src) + .bind(src) path, which correctly binds methods to the live Headers instance. This is consistent with how _sealHeaders already works.

One minor nit: The values line in the fixture route handler doesn't filter by prefix like the other assertions do, making the integration test's values check slightly weaker (arrayContaining instead of toEqual). Not blocking.

github run