We should ship both minified and normal versions of both jQuery and jQuery UI. When aggregate JavaScript/CSS is on, it should select the minified versions of those files.

Libraries

  • jQuery
  • jQuery UI

Original report by catch

#1328900: Update to jQuery 1.7 added an un-minified version of jQuery to core to ease debugging during the 8.x cycle. This is a great idea but we need an issue open to make sure that we remember to switch to the minified one prior to release.

Files: 
CommentFileSizeAuthor
#41 1341792-41.patch911.92 KBRobLoach
PASSED: [[SimpleTest]]: [MySQL] 37,260 pass(es).
[ View ]
#39 1341792.patch933.21 KBRobLoach
PASSED: [[SimpleTest]]: [MySQL] 36,995 pass(es).
[ View ]
#32 1341792.patch939.99 KBRobLoach
PASSED: [[SimpleTest]]: [MySQL] 36,991 pass(es).
[ View ]
#29 1341792.patch1023.97 KBRobLoach
FAILED: [[SimpleTest]]: [MySQL] Invalid patch format in 1341792_1.patch.
[ View ]
#28 1341792.patch939.99 KBRobLoach
PASSED: [[SimpleTest]]: [MySQL] 37,004 pass(es).
[ View ]
#26 1341792.patch943.56 KBRobLoach
FAILED: [[SimpleTest]]: [MySQL] Invalid patch format in 1341792.patch.
[ View ]
#17 composer.json_.txt897 bytesRobLoach
#17 ComposerEvents.php_.txt1.72 KBRobLoach

Comments

What about introducing built-in support to minify our (aggregated?) javascripts? This way we could ship with all the core/contrib libraries full versions, thus making development far easier. At the same time we would have yet another performance improvement available for every included javascript.

That's in #119441: Compress JS aggregation, which in turn is postponed on #352951: Make JS & CSS Preprocessing Pluggable, would be good to do that yeah - although the better minifiers now I don't think you can run in PHP (require server side js or similar) so we may still want to ship pre-minified js.

Priority:Critical» Normal
Issue tags:+revisit before beta

We have a tag for that. :)

Issue tags:+mobile

First, core shouldn't automatically minify JS for a couple reasons. Well two problems. Unless we have a way out of them.

  • We have JS libraries included with header block comments containing copyright info. Included libraries can come in core or contrib. There isn't a standard for doing these comments and we need to leave them in even when we minify everything else. How can we handle this?
  • Drupal.js and it's other components are GPL. Downloading JS from the server to the browser is distribution. If we minify we are shipping obfuscated code. We can't do this. Sometimes we need to not minify certain JS files. Other times we can have a comment with a link to the original source. I think this is different depending on the company, country, and lawyer involved in advising around this.

In addition to these two large objections (that I'm really not sure how to get around) there is a difference in the quality of the minifier. JSLint is a rather weak minifier (producing larger files) compared with something like uglifyJS (what jQuery currently uses). I'd like to see us use something smart and strong as opposed to something weak but we can stick it into PHP.

Maybe we should start on some build scripts to automate doing the minification.

Tagging as mobile since the file size issue has mobile implications.

Note, I am not a lawyer. This is not legal advice.

I wanted to make a clarification on bullet 2. If the source JS is already distributed through d.o or a libraries website (like jquery.com) then minified code is fine. Site specific code is where we run into a problem. Implement Drupal.theme to change the look of something in a theme and you now fall under the custom code being distributed based on GPL code problem.

Note, I am not a lawyer. This is not legal advice.

As long as the unminified code is still available, that's probably good enough. Strictly speaking GPLv2 requires you to offer original source on CD, but no one actually does that and GPLv3 does away with it. Download is fine. Legally it's no different than image styles, I'd argue.

Plus, I don't see why a minifier couldn't be programmed to leave the first docblock in a file alone and just strip the rest. That would cover the attribution question.

So legally, I am not all that worried, to be honest. The technical capabilities of different minifiers I cannot speak to.

This is the wrong thread for that discussion. We've been shopping minified jquery.js and jquery_ui for years via officially distributed versions with licencse headers at the top. About six weeks ago we put the unminified versions in instead to assist debugging during 8.x development. Before we release, we'll need to switch them back again.

Having minification itself in core is being discussed elsewhere. Possibly we could mark this issue won't fix in case we ever sort that out properly, but that's not what's proposed here.

Status:Active» Postponed

Definitely have to switch to minified jQuery/jQuery UIs before releases come. When should we do that though? Before alpha? Before beta?

Status:Postponed» Active

Here's the issue for general minification #119441: Compress JS aggregation.

Let's not mark this postponed, it's not waiting on anything except for the release cycle, and yeah we need to discuss when is a good time to do that.

We should ship minified versions of every core JavaScript file. If doing all JS files as minified needs to happen elsewhere I'll open or join an issue for that. We should do it for performance. There's a reason all the libs (like jquery) ship a minified version called the production version. The same reason applies to all our JS.

Right, that's in #119441: Compress JS aggregation, but if that doesn't happen, we should not ship with the unminified jquery.js just because that didn't get dealt with during the release, which is what this issue is for.

After a number of conversations with those around the Drupal community I believe #119441: Compress JS aggregation will fail. There are non-technical issues we have to work out. Because performance is important we ship with the recommended minified version of jquery.js (and some other components like jQuery UI). For the same reason these libraries use minified versions of their libraries in production we should minify all the JS within Drupal for production.

Since doing this will take more thought and work than dropping in a minified version of jquery.js I'm looking to start work on this earlier in the Drupal 8 life cycle. Ideally, it would be nice to backport this to Drupal 7 if possible. If I need to tackle this case elsewhere or we need more justification on why I'd be glad to share any of the details I have.

Because #1085590: Update to jQuery UI 1.9 will make jQuery UI also un-minified, I added a little list to the issue summary so that we don't lose track of which files to minify: link

BTW, might be bikeshedding this way out of proportion, but Assetic has filters to minify/compress JavaScript and CSS.... CssCompressorFilter and JsCompressorFilter allow you to use YUI Compressor. It's also quite easy to extend it to add your own filters.

This issue has absolutely nothing to do with core aggregation or compression. It is about making sure the shipped versions of jquery.js et al. are minified before we release, because (for possibly the first time ever) 8.x currently has un-minified versions to make debugging easier during development.

As mfer points out it's very unlikely we'd want to do that via any possible minification method that could possibly be shipped with core, because none of the better minification methods are in PHP, not to mention the fact that jquery supplies those files for production anyway. Anything else should be discussed in #119441: Compress JS aggregation or one of the several other issues dealing with this.

I'd be fine with opening a separate issue to provide minified versions of Drupal's own javascript (i.e. checked in, not via a runtime pre-processer) (drupal.js etc.) but again, not for this issue.

Ah, I understand. Thanks for explaining the scope of this issue :-) . Definitely things to think about!

StatusFileSize
new1.72 KB
new897 bytes

Composer has the ability to execute certain scripts post-installation. So, I thought it might be a good idea to use that, along with Assetic, to minify/compress all JavaScript/CSS files. Don't really have the time to do up a patch right now, buuuuuut.....

  1. composer.json goes to /composer.json
  2. ComposerEvents.php goes to /core/lib/Drupal/Components/ComposerEvents.php
  3. Once it's set up, install Composer via drush dl composer
  4. Then install the Drupal packages via drush composer install

Assetic will process all JavaScript and CSS for Drupal core and jQuery UI, and save the .min files. Currently it does not compress them, but that's as easy as adding the filters in...

<?php
     
new GlobAsset($coreDir . '/misc/*.css', new CssMinFilter()),
?>

Having a proper folder for our javascript would help. core/js/src with unminified JS (including jquery/ui for debug) and core/js/min for the processed files. Core modules can use the same paradigm, js/src folder and be picked up by the build script. And the build script in core/scripts to minify all our things.

For contrib modules what Rob proposed that would be a good follow-up for the pluggable css and js processing.

We can't simply minify all the JS and CSS automatically. I wish it were that simple. For example, if we automate the minification jquery.ba-bbq.js will have the second copyright/license notice removed. This is not a good thing.

Note, for anyone not familiar with the situation, JavaScript and CSS sent from the server to the browser is distributed. We need to send the copyright and license info even for minified files. We need a process that makes sure this is properly handled.

That said, if we manually handle the external libraries and automate the generation of JS for our core stuff we should be in good shape.

We should also minify module JavaScript as well.

I would suggest using UglifyJS which Assetic can support.

Well if we're allowed to add a little ! to third party libs. in the comments (as in /**!) then yuicompressor won't delete the comment block. I'm just hoping other minification tools can work with something like that. It might be an option and an encouraged practice if that's the case.

But yes, that's something to pay attention to.

@_nod YUI compressor, Google Closure Compiler, and UglifyJS each have their own way of handling comments you want to save. UglifyJS generates the smallest minified files and does so the fastest. There is no standard for handling comments you want to save. We would either A) have to modify the libraries we include when we include them (and update them), B) Get the library authors to change their libraries and keep them our way or C) Manually manage the minified files.

I just want to make sure this is handled well up front rather than forgotten or a bolt on after thought. Since this has to do with licenses it can affect all of the big Drupal users who have lawyers who make sure everything is handled well.

Actually I thought about it and wanted to take the same approach as the libraries modules does for versions. Provide a callback in php that'll parse the JS file to look for a license block or a hardcoded license in PHP when declaring the js lib. That's pretty much the only way I can see that working programatically.

That way we can minify the file, ask the library registry for the license block of this script and add it to the minified file.

There are some handy little tools in the JavaScript/Node world that we could use:

  • npm is a package manager, written in node/JavaScript. Most JavaScript libraries are using it now (you'll see package.json everywhere)
  • grunt.js is a build tool for node/JavaScript which allows you to do things like concatenate and compress files with Uglify. Scripts themselves tell grunt.js how to build itself.

Now, I don't mean commit these to the Drupal git repository, I mean we provide our own package.json and grunt.js scripts that instruct these tools to build/compress all of our JavaScript/CSS for us. Once they're built, we commit the built files to Drupal core. Yes, it means that whenever we make changes, to the JavaScript/CSS, we'd have to re-compile it, but that's to be expected.

npm install
node node_modules/.bin/grunt

This also means using AMD and require.js as we'd need a way to reference the newly built files.

In the PHP world, the two equivalent tools would translate to Composer (package manager) and Assetic (asset handler). But JavaScript libraries use JavaScript tools, "when in Rome".

If making the full source .js available along with the minified version removes any legal implications, then why don't we simply ship both minified and full versions of all js libraries we use? We should set a naming convention/standard for all pairs filenames: either *.min.js for minified versions and *.js for full source or we could alternatively keep full source in /src subdirectory(ies) as suggested in previous comments here.

If we did something like that, then I see this procedure for updating js files:

- test everything with full source in order to ease debugging during development
- once we're ready to commit changes, minify the js files that were updated making sure any license comment blocks are left intact for legal reasons.
- make the final commit with both updated files for each library (minified and full source) as a pair

The only implication I see in this approach is that we'd have to update two files each time we make changes to a single library, but that wouldn't be much of an issue since most vendors provide both minified and full versions.

Perhaps shipping with both full and minified versions could also make it possible to provide the means so that site admins could easily switch between dev and production state (with a config setting similar to the offline mode). Production state would use the *.min.js files while dev would switch to using the *.js ones (or the ones under the /src dir if we chose to go that way).

PS: If I'm off-topic here, then which would be the most appropriate issue to discuss this? File a new one perhaps?

Having both seems appropriate. jQuery and jQuery UI use npm and grunt for building/compressing themselves, which gives us jquery/dist and jquery-ui/dist folders with the .min files. So, that seems like a thing we could do prior to releases. Maybe have a "min" entry in hook_library?

[EDIT] @droplet mentioned...... run "grunt release" to replace @VERSION tags :-) .

Status:Active» Needs review
StatusFileSize
new943.56 KB
FAILED: [[SimpleTest]]: [MySQL] Invalid patch format in 1341792.patch.
[ View ]

This patch....

  • Is large because it includes minified versions of both jQuery and jQuery UI
  • Switches to the minified versions when JavaScript preprocessing is enabled
  • jQuery UI provided from the built jquery-ui/dist/jquery-ui-1.9.0pre directory (.gitignore is there to avoid adding files we don't need)
  • We should have a follow up to extend this to CSS

Status:Needs review» Needs work

The last submitted patch, 1341792.patch, failed testing.

Status:Needs work» Needs review
StatusFileSize
new939.99 KB
PASSED: [[SimpleTest]]: [MySQL] 37,004 pass(es).
[ View ]

Hmm?

StatusFileSize
new1023.97 KB
FAILED: [[SimpleTest]]: [MySQL] Invalid patch format in 1341792_1.patch.
[ View ]

It might be i18n.

Status:Needs review» Needs work

The last submitted patch, 1341792.patch, failed testing.

I really like the idea of having both the original + minified files readily at hand, and making Drupal use the desired variant.

The approach of switching between variants in these patches, however, looks "improvable" to me ;) I also think we should perform the update/addition of libraries with multiple variants in a separate issue + commit.

Status:Needs work» Needs review
StatusFileSize
new939.99 KB
PASSED: [[SimpleTest]]: [MySQL] 36,991 pass(es).
[ View ]

I also think we should perform the update/addition of libraries with multiple variants in a separate issue + commit.

There is only a .js and a .min.js for these. It uses the 'data' tag to switch it out. Do you have a better idea on how to switch it out? Maybe add the "data" tag in system_library_info_alter()?

It's not directly related but any chances we get to have a core/js/jquery/{src,min} (or replace "min" with "dist") as well as mymodule/js/{src,min} a bit like psr-0 for JS :p

Title:Minify jquery.js (et al) prior to releaseShip minified versions of jQuery and jQuery UI
Issue tags:+JavaScript

Re-purposing the title.

Issue summary:View changes

Added a list of libraries to update for when we add more.

Logical POV:
Since Drupal doesn't have the concept of a "build process" built-in, I'd rather not see Drupal ship with both original and minified JS files. After all, it requires humans to do a manual build process. Further: it implies that this is a best practice that contrib should do as well.
We'd essentially be bundling the same stuff twice with each download. It makes more sense to fix JS minification in core.

Practical POV:
Mostly due to legal reasons, JS minification is a very hard problem to solve. So we should go with the most practical solution for now.

HOWEVER, an interesting comment was made at #119441-149: Compress JS aggregation back in March, but nobody seems to have noticed it. In essence: a proposal by GNU to enable minification without violating licenses: http://www.gnu.org/licenses/javascript-labels.html. Maybe that makes JS minification in core viable again? :)

Since Drupal doesn't have the concept of a "build process" built-in, I'd rather not see Drupal ship with both original and minified JS files.

That's actually why we want to ship both. If there was a Drupal build process, then we could use it, along with npm/grunt to build these files.

Further: it implies that this is a best practice that contrib should do as well.

Why not... Contrib can do it, or not do it. It's up to them. There's no way we could bring CDN module into Drupal Core. We need projects like that to exist in the contrib space so that they can accomplish all that they do.

Mostly due to legal reasons, JS minification is a very hard problem to solve. So we should go with the most practical solution for now.

The .min files have their licence information provided with it, and the JavaScript projects actually recommend using these files. Building with npm/grunt leaves those doc blocks in.

Status:Needs review» Needs work

My "Practical POV" line was meant to mean "it may not be ideal, but the solution proposed here is the best one we've got, so let's run with it!".
So #36 was actually a vote to support the direction taken in #24–#35 :) Sorry for not making that more clear!

The one question I still had/have is whether the JavaScript license web labels thing could be helpful. You've answered that over at #119441-152: Compress JS aggregation.

Hence: sorry for the interruption! To make up for it, I reviewed the patch in #32.

+++ b/core/modules/system/system.moduleundefined
@@ -1822,6 +1822,46 @@ function system_library_info() {
+    $libraries['jquery']['js']['core/misc/jquery.js']['data'] = 'core/misc/jquery.min.js';

Can't we iterate over $libraries and alter the path of each file in a consistent manner? That's more understandable than this wall of text and guarantees that our directory structures are and remain consistent.

Status:Needs work» Needs review
StatusFileSize
new933.21 KB
PASSED: [[SimpleTest]]: [MySQL] 36,995 pass(es).
[ View ]

Ah, sorry about the confusion :-) .

Can't we iterate over $libraries and alter the path of each file in a consistent manner?

Great idea! This looks much nicer. I'd rather not do string alter operations though, let's keep it fast.

un-minified version of jQuery to core to ease debugging

Seems like this is the only reason to include source code.

In case if Drupal package grows up too big. It can suggest to use Source Map for debugging. Packaged all these stuff as a developer package.

Developer package:
- jQuery Soruce, 3rd party script source
- #1532612: Move Examples Project into Drupal core
- API Docs
...
(sorry, slightly off-topic)

StatusFileSize
new911.92 KB
PASSED: [[SimpleTest]]: [MySQL] 37,260 pass(es).
[ View ]

#41: 1341792-41.patch queued for re-testing.

Issue tags:-revisit before beta

Doesn't have to be done right before the release. Can be done before then.

Issue tags:+revisit before beta

Well, the thing is, we *definitely* want to do this before release. So if we remove the tag, this should be marked critical. I would be fine with that, but for now, re-adding tag.

I'd like to see the unminified version of jQuery (and jQuery UI, etc) even in the final release. This will help a lot to developers who do heavy-js sites on Drupal. Even if we don't compress JS on-the-fly, we can ship both versions and put an option on Performance page to use full or minified versions of libraries.

we might want to come up with a more standard way of dealing with minified files, maybe brainstorm that in #1663622: Change directory structure for JavaScript files ?

@droplet just tested out that source mapping thing and it's really spiffy. The potential for coffeescript or, god forbid, php-snow *shudder* is pretty interesting too :)

While it would definitely allow people to do much of the everyday debugging using the minified JS, it would still leave the old browsers high and dry if we didn't ship with uncompressed files.

And +1 for considering package managers for js libs as well!

#41: 1341792-41.patch queued for re-testing.

There is an issue to add a development/production toggle to Drupal intended to specify if production or dev JavaScript should be used. This could be used for other things as well. #1537198: Add a Production/Development Toggle To Core

Ideally all of the core JavaScript would be minified and you could choose to send the dev or production versions. Contrib could even loop into this.

The Speedy Module already has scripts to do the minification and the toggling. Though, it might be better to use a tool like grunt than drush.

Status:Needs review» Postponed

We're not shipping yet :)

Issue summary:View changes

Updated issue summary.

Priority:Normal» Critical
Issue summary:View changes
Status:Postponed» Active
Issue tags:-revisit before beta+beta target

Bumping this. Needs to be done in an early beta if not before.

Shipping both an allowing a switch is fine, but let's mark this duplicate if and when that happens.