How do I use this.match() with a stack of middleware while maintaining the controller action? #50

Closed
trusktr opened this Issue Sep 27, 2012 · 10 comments

Projects

None yet

5 participants

@trusktr
trusktr commented Sep 27, 2012

I have something like this: this.match('/', 'pages#main');

but I want to add middleware to it, and keep the controller action at the end, but doing something like this doesn't work: this.match('/', [lock, 'pages#main']); where lock is a middleware function.

How do I add middleware to the route while maintaining the controller-action?

@GothAck
GothAck commented Oct 12, 2012

routes.js

this.match('/', [middleware, middleware, middleware']);
this.match('/', 'pages#main');

or I believe this should work too:
routes.js

this.match('/', 'pages#main');

pages_controller.js

PagesController.main = function () { ... }
PagesController.before('main', [middleware, middleware, middleware]);
@trusktr
trusktr commented Nov 26, 2012

Thanks, I ended up doing your first suggestion:

this.match('/', [middleware, middleware, middleware]);
this.match('/', 'pages#main');

If someone doesn't get to it first, I'd like to write and suggest a modification to Locomotive so that

this.match('/', [middleware, middleware, 'pages#main']);

or

this.match('/', [middleware, middleware], 'pages#main');

are also options.

@brianjmiller

+1 to the suggestion, would be nice to have either a middleware possibility or to expose a "locomotive controller" middleware that would function as @trusktr's first example.

@jaredhanson
Owner

Don't mix middleware and controllers/filters. Stick with one or the other.

@trusktr
trusktr commented Mar 21, 2014

I just realized that the format

this.match('/path', [middleware, middleware], 'controller#method');

actually does work (I'm still on version 0.3.2). I'm not sure why I thought it didn't. Sorry about that.

@jaredhanson
Owner

If that works I don't advise doing it. It's not something I intend to support, and using it will make upgrades harder.

@trusktr
trusktr commented Mar 21, 2014

I kinda like it because it's easy to require login for certain routes:

this.match('/', 'pages#root');
this.match('/profile', [lock], 'pages#profile');

It's nice adding a few characters rather than a whole extra line like

this.match('/', 'pages#root');
this.match('/profile', [lock]); // too much
this.match('/profile', 'pages#profile');
@trusktr
trusktr commented Mar 21, 2014

Aaah, I hadn't tested fully. So doing

this.match('/profile', [lock], 'pages#profile');

in that it will load the login page (due to the lock middleware), but when logged in, I cannot visit /profile.

If I do it the separated way

this.match('/profile', [lock]);
this.match('/profile', 'pages#profile');

then it works.

I still +1 the idea of being able to combine them into one line. No need for redundance. 50% of the code in those two lines is duplicate code.

@adamdonahue

Another option is to use filters, but I agree with the posters here, this really doesn't seem to be an elegant way to chain actions via a route (as opposed to in a controller or via multiple routes and middleware).

Arguably a controller might be the right place for them -- if one assumes a controller is always meant to be context aware, that is, /profile would always need a lock, say. But if that is not the case (and in some sense I feel it isn't), marking the pages#profile call as needing a lock at the controller level seems wrong. It reduces my ability to reason about the code independent of its context, and it embeds into implementation details that should be elsewhere.

But I think the main confusion is that an action (by which I mean a callback that can be added to a route directly) and a controller (which can do much of what an action does but is put into its own namespace with convention-based naming) are very similar.

So, Jared, when you say, "don't mix middleware routes and controllers/filters" I register a small inconsistency because in the degenerate case could I not merely suck all of my controller logic into a middleware 'action'/'callback' and accomplish what trusktr is asking to do? If so then his suggested format seems completely reasonable.

For example, it's surely possible (somehow) to do

this.match('/profile', [lock], function(req, res) {
  // Load PagesController, set variables, and call profile on it.
});

in fact, if all I'm using in my controller is this.req, this.res, ..., etc., and a little convention around rendering and so forth, there's very little work that would be required here to implement this workaround (hack) on the user side.

So the whole distinction between controller and action is really where your users (myself included) could use a little more color.

Or by answering this question: How do I mark certain 'actions' (definition: anything that works to fulfill the semantics of the user's target URI) as needing authentication independent from the action implementation itself?

Thanks.

Adam

@trusktr
trusktr commented Apr 7, 2014

Semantically,

this.match('/profile', [lock], 'pages#profile');

is just cleaner than

this.match('/profile', [lock]);
this.match('/profile', 'pages#profile');

If we are not to "mix middleware routes and controllers/filters", then arguably we should only be able to do

this.match('/profile', 'pages#profile');

and nothing else, leaving it all to the controller!

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