feat: directly calling response #286
Conversation
|
I've included a proposal to detect whether the user code handled the response manually or not. |
|
P.S.: the failing tests are newly detected failures from the |
fabiob
changed the title
src/driver/koa/KoaDriver.ts
Outdated
| @@ -209,6 +209,12 @@ export class KoaDriver extends BaseDriver implements Driver { | |||
| */ | |||
| handleSuccess(result: any, action: ActionMetadata, options: Action): void { | |||
|
|
|||
| // if the action returned the context or the response object itself, short-circuits | |||
| if (result && (result === options.response || result === options.context)) { | |||
| options.next(); | |||
There was a problem hiding this comment.
You need to return next call in koa, otherwise you will break the chain. We don't have every edge case covered in tests.
There was a problem hiding this comment.
Looks good but the test-utils.ts part needs to be reviewed by @pleerock.
|
@19majkel94 we can merge right? or do we want to wait for a review from @pleerock ? |
|
I would wait for @pleerock review due to changes in test utils. |
|
Rebased to include latest |
|
Would it help if I extract the test utils into another PR? |
Signed-off-by: Fábio Batista <fabio@elementarsistemas.com.br>
Signed-off-by: Fábio Batista <fabio@elementarsistemas.com.br>
fabiob
changed the title
|
OK, I've extracted the test-utils change into another PR (#297). @NoNameProvided @19majkel94, do you guys think we can merge this faster? |
Yes, but we still have to wait for @pleerock to release the merged PRs on npm 😜 |
NoNameProvided
changed the title
|
This issue has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs. |
I've added some tests for the documented way to write to the response directly, as in: https://github.com/pleerock/routing-controllers#using-request-and-response-objects. It looks like #241 broke this usage pattern.