This repository was archived by the owner on Jan 28, 2025. It is now read-only.
Let CloudFront do the Gzipping #692
Merged
Conversation
This file contains hidden or 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
dphang
reviewed
Oct 19, 2020
danielcondemarin
force-pushed
the
use-cloudfront-native-http-compression
branch
from
a8a3952 to
dd31e3d
Compare
danielcondemarin
force-pushed
the
use-cloudfront-native-http-compression
branch
from
dd31e3d to
97862bf
Compare
danielcondemarin
changed the title
Codecov Report
@@ Coverage Diff @@
## master #692 +/- ##
==========================================
+ Coverage 80.18% 80.20% +0.02%
==========================================
Files 54 54
Lines 1731 1733 +2
Branches 366 369 +3
==========================================
+ Hits 1388 1390 +2
Misses 285 285
Partials 58 58
Continue to review full report at Codecov.
|
dphang
reviewed
Oct 21, 2020
| @@ -46,6 +49,9 @@ describe("Pages Tests", () => { | |||
| (method) => { | |||
| it(`allows HTTP method for path ${path}: ${method}`, () => { | |||
| cy.request({ url: path, method: method }).then((response) => { | |||
| if (method !== "HEAD") { | |||
There was a problem hiding this comment.
Maybe add for public files etc. but can be done later.
dphang
reviewed
Oct 21, 2020
| Cypress.Commands.add( | ||
| "verifyResponseIsCompressed", | ||
| (response: Cypress.Response) => { | ||
| expect(response.headers["content-encoding"]).to.equal("gzip"); |
There was a problem hiding this comment.
nice, thanks for adding a test in the e2e tests as well.
dphang
approved these changes
Oct 21, 2020
delete-merged-branch
bot
deleted the
use-cloudfront-native-http-compression
branch
Merged
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
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.
Motivation
We've been manually doing Gzip compression in lambda@edge so far due to CloudFront not compressing lambda@edge responses. However, seems like that has been fixed after discussing with Cristian from the CloudFront team and doing some testing of my own.
This should allow us to use Gzip and Brotli compression without extra code in the Lambda@Edge handler. Furthermore the CloudFront distributions created by serverless-next.js already enable compression by default.
From my initial findings Gzip works fine, but Brotli isn't despite being officially supported now
Seems distributions will need to set
EnableAcceptEncodingBrotli.Ref docs: https://docs.aws.amazon.com/AmazonCloudFront/latest/DeveloperGuide/ServingCompressedFiles.html
Proposal
Feature flag off the manual compression in the lambda@edge handler. Hope we can deprecate it in future from the component inputs. Maybe leave as an option in the compat layer if there is a genuine use case for it.
[x] - Added enableHTTPCompression input.
[x] - Look into Brotli support. (A cache policy needs to be added to support Brotli. Will look into this on a separate PR).
[x] - Document
enableHTTPCompressionin the lambda at edge package.[x] - Updated E2E to check for gzipped responses