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

No HTML compression via custom server & app.render() #16378

Closed
bobaaaaa opened this issue Aug 20, 2020 · 6 comments · Fixed by #18891
Closed

No HTML compression via custom server & app.render() #16378

bobaaaaa opened this issue Aug 20, 2020 · 6 comments · Fixed by #18891
Labels
good first issue Easy to fix issues, good for newcomers
Milestone

Comments

@bobaaaaa
Copy link
Contributor

Bug report

Describe the bug

If you write custom server routes via app.render() there is no gzip compression for the HTML document. I looked at the source code: the handleCompression() fn is only called via the app.getRequestHandler(). I guess this is not intended.

public async render(
req: IncomingMessage,
res: ServerResponse,
pathname: string,
query: ParsedUrlQuery = {},
parsedUrl?: UrlWithParsedQuery
): Promise<void> {

To Reproduce

  1. Clone the custom-server example (yarn create next-app --example custom-server custom-server-app)
  2. yarn build && yarn start
  3. Route to http://localhost:3000/a inspect HTML document response header via chrome dev tools or cURL
  4. There is no Content-Encoding: gzip response header 👎
  5. Route to index http://localhost:3000/ inspect HTML document response header via chrome dev tools or cURL
  6. Content-Encoding: gzip response header available 👍

Expected behavior

  • gzip compression of HTML documents rendered via app.render()
  • Response Header: Content-Encoding: gzip

System information

  • OS: macOS
  • Version of Next.js: 9.5.2
  • Version of Node.js: 12.18.2
@timneutkens timneutkens added good first issue Easy to fix issues, good for newcomers kind: bug labels Aug 21, 2020
@timneutkens timneutkens added this to the backlog milestone Aug 21, 2020
@YichiZ
Copy link
Contributor

YichiZ commented Sep 15, 2020

Hey,

I am going to try this out. Just to be clear, what you mean is that there should be an Content-Encoding: gzip in the Response Headers like this:
image

But this is not on the sub pages:
image

@bobaaaaa
Copy link
Contributor Author

@YichiZ exactly

@shobhitsharma
Copy link

@timneutkens Is the related bug or PR is manageable to have this resolved?

@timneutkens
Copy link
Member

The open PR has failing tests and no new tests added. Feel free to open a new PR that does have an integration test 👍

@radoslawgrochowski
Copy link
Contributor

I've added test cases, @timneutkens.

I have one question - should app.render with /static content also be compressed?

timneutkens added a commit that referenced this issue May 7, 2021
Co-authored-by: Radosław Grochowski <radoslaw.grochowski@grupawp.pl>
Co-authored-by: Tim Neutkens <timneutkens@me.com>
flybayer pushed a commit to blitz-js/next.js that referenced this issue Jun 1, 2021
…vercel#18891)

Co-authored-by: Radosław Grochowski <radoslaw.grochowski@grupawp.pl>
Co-authored-by: Tim Neutkens <timneutkens@me.com>
@balazsorban44
Copy link
Member

This issue has been automatically locked due to no recent activity. If you are running into a similar issue, please create a new issue with the steps to reproduce. Thank you.

@vercel vercel locked as resolved and limited conversation to collaborators Jan 28, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
good first issue Easy to fix issues, good for newcomers
Projects
None yet
Development

Successfully merging a pull request may close this issue.

6 participants