kwhitley / itty-router

A little router.
MIT License
1.81k stars 79 forks source link

Example Multi-route (Upstream) Middleware - Typescript #111

Closed calderonth closed 1 year ago

calderonth commented 2 years ago

The documentation shows that you can do this in plain JS to support multi-route upstream middleware:

// middleware that injects a user, but doesn't return
const withUser = request => {
  request.user = { name: 'Mittens', age: 3 }
}

router
  .get('*', withUser) // embeds user before all other matching routes
  .get('/user', request => new Response(`Hello, ${user.name}!`))

router.handle({ method: 'GET', url: 'https://example.com/user' })
// STATUS 200: Hello, Mittens!

However, I don't believe this is typescript compliant and I have been trying to find an elegant way to do this. Is there something that would be recommended?

Thanks

jacwright commented 2 years ago

https://github.com/kwhitley/itty-router/pull/99 should make this easier, however, until it is added you can do this:

interface MyRequest extends Request {
  user: {name: string, age: number};
}

// middleware that injects a user, but doesn't return
const withUser = request => {
  request.user = { name: 'Mittens', age: 3 }
}

router
  .get('*', withUser) // embeds user before all other matching routes
  .get<MyRequest>('/user', request => new Response(`Hello, ${user.name}!`))

router.handle({ method: 'GET', url: 'https://example.com/user' })
// STATUS 200: Hello, Mittens!

You have to add <MyRequest> to each route, but once #99 is added, you can add it once to the router when creating it.

kwhitley commented 2 years ago

I need to do some testing on #99 to follow these changes... if it flies, it'll get rolled into the 3.0 release

kwhitley commented 2 years ago

However, at a glance, I'm concerned that while it adds some convenience, it's not natively intuitive (in that you're declaring at the Router level the type of Request that handlers themselves will be handling), and really unless you're nesting routers, wouldn't the Router basically always be defined with Request, rather than a custom type? At this point, no middleware will have likely diverted it to a custom Request type... and the moment one does, you would still need to declare those on a per-route basis.

Thoughts?

kwhitley commented 1 year ago

This coverage has been added in version 3, with documentation. Let's re-open if concerns are still valid after testing 3.x!