Skip to content

Conversation

@rullzer
Copy link
Member

@rullzer rullzer commented Mar 26, 2017

Since in production the SCSS files are compiled once and the javascript
files are combined once we can just as well gzip them aggresively.

This means that once they are requested and the browser supports gzip we
can just serve the gzipped file saving precious bandwidth.

TODO:

  • Extend tests

@rullzer rullzer added the 2. developing Work in progress label Mar 26, 2017
@mention-bot
Copy link

@rullzer, thanks for your PR! By analyzing the history of the files in this pull request, we identified @skjnldsv, @LukasReschke and @juliushaertl to be potential reviewers.

.htaccess Outdated
#### DO NOT CHANGE ANYTHING ABOVE THIS LINE ####

ErrorDocument 403 //core/templates/403.php
ErrorDocument 404 //core/templates/404.php
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🙈

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

fixed 🙈

Since in production the SCSS files are compiled once and the javascript
files are combined once we can just as well gzip them aggresively.

This means that once they are requested and the browser supports gzip we
can just serve the gzipped file saving precious bandwidth.

Signed-off-by: Roeland Jago Douma <roeland@famdouma.nl>
rullzer added 2 commits March 28, 2017 23:13
Signed-off-by: Roeland Jago Douma <roeland@famdouma.nl>
Signed-off-by: Roeland Jago Douma <roeland@famdouma.nl>
@codecov-io
Copy link

Codecov Report

Merging #4070 into master will increase coverage by 0.01%.
The diff coverage is 90%.

@@             Coverage Diff              @@
##             master    #4070      +/-   ##
============================================
+ Coverage      54.2%   54.22%   +0.01%     
- Complexity    21294    21310      +16     
============================================
  Files          1310     1310              
  Lines         81253    81293      +40     
  Branches       1285     1285              
============================================
+ Hits          44043    44079      +36     
- Misses        37210    37214       +4
Impacted Files Coverage Δ Complexity Δ
lib/private/Template/JSCombiner.php 89.41% <80%> (-0.59%) 23 <0> (+1)
lib/private/Template/SCSSCacher.php 76.59% <85.71%> (+0.45%) 22 <0> (+1) ⬆️
core/Controller/JsController.php 93.33% <92.85%> (-1.12%) 8 <4> (+5)
core/Controller/CssController.php 93.33% <92.85%> (-1.12%) 8 <4> (+5)
lib/private/Files/Cache/Cache.php 82.85% <0%> (-0.35%) 103% <0%> (+4%)
apps/files_sharing/lib/SharedStorage.php 76.66% <0%> (ø) 90% <0%> (ø) ⬇️
core/templates/layout.base.php 0% <0%> (ø) 0% <0%> (ø) ⬇️
core/templates/layout.user.php 0% <0%> (ø) 0% <0%> (ø) ⬇️
core/templates/layout.guest.php 0% <0%> (ø) 0% <0%> (ø) ⬇️
apps/files_sharing/lib/Cache.php 90% <0%> (+0.2%) 20% <0%> (ø) ⬇️
... and 1 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update e26f138...3a0ef65. Read the comment docs.

@rullzer rullzer changed the title [PoC] Allow to gzip CSS/JS files GZip generated CSS/JS files Mar 28, 2017
@rullzer rullzer added 3. to review Waiting for reviews enhancement and removed 2. developing Work in progress labels Mar 28, 2017
@rullzer rullzer added this to the Nextcloud 12.0 milestone Mar 28, 2017
@rullzer rullzer requested a review from LukasReschke March 28, 2017 22:22
@rullzer
Copy link
Member Author

rullzer commented Mar 28, 2017

This is ready for review 😄

Copy link
Member

@MorrisJobke MorrisJobke left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Tested and works 👍

@MorrisJobke MorrisJobke merged commit 16b8c0c into master Mar 28, 2017
@MorrisJobke MorrisJobke deleted the gzip_scss_js branch March 28, 2017 23:11
@MorrisJobke
Copy link
Member

Followup to this one for Safari: #4132

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

3. to review Waiting for reviews enhancement

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants