-
Notifications
You must be signed in to change notification settings - Fork 27.9k
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
Add AOT gzip content-encoding support for main build files. #565
Conversation
Currently we only do this for main.js and commons.js only.
} | ||
|
||
res.setHeader('Content-Encoding', 'gzip') | ||
return await this.serveStatic(req, res, gzipPath) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
return await
is not needed. can just be return
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done.
Would it be easy to write a test? |
Adjusted the issue to divide the task in two parts per your recommendation @arunoda |
@rauchg Yep. |
const nextDir = path.resolve(dir, '.next') | ||
|
||
await gzip(path.resolve(nextDir, 'commons.js')) | ||
await gzip(path.resolve(nextDir, 'main.js')) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
await Promise.all
could have a perf advantage here
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah. Parallelism.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done.
Related to #555
Currently, we only do this for
main.js
andcommons.js
only.We can do this for individual
.json
pages as well.For that, we may need to change a bit of how we save pages and so on.
I think we could handle it in a different PR.