Edgewall Software
Modify

Opened 7 years ago

Last modified 3 years ago

#10672 new enhancement

Automatically minify Javascript and CSS

Reported by: Peter Suter Owned by:
Priority: normal Milestone: next-dev-1.3.x
Component: general Version:
Severity: normal Keywords: javascript css performance
Cc: Ryan J Ollos, leho@…, olemis+trac@…, felix.schwarz@… Branch:
Release Notes:
API Changes:

Description (last modified by Peter Suter)

As discussed previously it might be a good idea to introduce a minification step that pre-processes the JavaScript and CSS files stored in source control.

This would allow developing with clean, well-commented source files while improving performance. We could then switch our JavaScript coding style to 4 spaces indentation and add copyright and documentation comments to file headers. (See #10802)

The step / a related step could do JSLint checking.

How should this be implemented?

  • One suggestion was a make minify step.
  • Another idea would be to add it to setup.py. minify or distwebres provide setuptools commands (both use YUI compressor, which requires Java)
  • Alternatively, we could hook it into trac-admin deploy somehow.
  • Or the web frontend could dynamically minify and cache and combine just the files required for a request into one file. (Including scripts by plugins.)

An important requirement might be to keep this step optional.

Attachments (4)

0001-trac-admin-deploy-gains-minify-support-10672.patch (5.3 KB ) - added by Christian Boos 7 years ago.
implementing the suggestion of minifying during deployment
T10672-IDeployTransformer.patch (12.1 KB ) - added by Peter Suter 5 years ago.
Adapt cboos yuicompressor code to optional component and add uglify-js and clean-css alternatives.
T10672-ClosureCompilerDeployTransformer.patch (5.8 KB ) - added by Peter Suter 5 years ago.
T10672-online-minifiers.diff (3.6 KB ) - added by Peter Suter 5 years ago.

Download all attachments as: .zip

Change History (38)

comment:1 by Christian Boos, 7 years ago

Good idea! I had a run of the YUI compressor and indeed, this cuts the size of our .js by a half. The .css could also be shrinked down, though the savings are less important.

.js:

File Original size Minified size % of the original
auto_preview.js 4952 1650 33%
babel.js 4747 1406 29%
blame.js 3749 1474 39%
diff.js 4713 2361 50%
expand_dir.js 6321 2542 40%
folding.js 3022 1260 41%
keyboard_nav.js 2819 1230 43%
log_graph.js 1223 649 53%
noconflict.js 228 20 8%
query.js 18270 8405 46%
resizer.js 968 585 60%
search.js 3233 1674 51%
suggest.js 4257 2018 47%
threaded_comments.js 3336 1752 52%
timeline_multirepos.js 267 157 58%
trac.js 3703 2067 55%
wikitoolbar.js 3101 1708 55%
workflow_graph.js 8192 4236 51%

For the messages/*.js, the savings are marginal, between 93% and 97% of the original.

.css:

File Original size Minified size % of the original
about.css 767 667 86%
admin.css 3114 2575 82%
browser.css 6698 5285 78%
changeset.css 942 776 82%
code.css 4672 3855 82%
diff.css 5031 4068 80%
jquery-ui-addons.css 563 428 76%
prefs.css 989 869 87%
report.css 7990 6717 84%
roadmap.css 2901 2356 81%
search.css 605 531 87%
ticket.css 4715 4089 86%
timeline.css 2947 2332 79%
trac.css 18764 15417 82%
wiki.css 3021 2352 77%

As for the tool, minify seems to be the most convenient, as when you easy_install it, it also downloads the YUI compressor. distwebres can't (yet?) be easy_installed.

Also, I think the best solution would be to do this minification step at deploy time, as this will enable us to minify the resources from plugins as well. The .css and .js files would be minified before being copied in to the deploy location (in _do_deploy).

For the last step you suggested, minify and combine on the fly, I'm not sure it's worth the effort, as going through Python for serving .js and .css, no matter what optimization we do at that level, is probably less efficient than letting directly the front-end serve them. And the front-end itself might be able to do such things and more (e.g. mod_pagespeed).

by Christian Boos, 7 years ago

implementing the suggestion of minifying during deployment

in reply to:  1 comment:2 by Christian Boos, 7 years ago

Replying to cboos:

As for the tool, minify seems to be the most convenient, as when you easy_install it, it also downloads the YUI compressor. distwebres can't (yet?) be easy_installed.

Well, as I quickly found out when implementing the above idea, pipy:minify is just a setuptools wrapper for pipy:yuicompressor. If we don't do the minification via setup.py but rather at deploy time, only the latter is necessary (and only to the extent it simplifies the setup, as then it's only a matter of doing an easy_install yuicompressor; we could consider the alternative of asking people to specify the path to their yuicompressor-...jar but people are lazy, downloading, unpacking the YUI compressor and specifying the path is probably goinf to be overkill ;-) ).

comment:3 by Peter Suter, 7 years ago

Very nice!

Two small suggestions:

  • Print some informative message if minification fails, like "Minification of %(filename)s failed. easy_install yuicompressor and java in path are required."
  • yuicompress_all_but_jquery: Is this because jQuery is already minified? Would it make sense to detect a -min suffix instead of a jquery prefix?

in reply to:  3 comment:4 by Christian Boos, 7 years ago

Replying to psuter:

Very nice!

Thanks!

Two small suggestions:

  • Print some informative message if minification fails, like "Minification of %(filename)s failed. easy_install yuicompressor and java in path are required."

Ok, I agree some error reporting can be useful. Added this and also improved the command help, see [f187f18d/cboos.git] and previous.

  • yuicompress_all_but_jquery: Is this because jQuery is already minified? Would it make sense to detect a -min suffix instead of a jquery prefix?

Yes, this is also a good idea: some jquery... files could benefit from minification, and it's nice to be able to tell plugin authors "if you don't want a specific resources to be minified, make sure it ends up with .min.{css,js}". Only problem was for jquery-ui.css, which is not minified (so I didn't want to add .min to the name) but nevertheless should be excluded (as it triggers an error). I now explicitly blacklist that one.

comment:5 by Peter Suter, 7 years ago

Tiny correction:

-log("Package yuicompress is not installed")
+log("Package yuicompressor is not installed")

comment:6 by Christian Boos, 7 years ago

On second thought, I'll rework this to make it an optional component, it's not appropriate to make the core refer to one particular external package. The only drawback is that I think we'll lose the possibility to have a --minify parameter to deploy and instead we'll have to rely on the component being enabled or not.

comment:7 by Peter Suter, 7 years ago

Description: modified (diff)

Subsequent cleanup task extracted to #10802.

comment:8 by Christian Boos, 7 years ago

Keywords: javascript css added

comment:9 by Ryan J Ollos <ryan.j.ollos@…>, 6 years ago

Cc: ryan.j.ollos@… added

comment:10 by anonymous, 6 years ago

redirected from #11024 IMO, nothing can be compared to node.js' uglify-js and clean-css.

http://lisperator.net/uglifyjs/#stats

And they are already cross platform. so the only new requirement would be for one to install node.js for the OS.

We can have a package.json with the needed dependencies so then the user would need to do npm install and the dependencies would be retrieved for them.

comment:11 by Christian Boos, 6 years ago

Yes, that's why I didn't commit my initial approach, I wanted to make it possible to use different tools for that task.

comment:12 by Remy Blank, 6 years ago

I wouldn't be all too happy to add node.js as a required dependency for Trac. According to user feedback, Trac is already complicated enough to install. Having to install yet another "package manager" just for node.js doesn't strike me as a good idea.

comment:13 by anonymous, 6 years ago

node.js is the standard nowadays for web development. I hate dependencies myself but this is the optimal way IMO.

I would be happy if any way was used to minify and merge the CSS and JS files. Because at this point the problem is the size and the number of files.

comment:14 by Christian Boos, 6 years ago

Well, in a way installing node.js (which comes with its package manager npm) is somewhat less resource demanding than the alternative yuicompressor, which requires … Java. Not all Linux server platforms have it installed by default (for example, t.e.o does not).

Choice is good and neither minification tool should be required anyway.

comment:15 by anonymous, 6 years ago

I guess a simple solution for reducing the requests would be to just merge the files when deploying. Like doing cat 1.js 2.js>javascript.js

comment:16 by anonymous, 6 years ago

BTW, there's definitely something wrong in v1.0 css or js. After I minified the CSS and JS files, the /log layout stopped working like before. Unfortunately I cannot provide more details, but I can attach the minified files.

comment:17 by anonymous, 6 years ago

LOL @ node.js

and what's wrong with minify and it's deployment of chained files ?

comment:18 by Ryan J Ollos, 5 years ago

Cc: Ryan J Ollos added; ryan.j.ollos@… removed

comment:19 by lkraav <leho@…>, 5 years ago

Cc: leho@… added

comment:20 by Olemis Lang <olemis+trac@…>, 5 years ago

Cc: olemis+trac@… added

by Peter Suter, 5 years ago

Adapt cboos yuicompressor code to optional component and add uglify-js and clean-css alternatives.

in reply to:  6 comment:21 by Peter Suter, 5 years ago

Replying to cboos:

make it an optional component, it's not appropriate to make the core refer to one particular external package.

In this patch I define a new extension point IDeployTransformer and adapt your code to create a new optional component tracopt.env.yuicompressor.

Replying to anonymous:

nothing can be compared to node.js' uglify-js and clean-css.

Alternatively, the patch also contains tracopt.env.uglifyjs and tracopt.env.clean-css for people that prefer NodeJS to Java.

(The YUI compressor seems to compress slightly better though.)

comment:22 by Peter Suter, 5 years ago

Some points I'm unsure about:

  • Should trac.env.IDeployTransformer:
    • Be called something else? (E.g. IStaticResourcePreprocessor, IDeploymentParticipant, …)
    • Be located in another module? (E.g. trac.admin.api, trac.web.chrome, …)
  • Is that interface at the right abstraction level? Is it general enough to be useful for other use cases?
    • PNG optimization (#11024) was mentioned, and seems like it would be no problem.
    • Other vaguely similar preprocessing steps:
      • Javascript compilation from Coffeescript, Typescript, Clojurescript, Dart, … (#10735)
      • CSS compilation from LESS, SASS, … (#633)
      • But these are usually not optional. So I am not sure the (optional) deploy step is the right place for these. (Unless the default is to interpret the unpreprocessed files in the browser. But even then it's unclear how references to the unpreprocessed files would be overridden if the optional preprocessed files are created.)
      • These would probably want to change the extension of the destination filename. (I can't see anything that would prevent that at the moment, although it is currently not obvious from the interface documentation if that is allowed.)
    • A similar problem comes up if we want to merge / concatenate files.
  • Should the tracopt.env.* modules:
    • Be called something else? (YUI vs. Yui; is the ...DeployTransformer suffix a good idea? etc.)
    • Be located in another package? (E.g. tracopt.admin.deploy.*, …)
    • Have different module names?
      • Especially as tracopt.env.yuicompressor currently requires the from __future__ import absolute_import directive to import its external dependency yuicompressor.
  • Should the Java, NodeJS and NPM package dependencies be declared in setup.py somehow?
  • Are these three minifiers (YUI Compressor, UglifyJS and Clean-CSS) the ones we want to include?
    • More?
    • Less?

Any ideas or other concerns?

comment:23 by Peter Suter, 5 years ago

For minifying Javascript we previously used Google Closure Compiler. Attached is a IDeployTransformer for the online REST API, but there's a rate limit:

Too many compiles performed recently. Try again later.

Oops.

Also it refuses to compile diff.js:

Parse error. IE8 (and below) will parse trailing commas in array and object literals incorrectly. If you are targeting newer versions of JS, set the appropriate language_in option.

comment:24 by Peter Suter, 5 years ago

Milestone: next-major-releases1.1.2

comment:25 by Peter Suter, 5 years ago

Owner: set to Peter Suter
Status: newassigned

comment:26 by Peter Suter, 5 years ago

For comparison here are the results of the various Javascript transformers:

File Original Size YUI % UglifyJS % Closure WS % Closure Simple % Closure Advanced %
auto_preview.js 5080 1655 33% 2100 41% 2261 44% 1495 29% 970 19%
babel.js 6923 2249 32% 2859 41% 2974 43% 2135 31% 1750 25%
blame.js 3749 1474 39% 1918 51% 1999 53% 1363 36% 1258 34%
diff.js 4703 2360 50% 3072 65% 3222 69% 2188 47% 2055 44%
expand_dir.js 6277 2528 40% 3155 50% 3227 51% 2475 39% 2190 35%
folding.js 2973 1232 41% 1470 49% 1511 51% 1186 40% 1064 36%
keyboard_nav.js 2817 1230 44% 1714 61% 1772 63% 1126 40% 991 35%
log_graph.js 1211 649 54% 820 68% 845 70% 618 51% 536 44%
noconflict.js 228 20 9% 20 9% 20 9% 20 9% 11 5%
query.js 19653 8964 46% 11719 60% 12019 61% 8586 44% 7921 40%
resizer.js 952 585 61% 696 73% 713 75% 564 59% 521 55%
search.js 3225 1674 52% 2128 66% 2178 68% 1607 50% 1513 47%
suggest.js 4459 2018 45% 2557 57% 2632 59% 1928 43% 1787 40%
threaded_comments.js 3414 1830 54% 2122 62% 2201 64% 1738 51% 1531 45%
timeline_multirepos.js 266 157 59% 231 87% 233 88% 153 58% 149 56%
trac.js 5377 2820 52% 3259 61% 3411 63% 2684 50% 2260 42%
wikitoolbar.js 3087 1708 55% 2176 70% 2247 73% 1644 53% 1590 52%
workflow_graph.js 8218 4278 52% 5591 68% 5734 70% 3936 48% 3692 45%

The Closure Compiler has three optimization levels (Whitespace only, Simple and Advanced). YUI outperforms UglifyJS. Only Closure Compiler with Advanced optimizations achieves better rates.

For the CSS CleanCSS and YUI perform very similarly:

File Original Size CleanCSS % YUI %
about.css 949 824 87% 822 87%
admin.css 4156 3423 82% 3448 83%
browser.css 6639 5248 79% 5269 79%
changeset.css 590 482 82% 482 82%
code.css 4741 3896 82% 3915 83%
diff.css 5354 4359 81% 4375 82%
prefs.css 1062 921 87% 926 87%
report.css 8378 7000 84% 7050 84%
roadmap.css 2907 2379 82% 2370 82%
search.css 610 535 88% 536 88%
ticket.css 6000 5040 84% 5049 84%
timeline.cs 3066 2394 78% 2449 80%
trac.css 23223 18579 80% 18661 80%
wiki.css 3278 2566 78% 2570 78%

(For CleanCSS I had to add another flag: --skip-rebase.)

The online services http://cssminifier.com/ (CleanCSS) and http://javascript-minifier.com/ (UglifyJS) might also be interesting. (Although I guess there are too many alternatives to try all of them.)

Last edited 5 years ago by Peter Suter (previous) (diff)

by Peter Suter, 5 years ago

comment:27 by Peter Suter, 5 years ago

The results of these two online services are actually fairly competitive (very similar for CSS, slightly better than YUI for JS).

I'm actually tempted to only include that last simple implementation using these two online services in tracopt, and leave the others for external plugins.

comment:28 by Ryan J Ollos, 5 years ago

#11564 was closed as a duplicate, assuming that the CSS will be combined to a single file and made available on every page at the conclusion of this ticket. #11564 is requesting that wiki.css be available in the browser realm.

comment:29 by Peter Suter, 5 years ago

Milestone: 1.1.2next-dev-1.1.x
Owner: Peter Suter removed
Status: assignednew

Still not sure what the right approach is. Maybe no complexity should be added to Trac as you can always run your favorite tool on the deployed files yourself.

comment:30 by lkraav <leho@…>, 5 years ago

For WordPress / PHP, these guys know what they're doing with https://github.com/x-team/wp-dependency-minification. Perhaps poking around a bit to see how and what will help clarify things?

in reply to:  29 comment:31 by Ryan J Ollos, 5 years ago

Replying to psuter:

Maybe no complexity should be added to Trac as you can always run your favorite tool on the deployed files yourself.

The implicit optimization would be valuable to end users, many of whom already find Trac to be difficult to install and configure without needing to carry out an additional step of minifying the JS and CSS, even if it's not required and only an optimization. Once we expect the JS and CSS to be minified in a deployed application we can do all those things you mentioned in comment:description. That is enough to convince me.

I tested the code a few months back and it seemed good. I'll do some more testing once 1.1.2 is released and try to give feedback on comment:22.

comment:32 by figaro, 4 years ago

Keywords: performance added

Added keyword 'performance'.

comment:33 by Felix Schwarz, 4 years ago

Cc: felix.schwarz@… added

comment:34 by Ryan J Ollos, 3 years ago

Milestone: next-dev-1.1.xnext-dev-1.3.x

Modify Ticket

Change Properties
Set your email in Preferences
Action
as new The ticket will remain with no owner.
The ticket will be disowned.
as The resolution will be set.
The owner will be changed from (none) to anonymous.

Add Comment


E-mail address and name can be saved in the Preferences .
 
Note: See TracTickets for help on using tickets.