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

Consider getting rid of the concept of MiddlewareObject #184

Closed
ericmorand opened this issue Nov 8, 2021 · 3 comments
Closed

Consider getting rid of the concept of MiddlewareObject #184

ericmorand opened this issue Nov 8, 2021 · 3 comments

Comments

@ericmorand
Copy link

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:

  • it doesn't add anything valuable - the thing it brings to the table is already available natively
  • it adds some unecessary complexity to the types and the runtime code
  • it makes the Curveball Application more difficult to integrate with some generic application design
@evert
Copy link
Member

evert commented Nov 8, 2021

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
Copy link
Author

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
Copy link
Member

evert commented Feb 9, 2022

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.

@evert evert closed this as completed Feb 9, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants