Skip to content

refactor(request): replace private symbol with static method#4646

Closed
BarryThePenguin wants to merge 2 commits intohonojs:mainfrom
BarryThePenguin:refactor/replace-private-symbol-with-static-method
Closed

refactor(request): replace private symbol with static method#4646
BarryThePenguin wants to merge 2 commits intohonojs:mainfrom
BarryThePenguin:refactor/replace-private-symbol-with-static-method

Conversation

@BarryThePenguin
Copy link
Contributor

Replaces usage of the private GET_MATCH_RESULT symbol with a static matchedRoutes() method

This keeps #matchResult private while still allowing access to matchedRoutes()

@codecov
Copy link

codecov bot commented Jan 24, 2026

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 91.45%. Comparing base (bcc81b1) to head (5d44ff6).
⚠️ Report is 5 commits behind head on main.

Additional details and impacted files
@@           Coverage Diff           @@
##             main    #4646   +/-   ##
=======================================
  Coverage   91.45%   91.45%           
=======================================
  Files         173      172    -1     
  Lines       11322    11318    -4     
  Branches     3279     3279           
=======================================
- Hits        10354    10351    -3     
+ Misses        967      966    -1     
  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.

@yusukebe
Copy link
Member

Hey @BarryThePenguin

Thank you for the PR! One of the purposes of this PR is to fix the build problem, like #4640, right?

I think it is worth accepting, but for details, we have to be careful when updating the HonoRequest, and this change will increase the bundle size very slightly.

@BarryThePenguin
Copy link
Contributor Author

Thank you for the PR! One of the purposes of this PR is to fix the build problem, like #4640, right?

Yes, I was doing some investigating and found this alternative approach. I thought I'd share it to see what others thought.

I think it is worth accepting, but for details, we have to be careful when updating the HonoRequest, and this change will increase the bundle size very slightly.

Yeah, that makes sense. I wasn't able to find out why a symbol was chosen originally. If it works then there's no reason to change it. There's some added type safety using a static method, but it's all internal, so there's no real benefit for users.

@usualoma
Copy link
Member

hi @BarryThePenguin and @yusukebe

Thank you for the suggestion!

import type { Context } from './context' in src/request.ts is not ideal in terms of dependency order.

I haven't fully verified if this resolves the issue, but I think fixing it in the direction suggested in #4651 would be best.

@BarryThePenguin
Copy link
Contributor Author

import type { Context } from './context' in src/request.ts is not ideal in terms of dependency order.

Instead of passing Context as an argument, it could just be a HonoRequest. That would remove the import and any dependency ordering

I haven't fully verified if this resolves the issue, but I think fixing it in the direction suggested in #4651 would be best.

Sounds good 👍🏻

@usualoma
Copy link
Member

Hi @BarryThePenguin,

Thank you for the update!

I apologize for leaving an incomplete comment last time. My thoughts are as follows, so I'd like to maintain the current implementation approach.

  • matchedRoutes is strictly a helper; it should not be utility functionality held by the HonoRequest class.
  • While HonoRequest internally holds #matchResult, this should be hidden, and ideally, no means to access it should be provided.

@yusukebe
Copy link
Member

matchedRoutes is strictly a helper; it should not be utility functionality held by the HonoRequest class.

I agree with this!

@BarryThePenguin
Copy link
Contributor Author

Ok, thank you for the extra details

@BarryThePenguin BarryThePenguin deleted the refactor/replace-private-symbol-with-static-method branch January 26, 2026 21:28
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