Skip to content
This repository has been archived by the owner on May 17, 2019. It is now read-only.

Use end event rather than overwriting ctx.res.end #43

Merged
merged 5 commits into from
Aug 30, 2018

Conversation

ganemone
Copy link
Contributor

This simplifies the logic for knowing when a response is sent. Also fixes a bug where express and koa-helmet clash with respect to ctx.req.secure.

src/server.js Outdated
handler(ctx.req, ctx.res, () => {
// $FlowFixMe
ctx.res.end = oldEnd;
res.on('end', listener);

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

looking at how this is implemented in express, shouldn't we also listen on finish? asking because of https://github.com/jshttp/on-finished/blob/master/index.js#L104?

or maybe we could start using the on-finished package?

KevinGrandon
KevinGrandon previously approved these changes Aug 30, 2018
Copy link
Contributor

@KevinGrandon KevinGrandon left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Seems like this just needs some flow fixes. Feel free to fix and land, or re-request review if you want.

@ganemone ganemone merged commit 7f40e52 into fusionjs:master Aug 30, 2018
@AlexMSmithCA AlexMSmithCA mentioned this pull request Sep 5, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants