Skip to content

Add AOT gzip content-encoding support for main build files.#565

Merged
rauchg merged 3 commits intovercel:masterfrom
arunoda:gzip
Dec 29, 2016
Merged

Add AOT gzip content-encoding support for main build files.#565
rauchg merged 3 commits intovercel:masterfrom
arunoda:gzip

Conversation

@arunoda
Copy link
Copy Markdown
Contributor

@arunoda arunoda commented Dec 29, 2016

Related to #555

Currently, we only do this for main.js and commons.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.

Currently we only do this for
main.js and commons.js only.
server/index.js Outdated
}

res.setHeader('Content-Encoding', 'gzip')
return await this.serveStatic(req, res, gzipPath)
Copy link
Copy Markdown
Member

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

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Done.

@rauchg
Copy link
Copy Markdown
Member

rauchg commented Dec 29, 2016

Would it be easy to write a test?

@rauchg
Copy link
Copy Markdown
Member

rauchg commented Dec 29, 2016

Adjusted the issue to divide the task in two parts per your recommendation @arunoda

@arunoda
Copy link
Copy Markdown
Contributor Author

arunoda commented Dec 29, 2016

@rauchg Yep.
I not sure about to have a test case since we don't do a production build right now.
May be we need to have a different test setup, targeting individual examples.

const nextDir = path.resolve(dir, '.next')

await gzip(path.resolve(nextDir, 'commons.js'))
await gzip(path.resolve(nextDir, 'main.js'))
Copy link
Copy Markdown
Member

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

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Yeah. Parallelism.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Done.

@arunoda arunoda changed the title Add AOT gzip content-encoding support Add AOT gzip content-encoding support for main build files. Dec 29, 2016
@rauchg rauchg mentioned this pull request Dec 29, 2016
3 tasks
@rauchg rauchg merged commit 29c2267 into vercel:master Dec 29, 2016
@arunoda arunoda deleted the gzip branch December 30, 2016 10:10
@lock lock bot locked as resolved and limited conversation to collaborators Jan 19, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants