feat(http): Add redirect property to hook context#2484
feat(http): Add redirect property to hook context#2484vonagam wants to merge 1 commit intofeathersjs:dovefrom
redirect property to hook context#2484Conversation
2d14a00 to
4c657f4
Compare
|
Thank you for putting that up! I'm a little torn of adding HTTP specific things (this could be done in a Koa middleware as well right?) into the hook context although granted this has already snuck in with the |
|
Yeah, it could be implemented in a custom middleware. But if you create a library and don't know which transport end-user will have then you will need to implement a custom middleware for each http transport (before there was only one official, now there are two, in future there can be more). And there will be no standardization, meaning that if some other library or user himself want to work with redirects then it might lead to conflicts between solutions. For me redirects seem like a really essential part of http protocol that it is strange to see no way to specify them in feathers. With websocket |
4c657f4 to
8ffce9e
Compare
|
I was actually thinking exactly the same thing with |
|
To make |
|
#2496 has been merged so we probably want to update this one accordingly. |
|
Once So I rebased #2486 and after (if) it is merged |
|
#2524 is merged and this redirect functionality should be supported now with setting |
Adds standard way to specify a redirect location.
Before it was ok (not perfect, but ok) to do it with an express middleware but since there is now koa transport (and potentially in the future there will be other bare bone http one) it would be more beneficial to have a proper way for common functionality.
To do it consistently had to add
encodeurldependency totransport-commons- bothkoaandexpressuse it to escape location, butkoadoes not have.location()function (unlike express), only.redirect()which also does bunch of other stuff with body.If
statusCodeis not specified andredirectis present, then status defaults to 303. Default should be either that or 302 (former says to use GET request, later should be for temporary redirects with same request method).Made code in rest between
koaandexpressa little more consistent.Also since a hook manager can theoretically (if overwritten) return a different context from one that was passed in to
initializeContexthave to setres.hook/ctx.hooktwice - first whenbaseis created (in case a method throws), second after a final context returned. (Not that anyone would do it, just a pedantic change.)Still need to add some tests.