-
Notifications
You must be signed in to change notification settings - Fork 606
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
jquery-1.12.4.min.js on CDN is not compressed #103
Comments
@Krinkle any ideas? Didn't we clear the cache at one point? |
for anyone else experiencing this, i was able to resolve the issue by using Google's CDN instead: <script src="https://ajax.googleapis.com/ajax/libs/jquery/1.12.4/jquery.min.js"></script> |
@timmywil I see the same issue on my side. I. checked 2.2.4 & 3.7.1 as well and none were compressed. |
A previous similar issue: #98 |
From my investigation with Timmy, it looks like advertising Brotli support (which all browsers have done for a long time) is tripping the logic:
|
It seems to be more complicated than that, but we have a possible solution. |
This seems to be fixed now. Thanks for the report! We are purging some of the most popular URLs individually so they update quickly, but any ones we miss will update eventually. |
i'm not sure if it's worth the time investment or not, but you could set up an automated test to ensure that resources are properly compressed in the future. i may or may not be able to help with that depending on your tech preferences as far as the tests. |
We have a smoke test in the https://github.com/jquery/infrastructure-puppet repository, which includes tests for Code/Codeorigin. |
i see - so if i'm understanding correctly, would it be possible to add a test for the presence of compression there? and would it perhaps be worth checking (at a minimum) the most recent versions of jQuery 1,2,3? |
@EvaLok That's right! We might not need to test multiple versions since all our handling is file agnostic, but one test with gzip compression for one JS file (e.g. the minified JS file of a jQuery 3.x version), and one for a CSS file (e.g. the minified stylesheet of a jQuery UI theme), would be very useful! |
@Krinkle taking a bit closer look at this, it seems that the test i just highlighted is not interacting with a file served using compression, as the content length check is expecting a fairly large size. Does this mean that a test for this needs to be created somewhere else in the build process? |
@EvaLok The linked test suite sends requests to the live service. |
@Krinkle OK - i played around with this a bit, and thought i had a fairly reasonably modification to the test: Unit::testHttp( $server, '/jquery-3.0.0.js', [
"Accept-Encoding: gzip, deflate, br, zstd",
], [
'status' => '200',
'server' => 'nginx',
'content-type' => 'application/javascript; charset=utf-8',
'content-encoding' => 'gzip',
'content-length' => '77731',
'last-modified' => 'Fri, 18 Oct 1991 12:00:00 GMT',
'vary' => 'Accept-Encoding',
'etag' => '"28feccc0-40464"',
'cache-control' => 'public, max-age=31536000, stale-while-revalidate=604800',
'access-control-allow-origin' => '*',
'accept-ranges' => 'bytes',
] ); however, while most of the servers checked pass the test, the https://github.com/EvaLok/infrastructure-puppet/actions/runs/9103199343/job/25024530795#step:3:23
is it expected that this server will not support the |
@EvaLok Yes, on the origin we use dynamic Gzip, which means we respond with |
- we don't make assertions for `content-length` and `accept-ranges` due to some servers using dynamic gzip ( ref jquery/codeorigin.jquery.com#103 (comment) )
* add "Accept-Encoding" to request headers. * expect content-encoding in response (gzip compression). We don't make assertions for `content-length` and `accept-ranges` due to origin servers using dynamic gzip, ref jquery/codeorigin.jquery.com#103 (comment). Closes jquery/codeorigin.jquery.com#103.
was working on an older site and updated it to load jQuery 1.12.4 via CDN, but pagespeed dev insights says it isn't compressed. i then reviewed response headers and that does appear to be the case. screenshots below:
The text was updated successfully, but these errors were encountered: