New: Add rule to check if resources are served compressed #731
Conversation
alrra
force-pushed
the
master
branch
2 times, most recently
from
f1ebb90
to
98f3144
Compare
| @@ -645,6 +645,7 @@ const rule: IRuleBuilder = { | |||
| category: Category.performance, | |||
| description: 'Require resources to be served compressed' | |||
| }, | |||
| ignoredConnectors: ['chrome'], // Headless Chrome on Travis CI fails miserably. :( | |||
There was a problem hiding this comment.
Hopefully this is temporary, all the test pass even for the chrome connector on my machine. :(
|
Note: I've merged some of the fixes from this PR into |
|
|
||
| `gzip` is the most used encoding nowadays as it strikes a good | ||
| balance between compression ratio (as high as 70-90% especially | ||
| for larger files) and encoding time, and is supported pretty much |
There was a problem hiding this comment.
Do you have a link for the 70-90% claim?
| `gzip` is the most used encoding nowadays as it strikes a good | ||
| balance between compression ratio (as high as 70-90% especially | ||
| for larger files) and encoding time, and is supported pretty much | ||
| eveywere. |
| eveywere. | ||
|
|
||
| Better savings can be achieved using [`Zopfli`][zopfli] which | ||
| can reduce the size on average with [3–8% more then `gzip`][[zopfli |
There was a problem hiding this comment.
Also, there is another issue here: double [[.
| can reduce the size on average with [3–8% more then `gzip`][[zopfli | ||
| blog post]. Since `Zopfli` output (for the `gzip` option) is valid | ||
| `gzip` content, `Zopfli` works eveywere `gzip` works. The only | ||
| downsize is that encoding takes way more time than with `gzip`, |
| `gzip` content, `Zopfli` works eveywere `gzip` works. The only | ||
| downsize is that encoding takes way more time than with `gzip`, | ||
| thus, making `Zopfli` more suitable for static content (i.e. encoding | ||
| resources with is as part of build script, not on the fly). |
There was a problem hiding this comment.
with isas part of a build script
|
|
||
| When resources are served compressed, they should be served with | ||
| the `Vary` header containing the `Accept-Encoding` value (or with | ||
| something such as `Cache-Control: private` that prevents caching |
There was a problem hiding this comment.
that prevents caching in proxy caches and such altogether
The final user can cache the response so we are not preventing caching altogether:
Indicates that all or part of the response message is intended for a single user and MUST NOT be cached by a shared cache, such as a proxy server.
| * Resources should be served compressed only when requested as such, | ||
| appropriately encoded, and without relying on user agent sniffing. | ||
|
|
||
| What the `Accept-Encoding` request header specified should be |
| appropriately encoded, and without relying on user agent sniffing. | ||
|
|
||
| What the `Accept-Encoding` request header specified should be | ||
| respected. Sending a the content encoded with a different encoding |
|
|
||
| What the `Accept-Encoding` request header specified should be | ||
| respected. Sending a the content encoded with a different encoding | ||
| then one of the ones accepted can lead to problems. |
|
|
||
| ## Can the rule be configured? | ||
|
|
||
| You can overwrite the defaults by specifying what type of compression |
| @@ -0,0 +1,672 @@ | |||
| /** | |||
| * @fileoverview Check is resources are served compressed. | |||
| * | ||
| * 2. Zopfli | ||
| * | ||
| * On thing that Zopfli does is that it sets FLG and |
|
|
||
| /* | ||
| * TODO: Remove the following once connectors | ||
| * support Brotli compression. |
There was a problem hiding this comment.
Can you open an issue for this?
Basically what we need is requester to auto decompress Brotli.
There was a problem hiding this comment.
Can you open an issue for this?
Done.
| }; | ||
|
|
||
| const generateCompressionMessage = (encoding: string, notRequired?: boolean, suffix?: string) => { | ||
| return `Should${notRequired ? ' not' : ''} be served compressed${encoding ? ` with ${encoding}` : ''}${suffix ? ` ${suffix}` : ''}.`; |
There was a problem hiding this comment.
We should make the message clearer.
Right now if a resources is not compressed the user is going to get 2 messages:
Should be served compressed with Zopfli
Should be served compressed with Brotli
which can be confusing. A way could be to say the protocol as you proposed the other day:
Should be served compressed with Zopfli for HTTP
Should be served compressed with Brotli for HTTPS
There was a problem hiding this comment.
Fixed.
(Note: Only changed the message for Brotli since Zopfli applies to both)
|
@molant I addressed all your feedback! |
| */ | ||
|
|
||
| if (isTextMediaType(mediaType) || | ||
| ['image/x-icon', |
There was a problem hiding this comment.
I think you should have these kind of arrays of the method as a constant.
Fix several issues found when analyzing real websites with edge cases: * 204 status code with a body. * Missing headers. * Better error reporting.
|
🎉 |
Fix several issues found when analyzing real websites with edge cases: * 204 status code with a body. * Missing headers. * Better error reporting. - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - Close #731
Fix several issues found when analyzing real websites with edge cases: * 204 status code with a body. * Missing headers. * Better error reporting. - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - Close #731
Pull request checklist
Make sure you:
For non-trivial changes, please make sure you also:
Short description of the change(s)
Fix #1
Fix #12