hyper-express
hyper-express copied to clipboard
kartikk221
Improve Type Customization
This PR is a proof of concept implementing some ideas from #131 to ease hyper-express usage with Typescript.
All I did was changing the type declarations to allow a bit of customization to benefit from Typescript type checks and autocompletion. You'll find rough examples inside a new markdown page soberly called Typescript.
The customization is inspired by Fastify generics with a simple object having multiple properties : {Body, Params, Locals}. In my Fastify projects I frequently use this handy type customization, I find it really useful to avoid type errors.
If maintainers appreciate the idea, we may also push the customization further and allow Response type customization and so on. I would be happy to discuss further customization and choices about the Generic form.
I love this underrated package btw 🫶
Hey, this looks very interesting. I will review it within some large codebases of mine to test for breaking changes. Do you know about any major / guaranteed breaking changes that will come along with these types? or do these types extend any / are open ended and will not break existing Typescript codebase types?
No breaking changes, all types are based on standard Typescript mecanism/utility types. The only "complex" type I have written is the Pattern type but again, using standard Typescript. Furthermore, as the whole PR is based on generics, one can still uses hyper-express with typescript without typing the request parameters or body for more flexibility, it should be automatically inferred as any.
Hey, apologize been a bit busy with some stuff. Will review this later this week and get merged. From a small project I tried your types in, no errors were thrown by TS. Will try in a few larger projects and then we should be good to go.
No worries. As I stated in my first post, we could further enhanced type customization especially to allow Response type to be set. Shall I try to work on it and open a second PR after this one is merged ?
Of course, feel free to do so. I think your current approach inspired by Fastify generics is probably the best course of action given a lot of Node.js developers prefer Fastify's type safety.
@Tyrenn Looks like your PR is stale. Maybe after my PR ,#207, is merged. You can merge your changes.
@Geo25rey sorry I've been a bit overwhelmed in my life recently, I thought this PR had been merged already but we might misunderstood each other with @kartikk221 when I said I would also improve Response type, I thought afterwards.
Anyway, I might find the time to complete this PR and fix conflicts in the next weeks !
I've finally found time to merge latest changes. Seems to not break anything.
As I mentioned I would long time ago, I've added the possibility to type json Response. This types are still optional and only useful for Typescript usage.
This additional type lies on the "Response" prop of the RouteOption Generic. Fastify uses the "Reply" keyword to avoid confusion with the actual Response object, I don't know if we should follow their footsteps ?
I've also found out that most of response object method signatures return a Response object instead of this. Don't really know if its an issue nor voluntary
I've finally found time to merge latest changes. Seems to not break anything.
As I mentioned I would long time ago, I've added the possibility to type json Response. This types are still optional and only useful for Typescript usage.
This additional type lies on the "Response" prop of the RouteOption Generic. Fastify uses the "Reply" keyword to avoid confusion with the actual Response object, I don't know if we should follow their footsteps ?
I've also found out that most of response object method signatures return a Response object instead of this. Don't really know if its an issue nor voluntary
No worries at all. I have been busy with work as well. Once you are done, I will be sure to test it out with some existing codebases to ensure types don't break. Thanks again!
I think I'm done, only the vocabulary "Reply" / "Response" question remains, do you have a preference ?
I think I'm done, only the vocabulary "Reply" / "Response" question remains, do you have a preference ?
Maybe "ResponseFormat", "ResponseBody", or "ResponseBodyFormat"
@kartikk221 Have you had time to test it on existing codebases ? 😄
Maybe "ResponseFormat", "ResponseBody", or "ResponseBodyFormat"
I think I'll stick with "Response" for now, I found other proposal a bit long
The PR is ready, we just need to wait for @kartikk221 to test with existing codebases