feat(devserver/http): support gzipped responses #4011
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎
9 Ignored Deployments
|
🟢 CI successful 🟢Thanks |
kwonoj
force-pushed
the
feat-gzipped-response-turbo-dev
branch
from
b34ae77
to
029759c
Compare
Benchmark for 072559eClick to view benchmark
|
jridgewell
reviewed
Mar 1, 2023
| let stream_ext = content | ||
| .read() | ||
| .into_stream() | ||
| .map_err(|err| std::io::Error::new(std::io::ErrorKind::Other, err)); |
There was a problem hiding this comment.
Nit: (with appropriate use std::io; statement at the top)
Suggested change
| .map_err(|err| std::io::Error::new(std::io::ErrorKind::Other, err)); | |
| .map_err(|err| io::Error::new(io::ErrorKind::Other, err)); |
Comment on lines
130
to
132
| let gzipped_stream = tokio_util::io::ReaderStream::new( | ||
| async_compression::tokio::bufread::GzipEncoder::new( | ||
| tokio_util::io::StreamReader::new(stream_ext), |
There was a problem hiding this comment.
Nit: with use tokio_util::io::{ReaderStream, StreamReader}; use async_compression::tokio::bufread::GzipEncoder;
Suggested change
| let gzipped_stream = tokio_util::io::ReaderStream::new( | |
| async_compression::tokio::bufread::GzipEncoder::new( | |
| tokio_util::io::StreamReader::new(stream_ext), | |
| let gzipped_stream = ReaderStream::new( | |
| GzipEncoder::new( | |
| StreamReader::new(stream_ext), |
| .into_stream() | ||
| .map_err(|err| std::io::Error::new(std::io::ErrorKind::Other, err)); | ||
|
|
||
| let gzipped_stream = tokio_util::io::ReaderStream::new( |
There was a problem hiding this comment.
Not all content should be compressed, mainly images except SVG. Compressing binary formats will only increase the file size. Can we do an inspection for the content-type to ensure it's a textual format?
There was a problem hiding this comment.
Probably yes, could have some basic mime check for the content types.
There was a problem hiding this comment.
Express's compression middleware uses compressible
There was a problem hiding this comment.
yes, that's what I figured. But we should have rust corresponding one instead, isn't it?
There was a problem hiding this comment.
The rust crate seems out of date? We could get most of the way by just using the fallback algorithm from compressible:
text/**/*+json*/*+text*/*+xml
and a few extras for application/json and application/javascript
There was a problem hiding this comment.
You mean only compress for those, correct?
kwonoj
force-pushed
the
feat-gzipped-response-turbo-dev
branch
from
029759c
to
0a6892f
Compare
Benchmark for 6434755Click to view benchmark
|
jridgewell
reviewed
Mar 2, 2023
| @@ -92,14 +100,28 @@ pub async fn process_request_with_content_source( | |||
| ); | |||
| } | |||
|
|
|||
| // naively checking if content is `compressible`. | |||
| let mut should_compress = false; | |||
| let should_compress_predicate = |mime: &Mime| match (mime.type_(), mime.subtype()) { | |||
There was a problem hiding this comment.
We should also be checking mime.suffix() for:
- xml
- json
- text
kwonoj
force-pushed
the
feat-gzipped-response-turbo-dev
branch
from
0a6892f
to
a88aefb
Compare
jridgewell
approved these changes
Mar 2, 2023
kwonoj
added
the
pr: automerge
Benchmark for 3d84ec8
Click to view full benchmark
|
ijjk
added a commit
to vercel/next.js
that referenced
this pull request
Mar 2, 2023
# New Features * vercel/turbo#4011 # Performance Improvements * vercel/turbo#3955 * vercel/turbo#4018 # Bug Fixes * vercel/turbo#4037 * vercel/turbo#4028 # Other * vercel/turbo#3974 * vercel/turbo#4015 * vercel/turbo#3999 * vercel/turbo#4026 * vercel/turbo#4053 * vercel/turbo#3891 Co-authored-by: JJ Kasper <jj@jjsweb.site>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Implements WEB-666.
This PR enables gzip compressions for the responses by default. This can be extended to other compression method (i.e brotli), but for now implements minimal default compression only to pass existing test case.
Fixes
test/integration/compression/test/index.test.js.