Handle middleware passed after the service to app.use#178
Handle middleware passed after the service to app.use#178daffl merged 3 commits intofeathersjs:masterfrom dbkaplun:post-service-middleware
Conversation
lib/application.js
Outdated
There was a problem hiding this comment.
Small style nit, I'd make it
if (parts[1].length > 1) {
throw new Error('more than one service passed to app.use');
}|
Awesome, thank you! I made some comments and it would be nice to have some tests for this functionality (probably similar to https://github.com/feathersjs/feathers/blob/master/test/providers/rest.test.js#L337). |
|
@daffl fixed and added tests |
There was a problem hiding this comment.
I think we could set and then check for res.data.postService instead since the handler should run last.
There was a problem hiding this comment.
Sorry I don't understand, can you rephrase please?
There was a problem hiding this comment.
What I mean is to instead set something like:
res.data.postService = true;and then in the test check for
assert.ok(data.postService, 'Post-service middleware updated response');instead of assert.equal(response.headers['post-service'], 'dishes', 'Post-service middleware updated response');.
There was a problem hiding this comment.
I see. I did it this way because I wanted to ensure that the post-service handler actually runs after the service, but I can change it if you like.
There was a problem hiding this comment.
Good point. Lets leave it then. I will merge shortly. Thanks again!
|
This is great! Thank you! One more small comment for the test. Nothing major but then we can get it in and ready for a version 1.3. |
Handle middleware passed after the service to app.use
|
Oh nice! This PR snuck by me but I've run into this before. Thanks @dbkaplun! |
* Add .idea/ * Update _gitignore add some more.
Closes #176