Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[feat] Objects as middleware #1776

Open
2 tasks done
evert opened this issue Sep 15, 2023 · 6 comments
Open
2 tasks done

[feat] Objects as middleware #1776

evert opened this issue Sep 15, 2023 · 6 comments

Comments

@evert
Copy link

evert commented Sep 15, 2023

Adding support for using objects as middleware

Currently Koa requires middlewares to be functions. To make developing class-based controllers a bit nicer, it would be helpful to also allow objects to be used as middlewares. For example:

const myMiddleware {
  [Symbol.for('koa-middleware-invoke')]: ctx => {

  },
}

koa.use(myMiddleware);

Alternatively, a base class could be used that Koa recognizes via instanceof:

class MyMiddleware extends KoaMiddleware {

  invoke(ctx) {


  }

}

koa.use(new MyMiddleware())

Although I suspect that the symbol approach is closer to today's Javascript meta.

There are of course workarounds possible, but this seems like a pretty low-overhead ergonomic feature. The implementation of .use() can optimize this away:

  use (fn) {
    // This line is new.
    if (Symbol.for('koa-middleware-invoke') in fn) fn = fn[Symbol.for('koa-middleware-invoke')];

    if (typeof fn !== 'function') throw new TypeError('middleware must be a function!')
    debug('use %s', fn._name || fn.name || '-')
    this.middleware.push(fn)
    return this
  }

Happy to provide a PR for this if this is interesting, but thought I would first gauge interest.

Checklist

  • I have searched through GitHub issues for similar issues.
  • I have completely read through the README and documentation.
@siakc
Copy link

siakc commented Dec 22, 2023

Why no just passing the desired method from a class?

@evert
Copy link
Author

evert commented Dec 22, 2023

This feature request is for increasing ergonomics/ DX. Especially when dealing with routes there's a fair amount of repetition.

Of course helper functions are possible or as you say just work around this, but I asked because it will result in more elegant code.

@siakc
Copy link

siakc commented Dec 23, 2023

Can you provide a reference for this pattern you suggest? My understanding is semantically it makes no difference: there is a method which koa would call. But instead of providing that method directly you suggest we give an object and koa will look for that special method by itself. Now I can't see how this would make the code nicer or more developer friendly.

@evert
Copy link
Author

evert commented Dec 23, 2023

A specific example is that I want users to be able to build controllers as such:

class MyController extends ResourceController {

  get(ctx: Context) {
      /* ... */
  }
}

And I would like users to be able to add this controller using this form:

app.use(new MyController())

My feature request would allow me to define the ResourceController baseclass in a way that Koa can pick this up. Not having this feature requires me to either ask users to call something like:

app.use((new MyController())->mw())

Or add a helper function

app.use(mw(MyController))

Because this has to be done for each route/controller, this is kind of an annoying overhead. I went the helper function route for this, below is the full reference of my ResourceController. But having built-in support for this would be nicer.

import { METHODS } from 'node:http';
import { Context, Middleware } from 'koa';
import { MethodNotAllowed, NotImplemented } from '@curveball/http-errors';

/**
 * Takes a ResourceController subclass, instantiates it and returns
 * a koa-compatible middleware.
 */
export function mw(cl: typeof ResourceController): Middleware {

  const controller = new cl();
  return controller.dispatch.bind(controller);

}

/**
 * The ResourceController is a base class for controllers that
 * maps HTTP methods like GET, PUT to class methods like 'get', 'put'.
 */
export class ResourceController {

  constructor() {

    // HEAD fallback. It could be desirable to reimplement HEAD faster, but this is an OK default and just as slow as GET but with fewer bytes sent.
    if ('get' in this && !('head' in this)) {
      (this as any).head = this.get;
    }

  }

  options(ctx: Context) {

    ctx.body = '';
    ctx.set(
      'Allow',
      METHODS.filter(method => method.toLowerCase() in this).join(',')
    );

  }

  dispatch(ctx: Context): void|Promise<void> {

    if (!METHODS.includes(ctx.method)) {
      throw new NotImplemented(`HTTP method not implmented ${ctx.method}`);
    }

    const classMethod = ctx.method.toLowerCase();
    if (typeof (this as any)[classMethod] === 'function') {
      return (this as any)[classMethod](ctx);
    } else {
      throw new MethodNotAllowed(`The ${ctx.method} method is not supported at this endpoint`);
    }

  }

}

@siakc
Copy link

siakc commented Dec 23, 2023

Well I would have done it like this:
A global middleware factory:

function ControllerObj(c, ...params) {
  const controllerInstance = new c(...params)

  return function mw(ctx, next) {
    const method = ctx.method.toLowerCase()
    if(typeof controllerInstance[method] !== 'function') {
      ctx.throw(405)
    }
    controllerInstance[method](ctx, next)
  }
}

A controller class:

class ControllerX {
  constructor(aKnob) {
    this.aKnob = aKnob  // Some tweaks for this route 
  }
  async get(ctx, next) {
    ctx.status = 200
    ctx.body = this.aKnob
    await next()
  }
  // more methods here...
}

And finally:

app
  .use(ControllerObj(ControllerX, 'desiredResponse'))

My belief is that this is not such a general pattern to be implemented in Koa. A plugin maybe...

@evert
Copy link
Author

evert commented Dec 23, 2023

Sure, let's see what the maintainers say. Your approach doesn't really make a difference in terms of cognitive overhead.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants