Skip to content

fix(utils/url): preserve encoded duplicate query order - #5495

Merged
yusukebe merged 3 commits into
honojs:mainfrom
arloives:fix/query-encoded-key-order
Oct 25, 2026
Merged

yusukebe merged 3 commits into
honojs:mainfrom
arloives:fix/query-encoded-key-order

Conversation

@arloives

@arloives arloives commented Oct 1, 2026

Copy link
Copy Markdown
Contributor

fixes req.query(key) skipping an earlier encoded duplicate, with regression coverage

@arloives
arloives marked this pull request as ready for review October 1, 2026 19:46
@codecov

codecov Bot commented Oct 2, 2026 •

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 85.71429% with 2 lines in your changes missing coverage. Please review.
✅ Project coverage is 80.31%. Comparing base (6abd35b) to head (f34e90f).
⚠ïļ Report is 29 commits behind head on main.

Files with missing lines Patch % Lines
src/utils/url.ts 85.71% 2 Missing ⚠ïļ
Additional details and impacted files
@@            Coverage Diff             @@
##             main    #5495      +/-   ##
==========================================
+ Coverage   80.24%   80.31%   +0.06%     
==========================================
  Files         148      149       +1     
  Lines       10472    10529      +57     
  Branches     2336     2358      +22     
==========================================
+ Hits         8403     8456      +53     
- Misses       2069     2073       +4     

☔ View full report in Codecov by Harness.
ðŸ“Ē 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.

Added tests for query parameter handling in Hono app.
@usualoma

Copy link
Copy Markdown
Member

I explored a few alternative approaches from a performance perspective, but this PR seems to strike a good balance for the fast path and queries with relatively few parameters. I think we should merge it.

@yusukebe yusukebe left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM!

@yusukebe

Copy link
Copy Markdown
Member

@arloives Thanks!

@yusukebe
yusukebe merged commit eb78eca into honojs:main Oct 25, 2026
19 checks passed
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.

3 participants