curveball / core

The Curveball framework is a TypeScript framework for node.js with support for modern HTTP features.
https://curveballjs.org/
MIT License
525 stars 7 forks source link

Consider getting rid of the concept of MiddlewareObject #184

Closed ericmorand closed 2 years ago

ericmorand commented 2 years ago

I've been playing with Curveball for weeks, in search for a simple, lighweight and "pure" framework, and I am globally very satisfied with the whole package.

There is one thing though that I don't quite understand: the concept of MiddlewareObject, and more specifically what it brings to the table that is not already provided by the framework.

Here is the type definition of a MiddlewareObject:

type MiddlewareObject = {
  [middlewareCall]: MiddlewareFunction;
};

It permits using an object as a middleware, as long as this object has a property identified by the symbol middlewareCall.

Supporting this pattern adds a non-negligeable layer of complexity to the types themselves:

export type Middleware = MiddlewareFunction | MiddlewareObject;

And to the runtime code itself:

// this is called at multiple places in the code just to check if a middleware is a MiddlewareObject
function isMiddlewareObject(input) {
    return input[middlewareCall] !== undefined;
}

It looks legit to add complexity to implement something that adds some value to the framework, but here it doesn't add any value.

Would one want to use an object as a middleware, this simple native and straightforward approach is enough:

class MyMiddlewareObject {
     homePageRoute(context: Context) {
        // do whatever is needed
     }
}

const myMiddlewareObject = new MyMiddlewareObject ();

app.use((context: Context) => myMiddlewareObject.homePageRoute(context));
// or, less explicitly
app.use(myMiddlewareObject.homePageRoute.bind(myMiddlewareObject));

It is so straightforward and more readable and explicit than using some symbol-based convention that I fail to see what is the added value of the concept of MiddlewareObject.

But this is not the worst part of it. The MiddlewareObject support removes the purity of the definition of what a Middleware is.

Conceptually, and universally, a middleware is this:

type Middleware = (context: Context, next: Middleware) => Promise<void>;

Which is basically the definition of MiddlewareFunction.

By adding the concept of MiddlewareObject, the definition of a Curveball Middleware is something else than the pure and universally accepted definition above. It makes trying to integrate Curveball into some universal application design more difficult than needed. It is for example impossible to have Curveball Application match this generic and universally accepted interface:

type Application<C> = {
    listen(channel: C) => Promise<void>;
    use(...middleware: Array<Middleware>) => void;
};

I then propose to remove the MiddlewareObject support:

evert commented 2 years ago

The reason this was originally added, was to make it super easy to add the controller from @curveball/controller. Can we make this still work?

class MyController extends Controller {
}

application.use(new MyController());

I'm curious what kind of practical pitfalls you've found with the current design, your current argument seems to be that it's not 'pure', but it would be helpful to see a real-world example of when this might be detrimental.

ericmorand commented 2 years ago

I'll add a use case. 🙂

Maybe when you see it you'll find a way to solve my issue by the way.

But if you think about the reason you did add MiddlewareObject, you may consider that it is forward-thinking: you knew when writing the core module that you would add a controller module and that to support this module you would need a MiddlewareObject.

In theory, you write a core that exposes an opinionated design pattern and people interested in it write modules that support that pattern. Other people not interested in that pattern use another framework.

evert commented 2 years ago

I don't think this will happen! If anything, I think it's more likely we'll expand use-cases of middleware-objects. See #189 for some examples.