Open iann838 opened 3 months ago
Hey @iann838
Regarding the Legacy schema parameters:
I think we should keep them, while improving it, we can drop the class behavior and make them just a function like Path and Query is today, doing this would solve the as unknown as ..
Wrapping with z.preprocess(...)
when using with header, path or query is also a very good suggestion as most developers spend a lot of time debugging this issue today when using zod directly
Regarding the Unused handle parameters:
I think we can use the same approach as hono uses today, that is passing the validated data inside the request parameter
this would solve the unused parameters warning, and the data would be accessible as request.data
Regarding the middleware
I like the Depends approach, and this would solve some currently open issues
i'm just unsure where developers using the class based endpoint would define it, would a new property inside the schema
called dependencies
be a good option?
Regarding the Repeated explicit typings: I think we can have a function based definition, as for some use cases, defining a complete class and then having a handle function with 2 or 3 lines is a bit overkill
Currently, we tried to keep the schema property as close as possible to the real openapi definition with just some shiny quality of life improvements, this meant developers how knew openapi already to not need to learn a complete framework from start, and could get up to speed almost without reading the documentation
I'm just unsure if we should go the Fastapi parameters schema approach with almost everything on the root object like you are suggesting for example statusCode
and responseClass
Improved class based endpoints
Another braking change i thought we could do is having the class based endpoint behave like Django class based views
where instead of having the main code inside the handle
function you could change the function name to get
or post
, patch
, etc, and then you didn't need to verify the request.method
inside your code, as we could do that before calling the function.
This could easily be done by having a default handle function the OpenAPIRoute
class, that would do this routing, the means already existing applications didn't need to do anything, as the request always hits the handle function first, and then new endpoints or new application could start using this new function naming.
The main feature for this is that users could inside a single class have a get method to return maybe html with a form, and then the post method to validate the submitted form, and share some code between these.
My question is how would the route registration on the router look like for this, maybe we can just tell developers to register the this kind of endpoints with .all
and then in the default handle function that does the. routing when a given method is request but the function we expect is not defined, the handle function would just not return anything, resulting on the current router to continue calling matching routes like what middlewares do today
@meddulla can you share your thoughts here
@G4brym
views.py
, a given route points to a given view regardless of method or other information./items
, /items/:id
), and GET PUT DELETE PATCH will likely not use the same schemas..as_view()
, which returns a callable roughly like the function-based views.Not to say that class-based views are bad, I use them in production to this date, it's just that it is hard to make it brilliant in JS as the language is not as scriptable as Python (of course, Python is way slower for a reason).
As a long-time Django user (in truth, it was the first web framework I used in production), I have come to dislike it over time. Here are my thoughts after using them for years:
Hi @G4brym, I have some news here: Due to the complexity and effort of refactoring an existing package while being limited, I have decided to write a brand-new package from the ground up instead.
As an open-source enthusiast, of course, it will be open-source and here I have an alpha version of what I wrote: Apertum. Feel free to revise and try it out.
I do not know where the new package will lead, whether it will be widely promoted and used among developers or quietly used by a limited population. Continuous support of the package will also depend on this factor ...
Hey @iann838 yeah, i think that's probably the best way to implement these improvements, itty-router-openapi cannot do much of the changes you are proposing due to backwards compatibility I really like way you made defining endpoints, looks like fastapi for js Share your work with the community on discord they will like it, make sure to advertise it something like fastapi for workers for people to click and go see it 😄 https://discord.com/channels/595317990191398933/996501505936994374
Alright, that will require me finishing the docs first, which is probably tomorrow
This issue is for discussing the refactoring of this project to resolve issues and improve the developer experience. Here will be laid out topics and their potential improvements if any.
Legacy schema parameters
A major part of the developer experience involves type safety and IDE code auto-completion. Legacy parameters were not implemented in a way to support these improvements. In #130, an attempt to type legacy parameters ended up having to force type overwrites using
as unknown as ...
However, coercion in zod is rather tricky,
z.coerce.number()
meansNumber(...)
, an empty parameter will coerce to0
instead of throwing a bad request with "parameter required", things are even worse withz.coerce.boolean()
. My suggestion for fixing this is to preprocess the input with JSON parse only forz.number()
andz.boolean()
as these explicitly require coercion. Two ways to approach this:z.preprocess(...)
when the input type isz.number()
andz.boolean()
used withQuery
,Path
, andHeader
.z.coerce
that wraps JSON parse preprocessing. This approach however requires developers to define them explicitly.Unused
handle
parametersMost of the time due to the existence of the![image](https://github.com/cloudflare/itty-router-openapi/assets/58193352/382d53cd-b233-4377-b8d3-cefc80ab9148)
data
parameter inhandle
, the previous 3 parameters are very likely to be left unused:handle({ req }, data) ...
.Middleware
Currently, middleware functionality is limited, for example, the example provided in: https://cloudflare.github.io/itty-router-openapi/user-guide/middleware/. How many instances would developers like to have a
User
object with the authenticated user info accessible withinhandle
?A popular Python web framework FastAPI provides a solution by forcing middleware to be just "middleware" (like CORS, other general checks, etc.) and approaches authentication and other similar logic using Dependencies.
Repeated explicit typings
Due to how JS is implemented and how the TS compiler works, implicitly (or infer) typing a class method before it is defined is not possible, thus, developers have to explicitly type each of the parameters of the
handle
method after it is supposed to be typed already inschema
, this also applies for theenv
parameter ofhandle
. While writing extra typing may not be a problem, it could become a source of errors because it is explicit and not type-safe.To improve this experience, it is required to make route definitions to be function-based. While I agree that class-based route definition is unique for this project, my argument is that it can still be unique with function-based routes. JS classes are just syntax sugar after all. I have a rather different syntax that I want to propose heavily inspired by FastAPI, although the background logic will mostly remain the same, the surface identify would shift dramatically: