kwhitley / itty-router

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

Request interface overriding already defined interfaces #54

Closed emillime closed 2 years ago

emillime commented 3 years ago

Hi, I'm not sure if this is an issue with itty-router or just how I'm setting things up. But I'm using itty-router in my typescript project. But I can't get VS Code to recognize all the request properties.

In the example below, VS Code will complain about Property 'query' does not exist on type 'Request'.

import { Router } from 'itty-router'

const router = Router()

router.get('/', (request: Request) => {
  console.log('Cf: ', request.cf)
  console.log('Query: ', request.query)
})

addEventListener('fetch', (event: FetchEvent) => {
  event.respondWith(router.handle(event.request))
})

But if I import the request interface from itty-router. It overrides the already defined request types added to the tsconfig from Cloudflare. The below example will complain that Property 'cf' does not exist on type 'Request'..

import { Router, Request } from 'itty-router'

const router = Router()

router.get('/', (request: Request) => {
  console.log('Cf: ', request.cf)
  console.log('Query: ', request.query)
})

addEventListener('fetch', (event: FetchEvent) => {
  event.respondWith(router.handle(event.request))
})

How can I get VS Code to recognize all properties on the request?

irvinebroque commented 2 years ago

@emillime - import { Request as IttyRequest } from "itty-router"; should work.

Agree the naming of Request in this module should probably be named to be more specific, or namespaces should be used.

irvinebroque commented 2 years ago

It does seem like itty-router should be extending Request rather than defining its own interface though?

https://github.com/kwhitley/itty-router/blob/e8e76c9be5496ce2494743fb20dff802c41eb176/src/itty-router.d.ts#L19-L30

...should be something like:

interface IttyRequestExtras {
  params: Obj
  query: Obj
}

export interface IttyRequest extends Request, IttyRequestExtras {}

...and both params and query could be more specific, something like:

interface IttyRequestExtras {
  params: {
    [key: string]: string | undefined;
  };
  query: {
    [key: string]: string | undefined;
  };
}
kwhitley commented 2 years ago

The main reason I didn't want to extend native Request itself is I didn't want any such hard requirement on what's fed into the very agnostic itty. As long as it has the most basic two request-like attributes: method and url, I'll take any old object.

Happy to do whatever I can to support the much more rules-bound TS community, but I try to keep lightweight libs like itty accessible to the lowest common denominator, which means as few constraints as I can get away with.

With that in mind, what do you folks propose as the best path forward for this without adding hard requirements on a native Request (which also would count as a breaking change)?

kwhitley commented 2 years ago

Should be considered closed, as I believe this was addressed with the latest TS changes. Feel free to reopen if that's not the case!