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

feat: add forMiddleware types to Input #2562

Closed
wants to merge 6 commits into from

Conversation

nakasyou
Copy link
Contributor

I added a forMiddleware prop to Input.
In WebSocket helper, helper uses json property for telling data to client, but I think it isn't smart because websocket helper actually doesn't use JSON.

This PR adds properties for various helpers and middleware to tell data to clients.

In creating this PR, I enabled that ToSchema receives Input instead of Input['in'].

Author should do the followings, if applicable

  • Add tests
  • Run tests
  • yarn denoify to generate files for Deno

@yusukebe
Copy link
Member

Hi @nakasyou !

This is an interesting feature. Please let us know another use case, not only the WebSocket helper case.

@nakasyou
Copy link
Contributor Author

@yusukebe

For example, you can create your protocol such as type-safe SSE:

// server
import { typedSSE, sseClient } from 'hono-typed-sse'
import { Hono } from 'hono'
import { hc } from 'hono/client'

const app = new Hono().get('/sse', typedSSE<Data>(...))

// client
const client = hc<typeof app>('/')

const sse = sseClient(client.sse)

In this case, this package can use forMiddleware namespace.

@yusukebe
Copy link
Member

@nakasyou

Cool! I'll use it to know the feeling of using it.

@yusukebe
Copy link
Member

yusukebe commented May 1, 2024

Hi @nakasyou

Sorry for the delay. This is good. We can merge this.

But one thing. I wonder if the naming forMiddleware is not so good because it's not just for middleware; we have to name it with a more general name. My suggestion is stash. There are other words, but stash is proper to mean temporary places to store the types.

Any thoughts?

@nakasyou
Copy link
Contributor Author

nakasyou commented May 1, 2024

Hi @yusukebe, thank you for trying.

But one thing. I wonder if the naming forMiddleware is not so good because it's not just for middleware; we have to name it with a more general name. My suggestion is stash. There are other words, but stash is proper to mean temporary places to store the types.

Any thoughts?

I agreed. I think so. So I changed.

Also, I changed stash from unknown to Record<string, unknown>

@yusukebe yusukebe added the v4.3 label May 1, 2024
@yusukebe
Copy link
Member

yusukebe commented May 2, 2024

@nakasyou

Thank you!

However, we have other very interesting PRs. I'll consider merging this feature with them.

Anyway, can you see #2581 ?

@nakasyou
Copy link
Contributor Author

nakasyou commented May 2, 2024

#2588

@nakasyou nakasyou closed this May 2, 2024
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.

None yet

2 participants