Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix(koa): don't compress assets via send #789

Merged
merged 1 commit into from
Jun 30, 2018
Merged

Conversation

benjie
Copy link
Member

@benjie benjie commented Jun 27, 2018

Koa sometimes seems to hang when loading GraphiQL; I've not been able to deliberately reproduce it. This seems to be solving it so far... Really we ought to replace sendFile with something Koa-friendly.

@sjmcdowall
Copy link

sjmcdowall commented Jun 27, 2018 via email

@benjie
Copy link
Member Author

benjie commented Jun 27, 2018

Nothing's doing compression (except the Koa middleware of course); the issue is that send writes the data straight to the res response object (including setting the relevant headers/etc) which works just fine with Express because it's based on Connect based on http.createServer and the response cycle ends there. However Koa takes more control and doesn't like it when middleware writes directly to the response object - when we return control to Koa it wants the data to not already have been written. The Koa compress middleware has even stronger expectations than Koa itself, and these expectations are broken by most things that write directly to res, as PostGraphile does currently.

@sjmcdowall
Copy link

Thanks for the explanation -- that makes a lot of sense!

@benjie benjie merged commit bb851da into master Jun 30, 2018
@benjie benjie deleted the koa-no-compress-send branch June 30, 2018 12:10
@hengnee
Copy link

hengnee commented Jan 14, 2020

I think the same thing is happening for Fastify as it seems to skip the Fastify Routes and fastify.compress plugin don't seem to be able to compress the output from PostGraphile.

@benjie
Copy link
Member Author

benjie commented Jan 14, 2020

Please submit a PR with a failing test for your setup.

@hengnee
Copy link

hengnee commented Jan 23, 2020

Please submit a PR with a failing test for your setup.

Hi, I'm so sorry but may I provide a link for the failing test which has comments regarding the issue instead?

https://github.com/hengnee/rectify_fastify_postgraphile

@benjie
Copy link
Member Author

benjie commented Jan 23, 2020

If you file a PR it’ll be addressed faster; but if you can’t, please file and issue with the above otherwise your request will be lost.

@madflow madflow mentioned this pull request Jan 25, 2020
2 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants