Preserve Request headers in csrfFetch - Fixes #49

[muneebs]

When csrfFetch was called with a Request object as the first argument, any headers on the Request were stripped because only init?.headers were read. Merge Request headers, then init headers, then CSRF headers so existing headers survive and CSRF headers always take precedence.

Fixes #49

Summary by CodeRabbit

Bug Fixes

• Fixed header merging in CSRF token injection for Next.js and Nuxt packages. Existing headers from Request objects are now properly preserved while init headers and CSRF headers are correctly applied.

Tests

• Added test coverage for CSRF fetch with Request objects, verifying that custom headers are preserved and init headers properly override original Request headers.

[coderabbitai]

Warning

Rate limit exceeded

@muneebs has exceeded the limit for the number of commits that can be reviewed per hour. Please wait 51 minutes and 0 seconds before requesting another review.

Your organization is not enrolled in usage-based pricing. Contact your admin to enable usage-based pricing to continue reviews beyond the rate limit, or try again in 51 minutes and 0 seconds.

⌛ How to resolve this issue?

After the wait time has elapsed, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

We recommend that you space out your commits to avoid hitting the rate limit.

🚦 How do rate limits work?

CodeRabbit enforces hourly rate limits for each developer per organization.

Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout.

Please see our FAQ for further information.

ℹ️ Review info

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 0eb87a74-9104-4f28-895e-af02dcfe3ab6

📥 Commits

Reviewing files that changed from the base of the PR and between da1888b and 79f4116.

📒 Files selected for processing (2)

• .changeset/fix-csrf-fetch-request-headers.md
• packages/nuxt/test/client.test.ts

Walkthrough

Fixes csrfFetch to preserve headers from Request objects by initializing Headers from the input's existing headers, then merging init headers and CSRF headers. Applied across Next.js and Nuxt client implementations with corresponding tests.

Changes

Cohort / File(s)	Summary
Next.js Client Implementation packages/nextjs/src/client/client.ts	Modified header-merging logic to initialize Headers from input.headers when input is a Request, then apply init headers and CSRF headers in sequence. Ensures headers from Request objects are preserved.
Nuxt Client Implementation packages/nuxt/src/runtime/utils/client.ts	Applied identical header-merging fix to Nuxt's csrfFetch implementation, preserving input Request headers while properly layering init and CSRF headers.
Next.js Test Coverage packages/nextjs/tests/client.test.ts	Added two test cases verifying csrfFetch behavior with Request inputs: (1) headers are preserved without init overrides, and (2) init headers override corresponding request headers while CSRF headers are injected.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~12 minutes

Possibly related PRs

• #22: Both PRs modify the same Next.js client module and adjust csrfFetch header-merging behavior.
• #46: Both PRs address the same csrfFetch client behavior for Request input handling and header preservation.

Poem

🐰 Headers were lost, oh what a fright!
Request objects lost their might—
But now we preserve from the start,
Each header plays its rightful part! 🎉

🚥 Pre-merge checks | ✅ 5

✅ Passed checks (5 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title accurately describes the main change: preserving Request headers in csrfFetch and fixing issue #49.
Linked Issues check	✅ Passed	Changes correctly implement the objective to preserve Request headers when csrfFetch receives a Request object as first argument, merging headers in proper order.
Out of Scope Changes check	✅ Passed	All changes are focused on fixing the header-merging behavior in csrfFetch for both Next.js and Nuxt implementations, with supporting tests.
Docstring Coverage	✅ Passed	Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Commit unit tests in branch fix/csrf-fetch-request-headers

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[coderabbitai]

🧹 Nitpick comments (2)

packages/nuxt/src/runtime/utils/client.ts (1)

70-78: Header merge order is correct; consider adding matching Nuxt tests.

The merge order (Request headers → init headers → CSRF headers) correctly preserves headers on a Request input while letting init override and CSRF take final precedence, matching the behavior and tests added for the Next.js package.

However, the two new Request-based test cases were only added in packages/nextjs/tests/client.test.ts. Since this file has the same behavior change, it'd be worth mirroring those tests here (e.g., under packages/nuxt/tests/…) so the fix is regression-protected in both packages.

Want me to port the two new Request-input test cases from packages/nextjs/tests/client.test.ts into the Nuxt test suite?

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@packages/nuxt/src/runtime/utils/client.ts` around lines 70 - 78, Add
Request-based unit tests in the Nuxt test suite mirroring the two new cases from
packages/nextjs/tests/client.test.ts to ensure the header merge order in
packages/nuxt/src/runtime/utils/client.ts (the Headers construction, the loop
over init?.headers, and the loop over Object.entries(createCsrfHeaders(config)))
is regression-protected; specifically port the tests that construct a Request
input and assert headers preserve Request headers, allow init headers to
override, and ensure CSRF headers (from createCsrfHeaders) take final
precedence, placing them under packages/nuxt/tests with the same assertions and
setup as the Next.js counterparts.

packages/nextjs/src/client/client.ts (1)

152-160: Logic is correct; consider de-duplicating with the Nuxt implementation.

The header construction correctly initializes from input.headers when input is a Request, merges init.headers via set() (so init wins over Request), then applies CSRF headers last (so CSRF wins over both). This is the right precedence for the fix.

Minor observation: this function is now byte-for-byte identical to csrfFetch in packages/nuxt/src/runtime/utils/client.ts (aside from the import.meta.client vs. typeof window guard elsewhere). If there's a shared/core package in this monorepo, extracting csrfFetch (and possibly createCsrfHeaders) there would avoid the two implementations drifting in future fixes like this one.

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@packages/nextjs/src/client/client.ts` around lines 152 - 160, The
header-building logic in client.ts duplicates csrfFetch in Nuxt; extract the
shared behavior into a common utility (e.g., createMergedHeaders or csrfFetch
helper) and reuse it from both packages to prevent drift: move createCsrfHeaders
(if not already core) and the header merge sequence (initializing from Request
headers, then applying init.headers, then CSRF headers with set precedence) into
a shared module, export a function with the same calling surface used by
client.ts and csrfFetch in nuxt (referencing createCsrfHeaders and the
header-merge logic), then replace the inline logic in client.ts and
packages/nuxt/src/runtime/utils/client.ts with calls to the new shared function.

🤖 Prompt for all review comments with AI agents

Verify each finding against the current code and only fix it if needed.

Nitpick comments:
In `@packages/nextjs/src/client/client.ts`:
- Around line 152-160: The header-building logic in client.ts duplicates
csrfFetch in Nuxt; extract the shared behavior into a common utility (e.g.,
createMergedHeaders or csrfFetch helper) and reuse it from both packages to
prevent drift: move createCsrfHeaders (if not already core) and the header merge
sequence (initializing from Request headers, then applying init.headers, then
CSRF headers with set precedence) into a shared module, export a function with
the same calling surface used by client.ts and csrfFetch in nuxt (referencing
createCsrfHeaders and the header-merge logic), then replace the inline logic in
client.ts and packages/nuxt/src/runtime/utils/client.ts with calls to the new
shared function.

In `@packages/nuxt/src/runtime/utils/client.ts`:
- Around line 70-78: Add Request-based unit tests in the Nuxt test suite
mirroring the two new cases from packages/nextjs/tests/client.test.ts to ensure
the header merge order in packages/nuxt/src/runtime/utils/client.ts (the Headers
construction, the loop over init?.headers, and the loop over
Object.entries(createCsrfHeaders(config))) is regression-protected; specifically
port the tests that construct a Request input and assert headers preserve
Request headers, allow init headers to override, and ensure CSRF headers (from
createCsrfHeaders) take final precedence, placing them under packages/nuxt/tests
with the same assertions and setup as the Next.js counterparts.

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: b4f6b721-3419-4cea-b95a-3608fd748a3a

📥 Commits

Reviewing files that changed from the base of the PR and between 0eff730 and da1888b.

📒 Files selected for processing (3)

• packages/nextjs/src/client/client.ts
• packages/nextjs/tests/client.test.ts
• packages/nuxt/src/runtime/utils/client.ts