Skip to content

fix(cookie): return updated cookie values after setCookie/setSignedCookie in same request#4832

Open
Ishiezz wants to merge 1 commit intohonojs:mainfrom
Ishiezz:fix/get-cookie-after-set-cookie
Open

fix(cookie): return updated cookie values after setCookie/setSignedCookie in same request#4832
Ishiezz wants to merge 1 commit intohonojs:mainfrom
Ishiezz:fix/get-cookie-after-set-cookie

Conversation

@Ishiezz
Copy link
Copy Markdown
Contributor

@Ishiezz Ishiezz commented Mar 27, 2026

Summary

This PR fixes an issue where getCookie() and getSignedCookie() would return stale data (from the incoming request headers) even after setCookie() or setSignedCookie() had been called during the current request cycle.

Changes

  • Modified getCookie and getSignedCookie to check the response's Set-Cookie headers for any newly set cookies.
  • Implemented a getSetCookieString helper to extract and format cookies from response headers, ensuring they are correctly merged with request cookies.
  • Response-set cookies now take precedence over request cookies, matching expected behavior in a single request-response cycle.
  • Added comprehensive tests covering:
    • getCookie after setCookie
    • getSignedCookie after setSignedCookie
    • Overriding existing request cookies with new values
    • Merging multiple cookies from both request and response

Verification Results

  • Executed bun run test src/helper/cookie/index.test.ts
  • Result: 36 tests passed (including all new validation cases).

@codecov
Copy link
Copy Markdown

codecov bot commented Mar 27, 2026

Codecov Report

❌ Patch coverage is 85.71429% with 4 lines in your changes missing coverage. Please review.
✅ Project coverage is 92.82%. Comparing base (e1ae0eb) to head (7148b58).

Files with missing lines Patch % Lines
src/helper/cookie/index.ts 85.71% 4 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main    #4832      +/-   ##
==========================================
- Coverage   92.84%   92.82%   -0.03%     
==========================================
  Files         177      177              
  Lines       11643    11663      +20     
  Branches     3469     3472       +3     
==========================================
+ Hits        10810    10826      +16     
- Misses        832      836       +4     
  Partials        1        1              

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@usualoma
Copy link
Copy Markdown
Member

Hi @Ishiezz, thank you for creating the pull request.

This is the issue discussed below.

#4444

Personally, I agree with the opinion below and believe that the current specification (where getCookie and setCookie are unrelated) is better for this issue.

#4444 (comment)

In any case, I think sufficient discussion is necessary before implementation.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants