Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix(node-ws): allow Hono with custom Env for createNodeWebSocket #951

Merged
merged 2 commits into from
Feb 2, 2025

Conversation

yusukebe
Copy link
Member

@yusukebe yusukebe commented Feb 2, 2025

Fixes #948

Hono<any, any, any> can accept an app with custom Bindings/Variables types. This method is used in such the code:

https://github.com/honojs/hono/blob/093d3fd56ea4411aab305984dbc29e24ac3ac528/src/client/client.ts#L122

The author should do the following, if applicable

  • Add tests
  • Run tests
  • yarn changeset at the top of this repo and push the changeset
  • Follow the contribution guide

Copy link

changeset-bot bot commented Feb 2, 2025

🦋 Changeset detected

Latest commit: a1cf980

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 1 package
Name Type
@hono/node-ws Patch

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

@yusukebe yusukebe changed the title fix(node-ws): allow Hono with custom Env fix(node-ws): allow Hono with custom Env for createNodeWebSocket Feb 2, 2025
@yusukebe
Copy link
Member Author

yusukebe commented Feb 2, 2025

Hey @nakasyou !

Can you review this?

@nakasyou
Copy link
Contributor

nakasyou commented Feb 2, 2025

It looks good!
Was it created for making c.env typed?

@yusukebe
Copy link
Member Author

yusukebe commented Feb 2, 2025

@nakasyou Thanks!

Was it created for making c.env typed?

No. It's for just removing the type error. We may add the Bindigns/Variables type to the first arg c of upgradeWebSocket, but the type definition will be so complex. So we don't do that now.

@yusukebe
Copy link
Member Author

yusukebe commented Feb 2, 2025

Anyway, I'll merge this.

@yusukebe yusukebe merged commit c80ffbf into main Feb 2, 2025
1 check passed
@yusukebe yusukebe deleted the fix/allow-hono-variables-bindings branch February 2, 2025 12:25
@github-actions github-actions bot mentioned this pull request Feb 2, 2025
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.

Websockets helper not accepting a OpenApiHono type
2 participants