kwhitley / itty-router

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

COMMUNITY REQUEST: Full conversion of Router function to TS (as part of 3.0 release) #122

Closed kwhitley closed 1 year ago

kwhitley commented 2 years ago

Folks, I'm in the middle of converting the entire library over to TS for the 3.0 release (which bring extras into the core lib for added convenience). That said, I'm a novice (at best) in TS, which can be difficult enough to type on a good day, much less with the ambiguous nature of:

  1. Proxies (with unknown/infinite traps)
  2. Mutating Requests (that don't really have to be a true Request), as you'd experience while being injected with middleware

With that said, I'm straight up struggling to convert the Router function itself to full TS, so I'm going to ask for help from you fine people. I'd love someone to help me rewrite a full itty-router.ts that:

  1. Passes all the existing tests without modification (must preserve the original API)
  2. Remains true to the original concept of "itty"... as in, the code should compress to an absurdly tiny size, and the even in its uncompressed form should be as short/light as possible.
  3. Remains flexible/easy to type while being able to handle as many scenarios as possible. This part doesn't always play nice with the rigidity that TS users seem to expect, but by design, this library was designed to be flexible, specifically avoiding rigid definitions/expectations that may limit its applications.
  4. While this will fall under a major (breaking) revision, it's not critical that types remain identical to pre 3.0. More importantly, I'd like it to be typed in a way that requires (ideally) no/few imported types from the library... as in you can just use your own generics/Request, etc, and it'll still work as intended.
  5. Please fork off of, and PR into the include-extras branch... if someone nails this, I'd much prefer a direct PR so you get full contribution credit in the codebase, rather than just README credits (that I would obv include)!

Thanks in advance for any help!

<3

Current WIP Branch

mhamrah commented 2 years ago

Happy to try and help with this- it looks like you're starting with just the vanilla types in your branch and that will be a WIP branch for a bit?

kwhitley commented 2 years ago

Yep, WIP until we can get this stuff sorted... conversely, I can ditch the lofty TS goals and do a short-term "release as-is" to simply get the non-typed extras into the core (as a start) with a fully typed version to follow...

mhamrah commented 2 years ago

Got it. I’ll look through the code and get my bearings.

matthewrobb commented 1 year ago

@kwhitley I did some playing around with this and ran into issues with typing proxied "anything method" shorthands. The issue with it boils down to not being able to capture the actual method key as a string type dynamically. It could still be supported but I am curious how valuable you find that feature overall because it seems to add a lot of overhead for what you're getting from it.

An alternative would be to support something like:

router.add("method", Pattern | Regex, ...handlers)

We could still support the proxy of arbitrary methods through to that but the generic identity of the string type for the method literal would have to be not present. Again I think it would be wise to drop this feature.

kwhitley commented 1 year ago

Hey Matthew!

A few things:

  1. This is in widespread use now, and thus breaking changes to the API are basically a thing of last resort, or very intentionally done/communicated.
  2. I will always opt for a more simple/readable approach, which I think the original API does a good job of - it piggybacks off some patterns we've learned from Express as well, which is always a good thing for reducing cognitive load on new users. Abstraction layers (like this) exist to take some of the explicit headache and boilerplate out of doing tasks downstream, so in my opinion, the onus is on me/us to solve the puzzle here, rather than cop out with an easy-to-type solution resulting in (arguably) dirtier code in user-land.
  3. We (really @danawoodman) got this working well (and fully typed) recently over in itty-fetcher, so there's light at the end of the tunnel for us! I'll be attempting to backport the bits over here shortly! :)