jquery / codeorigin.jquery.com

jQuery CDN
https://releases.jquery.com
Other
57 stars 605 forks source link

jquery-1.12.4.min.js on CDN is not compressed #103

Closed EvaLok closed 2 months ago

EvaLok commented 3 months ago

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:

image

image

mgol commented 3 months ago

@Krinkle any ideas? Didn't we clear the cache at one point?

timmywil commented 3 months ago

Looks okay on my machine.

Screenshot 2024-04-25 at 7 35 30 PM

I can't see from the photo, but make sure the Accept-Encoding header is in the request.

EvaLok commented 3 months ago

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> 
mgol commented 3 months ago

@timmywil I see the same issue on my side. I. checked 2.2.4 & 3.7.1 as well and none were compressed. Accept-Encoding: gzip, deflate, br is present in the request headers so the issue is not on my side.

mgol commented 3 months ago

A previous similar issue: https://github.com/jquery/codeorigin.jquery.com/issues/98

mgol commented 3 months ago

From my investigation with Timmy, it looks like advertising Brotli support (which all browsers have done for a long time) is tripping the logic:

$ curl https://code.jquery.com/jquery-3.7.1.min.js --silent --write-out "%{size_download}\n" --output /dev/null 
87533
$ curl https://code.jquery.com/jquery-3.7.1.min.js --silent -H "Accept-Encoding: gzip, deflate" --write-out "%{size_download}\n" --output /dev/null 
30336
$ curl https://code.jquery.com/jquery-3.7.1.min.js --silent -H "Accept-Encoding: gzip, deflate, br" --write-out "%{size_download}\n" --output /dev/null 
87533
timmywil commented 3 months ago

It seems to be more complicated than that, but we have a possible solution.

timmywil commented 3 months ago

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.

EvaLok commented 3 months ago

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.

Krinkle commented 3 months ago

We have a smoke test in the https://github.com/jquery/infrastructure-puppet repository, which includes tests for Code/Codeorigin.

EvaLok commented 3 months ago

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?

https://github.com/jquery/infrastructure-puppet/blob/517510f875be4617792e4ff934437c626a70740a/test/CodeoriginTest.php#L28-L39

Krinkle commented 3 months ago

@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!

EvaLok commented 2 months ago

@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?

Krinkle commented 2 months ago

@EvaLok The linked test suite sends requests to the live service. testHttp takes reqHeaders as third parameter. You could add a case for compressed requests by setting Accept-Encoding header, for example, and then verifying the response for that. You can try it locally by invoking php test/CodeoriginTest.php "https://code.jquery.com" on a clone of the repo.

EvaLok commented 2 months ago

@Krinkle OK - i played around with this a bit, and thought i had a fairly reasonably modification to the test:

https://github.com/EvaLok/infrastructure-puppet/blob/d94418e0ba909a07f0680d15d58f2ad7d20c429d/test/CodeoriginTest.php#L28-L42

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 codeorigin-02.stage.ops.jquery.net server seems to gives an error:

https://github.com/EvaLok/infrastructure-puppet/actions/runs/9103199343/job/25024530795#step:3:23


Run make test
✅ test-codeorigin-next-http
✅ test-codeorigin-prod-http
✅ test-releases
not ok 8 GET https://codeorigin-02.stage.ops.jquery.net/jquery-3.0.0.js > header content-length
  ---
  actual:   null
  expected: "77731"
not ok 14 GET https://codeorigin-02.stage.ops.jquery.net/jquery-3.0.0.js > header accept-ranges
  ---
  actual:   null
  expected: "bytes"
✅ test-codeorigin-next-https
✅ test-codeorigin-prod-https
✅ test-contentorigin-prod
make: *** [Makefile:49: test-codeorigin-stage] Error 1
make: *** Waiting for unfinished jobs....
✅ test-miscweb
✅ test-wpdocs

is it expected that this server will not support the Accept-Encoding request header, and if so, how do you propose to modify the test so that we are able to make this assertion for other servers?

Krinkle commented 2 months ago

@EvaLok Yes, on the origin we use dynamic Gzip, which means we respond with Transfer-Encoding: chunked instead of Content-Length and Accept-Ranges: bytes. I suggest omitting these two header assertions from the new gzip test case as either response is fine.