Closed anbcodes closed 1 year ago
Thank you for your PR.
I don't think that we really need to define the generic on Router
and pass it all the way through, since the Router doesn't really care about Env
.
What I'm thinking is defining the generic only on handle<T = any>()
, which would enable us to have theEnv
type available in our handlers.
public async handle<T = any>(env: T, request: Request, extend: any = {}): Promise<Response> {
// ...
}
I don't think that we really need to define the generic on Router and pass it all the way through, since the Router doesn't really care about Env.
What are the downsides of adding optional type argument for Env
?
I personally find the ability to type Env
very useful. I see the type is optional (with any
as a default), so users who are not interested in typing env, can just omit the type argument.
@urish I think the downside is code complexity. In the code, I had to declare a type argument on around 10 different types in order to get the type variable passed to the router.get
, etc. functions. So if there is a less complex way to do it, that would be preferred.
@tsndr The problem with that is that you can't have type information in the handlers if you only declare it on handle
, at least not in a way that I know of. Say you called handle
twice with two different Env
types, then how would TypeScript know which Env
type the request handler functions should have?
const router = new Router();
router.handle<Env1>([args]);
router.handle<Env2>([args]);
router.get("/", (ctx) => {
// Should ctx.env be Env1 or Env2?
});
Because it would cause problems like the above, I don't think there is a way to express it within TypeScript, but I could be wrong. I don't know everything about TypeScript.
It's true that Router doesn't really care about Env
, but I can't think of another way to do it.
@anbcodes You're making an excellent point.
So, this would be the way of doing it then:
import { Router, RouterHandler } from '@tsndr/cloudflare-worker-router'
export interface Env {
TEST: string
}
const router = new Router<Env>()
// either this
router.get('/test', ({ env }) => {
env.TEST
})
// or this, for splitting into multiple files
const testHandler: RouterHandler<Env> = ({ env }) => {
env.TEST
}
router.get('/test', testHandler)
export default {
async fetch(request: Request, env: Env): Promise<Response> {
return router.handle(env, request)
}
}
Cloudflare's worker example suggests using a type for the environment like this
This PR adds support for this to the
Router
class by allowing this interface to be optionally specified using a type parameter.