On middleware registration, pass in the mojito object

[imalberto]

From @aljimenez
This argument is to be useful for a couple of our middlewares mostly so that we can return a middleware function based on the appConfig. Now it seems like we only have access to the store/appConfig per request so we have to execute extra logic every request instead of once, when the middleware gets added. Maybe it would be useful to pass the mojito object, when the middleware gets added.

[caridy]

@imalberto can you provide more details about this? in theory, you can have a top level middleware that adds the mojito instance into the req object potentially, and that will enabling accessing in any extra middleware.

[aljimenez]

Take mojito-jscheck middleware as an example; we have some logic in order to determine whether we want this middleware, based on the appConfig. But now that the midConfig is not passed, we would have to execute this logic during request time, using req.app.mojito.store.getAppConfig.

[aljimenez]

The common pattern that I see in a couple of our middlewares is that when it is added, we use the appConfig to determine whether this middleware should be executed per request. If not we return:

function (req, res, next) {
    next();
}

So maybe there should be a way for us to specify that we don't need this middleware by returning null instead of a function when it is created. If this happens then middleware.js should not append the middleware to the handlers array.

[realistschuckle]

I think that @aljimenez wants the configuration object. I figured out a way to do just that using Yahoo! Configuration Bundle that requires using a "private" property. It would be nice if we could have that information without that hack.

If we look at ~/mojito/lib/mojito.js in the MojitoServer.prototype._useMiddleware function, we can see that user-supplied middleware gets included with app.use(require(midPath));. However, mojito-specific middleware gets loaded with app.use(require(midPath)(midConfig));. Can we provide a way to have the Configuration Object (as midConfig) to be passed to the user-supplied middleware just as we do with the mojito-specific middleware? We'd need a way to specify that the user-specified middleware contains a "middleware factory" that will accept the Configuration Object as opposed to a plain, Jane middleware object that only exports function(req, res, next)...

Any thoughts? If we come up with a sane scheme to support this, I would like the opportunity to implement it and submit the pull request. I find it a germane issue for the application that I have under construction...

Or, should I just package my earlier get-config-in-middleware logic as a middleware component, attach the config to the req as proposed by @caridy, and just put it somewhere that shared mojito things go? Is there some gallery-ish thing? Like YUI Library - Gallery? :)

[caridy]

@realistschuckle that mechanism exists today, and will remain even after we release mojito next officially (in ALPHA today). The mechanism is triggered by the prefix in the middleware name in application.json, if the middleware starts with mojito- it will be treated as a "middleware factory", and it will pass the proper configuration midConfig, if not, it will be treated as an express middleware.

If this issue is not targeting that specific mechanism but the fact that calling app.use() directly by developers in app.js without having to add the middleware into application.json, and still use the "middleware factory", then I'm against it. I think there are plenty of mechanism for custom middleware in express, and we should not be dictating how to do it.

[realistschuckle]

@caridy I see that mechanism and have tried to use it. I found that the method that makes the list, MojitoServer.prototype._makeMiddewareList, gets in the way. If I write a "special" middleware named mojito-is-loved-by-realistschuckle-middleware, then I have to make sure that I configure the entire middleware stack correctly because of the logic branch if (hasMojito) that builds the list. Now, I have to know all of the expectations of the Mojito stack to get it "configured correctly" to include the order in which they should occur.

It may turn out that it does not matter; however, when I tried it for my current application, I found that stuff just didn't work anymore. Instead of spending the time that it would take to discover the correct configuration, presumably what I find in MojitoServer.MOJITO_MIDDLEWARE, I just decided to hack around this issue as a stopgap. I'd like a "right" solution. Because this feels as if I need to know too much about the internal workings...

So, I want to target that specific mechanism. However, I think that MojitoServer.prototype._makeMiddewareList should be a little smarter so I can specify my "mojito"-prefixed middleware names for "middleware factory" stuff and, yet, still get all that Mojito goodness?

EDIT: Also, thanks for responding so quickly. I appreciate that you take the time to comment though you have so much to do on the project. If I'm taking too much time, just tell me to shut it. :D

[caridy]

oh, I forgot to mention a very small detail :), my bad, but it is an important one, if you have a mojito- prefixed middleware, that means you're trying to mess with the default order, and you will have to re-specify the default middleware by name as well (keep in mind that by default your custom middleware will work before the default ones). In other words, you can do this:

{
middleware: [
    './mymiddleware/mojito-a-test'
    'mojito-handler-static',
    'mojito-parser-body',
    'mojito-parser-cookies',
    'mojito-contextualizer',
    'mojito-handler-tunnel',
    './mymiddleware/mojito-another-test'
]
}

hope that help to clarify how to use the factory. in this particular example, I'm loading two custom factory middleware, mixing them with the default ones. I know I know, this is a terrible API, and a very bad design, (for the record: no my fault), and that's why I want to keep it in place for backward compatibility, but encourage people to follow express principles, but if you look into mojito-next, you will see that the low-level middleware are not longer doing much with the factory.

@imalberto can you double check all the things that I'm saying here? I'm abusing my memory, so I might be wrong. Ideally, we could move away from the factory at some point in the near future, and ideally we can start by exposing the low-level middleware side by side with their factory counterpart for backward compatibility.

[realistschuckle]

@caridy I don't know why I didn't check mojito-next to check out the future direction. That is completely my fault! To the point of your comment, though, what you write makes sense (and I agree it could be easier for developers to use custom middleware:).

Just as a point of clarification, will the Mojito team release mojito-next with a new major revision number to imply incompatible changes with the existing Mojito stack? Or should mojito-next fully support existing Mojito applications and provide new features?

[imalberto]

@caridy What you mentioned in here is correct.

@realistschuckle The next major version is anticipated to be versioned 0.9.0 and should be available by end of Feb. Most of the improvements are internal, but from the app perspective, there will be some backward incompatibilities mostly related to how your application is bootstrapped with Mojito. But how you configure and execute your mojits will remain unchanged.

Btw, this issue only applies to the mojito-next branch, and has been addressed. So closing this one.

[drewfish]

Yeah the current mojito middleware is a mess (and I'm to blame for much of it!). For mojito-next I would love if we could make it very simple -- just follow express common approaches. Even if this means breaking backwards compatibility.

If you look at the express middleware most of them are factories. Some of them take arguments (such as app.use(express.static(__dirname + '/public'))) and some of them don't (such as app.use(express.bodyParser())).

What would be nice, to me, is that we remove any magic and make it so that every app has to register all middleware themselves. We can provide a mojito.middleware which contains the loaded files but it is still up to the app.js author to write app.use(mojito.middleware.tunnel(...)), app.use(mojito.middleware.contextualizer(...)), etc. Each middleware should be passed the arguments (if it is a factory) as appropriate for that middleware.

Now, on top of that we could provide some sugar. Perhaps something like mojito.middleware is really also a function so that you can do app.use(mojito.middleware()) and it'll create and return all the middleware in the "right" order. (Hmm... maybe that could be done with mojito.middleware(app) instead.) Or something like that, you get the idea. It's all just sugar to make common stuff easy.

In reality mojito doesn't have that many middleware, and each is performing a specific independent roll. It would be nice if app developers become familiar with them so that they can decide when to use them, or even when not to use them or use a replacement instead.

[caridy]

I agree with @drewfish, let's remove the ability to use application.json's middleware to contextualize middleware.

[caridy]

solved, we are now fully aligned with express. no more configuration in application.json for middleware.

[realistschuckle]

+1 - I like this solution because it makes middleware configuration more deliberate. Thank you, everyone, for participating in this discussion.