kwhitley / itty-router

A little router.
MIT License
1.71k stars 77 forks source link

Add full TypeScript support & inference #118

Closed tgriesser closed 1 year ago

tgriesser commented 1 year ago

👋 pretty big PR, and a breaking change from the current type definition structure (though no runtime-api changes), so I'll give a summary of the things that make it a worthwhile addition 😄

Auto-inference of parameters & method in a route:

Uses TypeScript's generic capturing + advanced conditionals & mapped properties, in combination with the newer (4.1+) feature of "Template Literal Types" to Infer the route params in the handler (closes #116):

image

Also handles trailing optional params correctly:

image

Types the method name as the string constant literal (GET, POST, ...), rather than string when it's one of the "known" methods

image

Properly allows typing the "rest" args to a handler call:

Without providing generics, we default to [unknown, ExecutionContext]:

image

With generics, we can get the Env:

image

Explicit TS extension via additional Router() generic property:

image

Additional changes & info:

tgriesser commented 1 year ago

Actually just noticed that this library isn't meant to be used exclusively w/ Cloudflare. Let me know if that's still a design goal, and I can try to rework the types a bit.

kwhitley commented 1 year ago

Actually just noticed that this library isn't meant to be used exclusively w/ Cloudflare. Let me know if that's still a design goal, and I can try to rework the types a bit.

Appreciate all the work on this, but you're absolutely correct... it's meant to take a generic request-like object (any object with method + url properties) and route accordingly... while it was designed for my own use with Cloudflare, I absolutely wanted to leave the core lib as generic/vague as possible to allow various downstream solutions.

tgriesser commented 1 year ago

while it was designed for my own use with Cloudflare, I absolutely wanted to leave the core lib as generic/vague as possible to allow various downstream solutions.

Cool, in that case, I stripped the /// <reference types="@cloudflare/workers-types" /> and updated it to make the types a bit more flexible/correct, adding some type-tests to ensure things work with the correct inferred types both in a Cloudflare & generic runtime. I can update the docs a bit if you think this is something you might be interested in landing.

Also if you don't want to overcomplicate things with all the extra type noise, I'd totally respect that and be fine publishing as a different lib. Getting the types right in the library leads to less casting & pain everywhere else, but means there's quite a lot going on to keep things both flexible & correct.

tgriesser commented 1 year ago

Alternatively, I could just extract the new types and replace those in the current itty-router.d.ts, and keep the JS implementation as-is

tgriesser commented 1 year ago

Closing in favor of #119