feat(client): Add WebSocket Provider Integration Tests and Enhance WebSocket Initialization

[naporin0624]

Background

I use hono/client and wanted to reconnect websockets. However, the $ws method in hono/client internally creates a new WebSocket, so in order to use a websocket client with reconnection, it was necessary to define the url directly. This would lose the type safety of Hono RPC.

This change allows the use of clients that conform to WebSocket interfaces with reconnection, such as reconnecting-websocket, internally.

example usage

import ReconnectingWebSocket from 'reconnecting-websocket';

const client = hc<AppType>(url, {
  webSocket(url, options) {
    return new ReconnectingWebSocket(url, options)
  },
})
client.index.$ws({ query })

What was done

• Added new test cases for WebSocket provider integration to ensure correct initialization and handling of query parameters.
• Enhanced WebSocket initialization logic to support custom WebSocket implementations via client options.
• Updated hc function to use establishWebSocket method for flexible WebSocket creation.
• Modified ClientRequestOptions to include webSocket for custom WebSocket handling.

How to Test

• Run the new and existing test suite to verify functionality and integration.
• Verify the correct inclusion and handling of query parameters in WebSocket URLs.

The author should do the following, if applicable

• Add tests
• Run tests
• bun run format:fix && bun run lint:fix to format the code
• Add TSDoc/JSDoc to document the code

[codecov]

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 96.21%. Comparing base (1fafc7a) to head (45740cd).
Report is 1 commits behind head on main.

Additional details and impacted files

@@           Coverage Diff            @@
##             main    #3213    +/-   ##
========================================
  Coverage   96.21%   96.21%            
========================================
  Files         151      151            
  Lines       15096    15102     +6     
  Branches     2620     2757   +137     
========================================
+ Hits        14524    14530     +6     
  Misses        572      572            

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

[yusukebe]

Hi @naporin0624 ! Thank you for the PR.

Enhanced WebSocket initialization logic to support custom WebSocket implementations via client options.

Before I review the details, I would like to know the real-world use case of this option. Can you share it?

[naporin0624]

@yusukebe
I didn't explain the background. I apologize.
I use hono/client and wanted to reconnect websockets. However, the $ws method in hono/client internally creates a new WebSocket, so in order to use a websocket client with reconnection, it was necessary to define the url directly. This would lose the type safety of Hono RPC.

This change allows the use of clients that conform to WebSocket interfaces with reconnection, such as reconnecting-websocket, internally.

[yusukebe]

@naporin0624

That makes sense. Thanks! I'll review this.

[yusukebe]

Hi @naporin0624

One thing I want to consider. You've added an option for webSocket in two places:

// 1
const client = hc<AppType>(url, {
  webSocket() {}, // <==
})

// 2
const client = hc<AppType>(url)
client.index.$ws(
  undefiend,
  {
    webSocket() {}, // <==
  }
)

It's good, but in my opinion, it's better to have one than two, a few changes as possible. This is because it is easy to add options but difficult to delete existing ones.

If your needs can be satisfied with 1, using only 1 is best. This would simplify the argument checking. What do you think?

[naporin0624]

hi @yusukebe

Thank you for your review.
I specified two types in the same way as fetch, but I think it's not too late to add the implementation of 2 once the need arises. Given the nature of OSS, I understand that it is difficult to delete an API once it has been added, so I will change it to only implement 1, which is necessary this time.

This review has been a very valuable conversation for me. Thank you.

[nakasyou]

Hi @naporin0624, I think it's a nice idea. You have to import WebSocket from ws module if you use old Node.js version, so it's helpful for that.

Then, what do you think about receiving a class, not a function?

const client = hc<AppType>(url, {
  WebSocket: ReconnectingWebSocket
})

Because WebSocket is just class, and ReconnectingWebSocket has an API compatible with WebSocket. I think it more intuitive for WebSocket to receive WebSocket compatible APIs just as fetch receives fetch compatible functions.

[naporin0624]

@nakasyou
Thank you for your review. I also considered taking the class as an option and creating a new one at runtime, but I wanted to avoid limiting the options to classes, so I made it a function-based interface.

I'm also thinking about making it so that it can accept retry like retry(new WebSocket()).

[nakasyou]

@naporin0624 I agree. As you said you can easily change options.

[yusukebe]

LGTM!

[yusukebe]

@naporin0624

Thank you for changing it. This is simpler and looks good. I'll merge it later. This is actually a feat change but is not a so big feature for users, so we can release it in a patch release. Thanks!

[naporin0624]

@yusukebe Okay! Looking forward to the release!

[yusukebe]

Hi @naporin0624

I'll merge this into the main and release a new patch version that includes this change!