Solving the withParams global middleware issue

[kwhitley]

Currently a withParams middleware that targets request.params cannot be injected upstream as global middleware, because at this point, params would have not been set (to be spread into the request).

import { Router } from 'itty-router'
import { withParams } from 'itty-router-extras'

const router = Router()

// currently works
router
  .get('/:id', withParams, ({ id }) => new Response(id))

// currently fails
router
  .all('*', withParams)
  .get('/:id', ({ id }) => new Response(id))

Proposed Solution

1. Pass request.proxy || request through to handlers... middleware can inject a proxy (if needed) to trigger this. This costs about ~5 bytes.
2. Middleware can create a Proxy around the request on request.proxy and effectively watch the request (for loggers and such), or modify how they read things (e.g. withParams intercepts "gets" to the request proxy and find the values in the request.params)

After (Line 12)

This allows a middleware upstream to watch any change on the request itself, including (but not limited to) request.params, and fire appropriately. It also allows each middleware to proxy the existing proxy, meaning multiple middleware can watch independent request props without worrying about collisions.

With this, an upstream withParams could be defined as follows, then downstream changes to params would trigger the proxy (assigning params into the request).

[kwhitley]

In order to intercept changes to the request, all "observable" changes would need to modify the request through the request.proxy path. For example: request.proxy.foo ='bar' instead of request.foo = 'bar'.

Conversely, we could consider passing request.proxy to downstream handlers in place of the bare Request... meaning anything that tries to modify the request itself would be going through the proxy (and thus interceptable) anyway...

[kwhitley]

Other alternative that @mvasigh proposed (if I understood correctly), is leaving core itty alone, and taking advantage that itty proxies the options... allowing us to add a proxy layer to literally anything in the router flow (including the request)...

If he pulls this off (or I beat him to it), this could mean a composable ProxiedRouter(ThrowableRouter()) scenario that requires no changes to the itty core, AND would allow for direct changes to the request to fire proxies interceptors :)

🤔

[mvasigh]

This doesn't feel great, obviously, but I can get the following test to pass with no changes to the source and no wrapping routers:

    fit('lets me hack it with prototype and proxy magic', async () => {
      const router = Router()

      function withParams(req) {
        // We need to delete the `params` key so we can reset it after we 
        // wrap our object's prototype
        const _temp_params = req.params;
        delete req.params;

        // Then, let's create a container object and use our object's prototype
        // chain so it is preserved
        const container = Object.create(Object.getPrototypeOf(req));

        // Let's also use an object that will store our params
        let params = {};

        // Wrap our request object with a proxied container object
        Object.setPrototypeOf(req, new Proxy(container, {

          set(target, key, val) {
            // Intercept any sets on `params` and spread into our params object
            if (key === 'params') {
              params = {...params, ...val}
            }
            Reflect.set(container, key, val);
            return true;
          },

          get(target, key) {
            // If the requested property exists in our params object, serve it up!
            if (params.hasOwnProperty(key)) {
              return params[key];
            }
            return Reflect.get(target, key);
          },

        }))

        req.params = _temp_params;
      }

      router.all('*', withParams).get('/:id', ({ id }) => id)

      const res = await router.handle(buildRequest({ path: '/abcd' }))

      expect(res).toBe('abcd')
    })

It depends on how far you want to take it! But the idea is basically to grab the reference to req in your middleware and set its prototype to a proxied container object. Relevant Stackoverflow

[kwhitley]

🤯 I still can't believe you came up with this so fast...

[kwhitley]

Where we've left it for the night... by simply passing request.proxy || request into the handle() calls, we don't have to explicitly set request.proxy upfront in itty, but simply honor downstream injections of it. This does two things:

1. Keeps the core code short and sweet.
2. Allows downstream proxying of the request, by simply adding a request.proxy = new Proxy(request, { whatever }).

This is important, because it can not only allow things like withParams to modify the get into the request [proxy] that the handlers see, but things can actually watch for future changes by modifying the set path. This could be useful for loggers that are set up to log downstream changes to the request, for instance!

Example test case showing some of this power in action:

// mmmmmmmmmmmmm the following utility functions would be relocated to itty-router-extras mmmmmmmm

const notProxy = prop => prop !== 'proxy'

// similar to watch, but takes a predicate function (receives prop, value, and request)
// to determine if fn should fire (with value and request)
const dynamicWatch = (fn, predicate = notProxy) => request => {
  request.proxy = new Proxy(request, {
    set: (obj, prop, value) => {
      obj[prop] = value
      predicate(prop, obj) && fn(value, prop, request)

      return true
    }
  })
}

// 1. this executes a watcher function when the prop changes on request
// 2. curried signature to allow the product to BE middleware, if desired
// (would export this from itty-router-extras)
const watch = (prop, fn) => dynamicWatch(fn, key => key === prop)

// predicate comes last again, to default to all things flowing through this, if not defined,
// rather than the awkward () => true syntax for using everywhere
const dynamicRetrieve = (fn, predicate = notProxy) => request => {
  request.proxy = new Proxy(request, {
    get: (obj, prop) => predicate(prop, obj)
                        ? fn(prop, request)
                        : obj[prop]
  })
}

// 1. this allows for modifying reads from the request
// 2. curried signature to allow the product to BE middleware, if desired
// (would export this from itty-router-extras)
const retrieve = (prop, fn) => dynamicRetrieve(key => key === prop, fn)

// now for the test!
  describe('request.proxy embedding', () => {
    it('allows for upstream request-watching', async () => {
      const router = Router()
      const handler = jest.fn(req => req.id)
      const logger = jest.fn(v => v)
      const simplelogger = jest.fn(user => user.id)
      const dynamicLogger = jest.fn(user => `${user.id}*`)
      const everythingLogger = jest.fn((value, prop) => console.log(prop, 'changed to', value))

      // without a predicate, a dynamic watcher will fire on any request update
      const watchEverything = dynamicWatch(everythingLogger)

      // longhand for watch('user', fn), using the predicate to target props
      const withDynamicWatch = dynamicWatch(
        dynamicLogger,
        prop => prop === 'user',
      )

      // if you need access to the request, you could do it this way
      const withUserTracking = request => {
        request.foo = 'bar' // this is here to see if the watchEverything middleware fires twice
        watch('user', user => logger({ user, url: request.url }))(request)
      }

      // but this is simpler when watching a single prop
      const withSimpleUserTracking = watch('user', simplelogger)

      // similar syntax for retrieving [dynamic] props.  No predicate 2nd param means everything
      // passes through this function.
      const withParams = dynamicRetrieve((prop, request) =>
                                    request.params && request.params[prop]
                                    ? request.params[prop]
                                    : request[prop])

      // just embeds user in the request... other middleware will fire as a result!
      const withUser = request => {
        request.user = { id: '15' }
      }

      router
        .all('*', watchEverything, withUserTracking, withDynamicWatch, withSimpleUserTracking, withUser, withParams)
        .get('/:id', handler)

      await router.handle(buildRequest({ path: '/13' }))

      expect(handler).toHaveReturnedWith('13')
      expect(logger).toHaveReturnedWith({ url: 'https://example.com/13', user: { id: '15' } })
      expect(simplelogger).toHaveReturnedWith('15')
      expect(logger).toHaveBeenCalledTimes(1)
      expect(everythingLogger).toHaveBeenCalledTimes(2)
      expect(dynamicLogger).toHaveReturnedWith('15*')
    })
  })