kwhitley / itty-router

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

Refactor: inspired by taralx #41

Closed kwhitley closed 3 years ago

kwhitley commented 3 years ago

So yarn verify showed an estimated gzip size of 420 bytes before AND after the @taralx mod (once I added in the updated regex to pass the latest tests, dot-escaping, etc). Note these figures aren't particularly accurate when compared to bundlephobia, but at least gives us a comparison point before/after changes to the codebase.

By leaving the original proxy syntax, but moving routes to an outer default variable as he's done, and leveraging his entire reorganized interior to handle, we get it down to 400 bytes while passing all tests.

*NOTE: I'm leaving this PR as a reference point for future discussion only, as I'd prefer he make the real PR - he def deserves the contrib credit for his ideas! ❤️

Negative Side Effects

There's an undocumented "feature" that moving the routes to a locally-scoped variable kills off, and that's the previous ability to "sniff" the routes for regex debugging, as they were stored via mutation within the original options object (for better or worse). This allowed you to do this:

const options = {}
const router = Router(options)

router
  .get('/foo/bar', handler1)
  .get('/foo/bar/baz', handler2) 

console.log(options)

and see something like this (different router I nabbed for ease) image

This ability would definitely be lost here. Useful for debugging tests for sure, but the mutation/injection was definitely a dirty trick... thoughts @mvasigh @taralx?

kwhitley commented 3 years ago

Which is to say... if there was a[n alternative] way we could allow capturing/displaying the routes from a router without adding in a ton of characters... that would be awesome :D

kwhitley commented 3 years ago

Conversely, we can go back to the original injection style, and with his changes to the handle method alone, we get down to 405 bytes, while leaving the ability to see/capture routes.

const Router = (options = {}) =>
  new Proxy(options, {
    get: (obj, prop, receiver) => prop === 'handle'
      ? async (request, ...args) => {
          let response, match,
            url = new URL(request.url)
          request.query = Object.fromEntries(url.searchParams)
          for (let [route, handlers, method] of obj.routes) {
            if ((method === request.method || method === 'ALL') && (match = url.pathname.match(route))) {
              request.params = match.groups
              for (let handler of handlers) {
                if ((response = await handler(request.proxy || request, ...args)) !== undefined) return response
              }
            }
          }
        }
      : (route, ...handlers) =>
          (obj.routes = obj.routes || []).push([
            `^${((obj.base || '') + route)
              .replace(/(\/?)\*/g, '($1.*)?')
              .replace(/\/$/, '')
              .replace(/:(\w+|\()(\?)?(\.)?/g, '$2(?<$1>[^/$3]+)$2$3')
              .replace(/\.(?=[\w(])/, '\\.')
            }\/*$`,
            handlers,
            prop.toUpperCase(),
          ]) && receiver
  })
taralx commented 3 years ago

Oh, introspection is cheap and clever. I send a PR.

taralx commented 3 years ago

(I originally had full introspection, but then I ran into the requirement that handle be auto-bound.)

kwhitley commented 3 years ago

Closing this in favor of the final consolidated PR...