kwhitley / itty-router

A little router.
MIT License
1.76k stars 78 forks source link

Automatic Typescript route param parsing #150

Open hmnd opened 1 year ago

hmnd commented 1 year ago

This PR enables automatic type inference from provided route paths, ensuring that users can only access params defined in the route.

I've added type tests to cover these changes. I think they cover all possible eventualities, but would appreciate a second opinion.

Given that this is proposed for a major version upgrade, I did include one semi-breaking change. Instead of having to extend the Router in order to add custom methods, I've added a TMethods type parameter to Router. This allows the user to provide a union of any number of method types they would like (eg. 'puppy' | 'kitty'); all of which are mapped to the Route function type internally. This has the added benefit of ensuring the Route type remains consistent across all routes.

P.S. thanks to this gist and @supabase/postgrest-js for some of the inspiration on how to achieve this :)

kwhitley commented 1 year ago

Awesome, thanks David!

Re. your proposed breaking change, mind including (here) a brief before & after snippet of what implementation would look like compared to what it is?

kwhitley commented 1 year ago

Briefly looking at the example.ts though, I think I def prefer your direction!

hmnd commented 1 year ago

Re. your proposed breaking change, mind including (here) a brief before & after snippet of what implementation would look like compared to what it is?

I think the change to the example shows the difference pretty well.

The previous method is brittle in a couple of ways.

An alternative (possibly nicer?) option is to replace this with a simple Record<string, Route>, allowing users to use custom methods, while retaining niceties like completion for the methods in RouterHints.

Re my current implementation, say you want to support the puppy and kitty methods:

Before

interface CustomRouter extends RouterType {
  puppy: Route;
  kitty: Route;
}

const router = Router({ base: '/' })

router
  .all<CustomRouter>('*', () => {...})
  .puppy('*', () => {...})
  .kitty('*', () => {...});

After

const router = Router<'/', 'puppy' | 'kitty'>({ base: '/' })

router.puppy('*', () => {...})

router.kitty('*', () => {...});
hmnd commented 1 year ago

If you agree, I think it would also be nice to allow providing your own extensions to the IRequest type, at the Router and Route level. itty v2 supported this, but the ts rewrite dropped it, which makes it more cumbersome to use custom properties added on to the request in previous handlers.

When provided, the extended types would replace the GenericTraps that we currently have.

Edit: working on workers project now and the current v3.x paradigm of providing an augmented IRequest within each route handler is really annoying to me.

The particular issue I have is is the properties you wish to add on to IRequest must be optional. That's because request in the RouteHandler type is explicitly typed as IRequest. So if the extended properties aren't marked optional, TS complains that the type doesn't sufficiently overlap with IRequest.

In my current use case, I'm assigning an API client to request in a .all('*') handler before any other routes, so wherever I use the client, I have to tell TS that it definitely exists with request.myClient!.

kwhitley commented 1 year ago

The particular issue I have is is the properties you wish to add on to IRequest must be optional. That's because request in the RouteHandler type is explicitly typed as IRequest. So if the extended properties aren't marked optional, TS complains that the type doesn't sufficiently overlap with IRequest.

In my current use case, I'm assigning an API client to request in a .all('*') handler before any other routes, so wherever I use the client, I have to tell TS that it definitely exists with request.myClient!.

Have a suggestion for solving this one in an elegant way? I totally agree this is a problem, and one I too face all the time... usually forcing me to create a custom downstream request type and case the request as that in the downstream handlers (cumbersome/boilerplatey)

Basically, how do we allow an upstream middleware modify the downstream expected request type effectively?

kwhitley commented 1 year ago

Additionally, I love this: image

Know of an elegant way to allow this to work with withParams? This is a middleware that proxies the request, checking attribute requests off the main request against the params object... meaning name and id could be requested directly off the request, as well as from request.params.

kwhitley commented 1 year ago

Ok, revisiting this one after some time off (and time spent tackling the greater TS issues):

First of all, guaranteed this no longer works with v4.x as it's made some heavy under-the-hood TS changes in the last several days (pending release).

I'd love to see a PR (based on the current types in v4.x) that just tackles the route params, while playing nicely with the typing methods described here: https://itty.dev/itty-router/typescript

If we can get this in without adding too much complexity, or modifying the existing type signatures (described in the docs), that would be amazing!