We should ship both minified versions of every third party JavaScript library in core.

This will probably be done as a part of #2203431: [meta] Various asset (JavaScript) libraries have to be updated to a (minified) stable release prior to 8.0.0.

Libraries

  • jQuery
  • jQuery UI

Proposed solution

Go through third party libraries added in core.
Find a minified version of the library matching each version
Post a patch to switch.

Potential follow-up

Include both minified and un-minified versions in core, and a switch to change between them.

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

plach’s picture

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.

catch’s picture

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.

sun’s picture

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

We have a tag for that. :)

mfer’s picture

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.

mfer’s picture

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.

Crell’s picture

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.

catch’s picture

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.

RobLoach’s picture

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?

catch’s picture

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.

mfer’s picture

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.

catch’s picture

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.

mfer’s picture

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.

tstoeckler’s picture

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

RobLoach’s picture

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.

catch’s picture

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.

RobLoach’s picture

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

RobLoach’s picture

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()),
?>
nod_’s picture

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.

mfer’s picture

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.

nod_’s picture

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.

mfer’s picture

@_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.

nod_’s picture

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.

RobLoach’s picture

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".

klonos’s picture

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?

RobLoach’s picture

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 :-) .

RobLoach’s picture

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.

RobLoach’s picture

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

Hmm?

RobLoach’s picture

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.

sun’s picture

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.

RobLoach’s picture

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()?

nod_’s picture

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

RobLoach’s picture

RobLoach’s picture

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

Re-purposing the title.

RobLoach’s picture

Issue summary:View changes

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

Wim Leers’s picture

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? :)

RobLoach’s picture

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.

Wim Leers’s picture

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.

RobLoach’s picture

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.

droplet’s picture

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)

RobLoach’s picture

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

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

RobLoach’s picture

Issue tags:-revisit before beta

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

tstoeckler’s picture

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.

danillonunes’s picture

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.

nod_’s picture

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 ?

patcon’s picture

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

RobLoach’s picture

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

mfer’s picture

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.

nod_’s picture

Status:Needs review» Postponed

We're not shipping yet :)

nod_’s picture

Issue summary:View changes

Updated issue summary.

catch’s picture

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.

catch’s picture

Title:Ship minified versions of jQuery and jQuery UI» Ship minified versions of external JavaScript libraries
moshe weitzman’s picture

We got [ #2276219] and #2258313: Implement JS Web License Labels to allow for JS minification is nearly in. We are getting closer to minification in core which would make this issue moot. Please help!

nod_’s picture

Not necessarily moot. UglifyJS will be better than any PHP-based minifier we'll find so having minified files still makes sense.

catch’s picture

Issue summary:View changes
xjm’s picture

Issue tags:+Triaged D8 critical
catch’s picture

Title:Ship minified versions of external JavaScript libraries» [meta] Ship minified versions of external JavaScript libraries
Status:Active» Postponed
droplet’s picture

can anyone update the summary ?

Ship "both minified and un-minified versions" of all 3rd party scripts

Or

Both minified and un-minified versions of jQuery & jQuery UI only, and minified version of all other 3rd party scripts.

Or

minified versions of all 3rd party scripts

nod_’s picture

I would go with minified version of all 3rd party scripts. I hope that will discourage people from hacking into them instead of replacing them properly. I don't see a use case for unminified in a stable core version.

catch’s picture

Issue summary:View changes
catch’s picture

Issue summary:View changes
Crell’s picture

nod_ The use case for including the unminified versions is debugging and development. If I'm writing code against one of those 3rd party libraries and something wonky is happening, I want to be able to step through the code in my debugger and see where it's going nutty. If the entirety of those libraries is one long line then as soon as I hit a line of code in that library my debugging is no longer useful, which sucks. :-)

That said, I could see the argument for devel providing the unminimized versions and having a toggle to enable them, assuming we make it really really easy for devel to do so. (Which would be a good smoke test to make sure the JS API is as clean as we want it to be.)

catch’s picture

Yes either devel could do that or we could add unminified versions and a toggle in a minor release, but no reason for that to block 8.0.0.

achton’s picture

Won't source maps help with that situation? See comment from droplet in #40.

webchick’s picture

This issue was marked as triaged back in November, but without a comment explaining why. IMO, this issue is a direct duplicate of #2203431: [meta] Various asset (JavaScript) libraries have to be updated to a (minified) stable release prior to 8.0.0, and we only need to amend the issue summary of that one to say "while you're upgrading external libraries, move to the minified versions." This has already been happening on all the library update patches I've seen lately.

It makes sense that we can't ship D8 with bloated source files being served by default, so I agree with a "minify them all" approach. The ability to toggle between minified and unminified (also suggested in the issue summary) is not something we need to block shipping D8 on. That sounds like a nice-to-have feature, and probably for Devel rather than core.

catch’s picture

I don't think it's a direct duplicate - if we have any non-out-of-date libraries they'll still need swapping to the minified versions but won't be caught by #2203431: [meta] Various asset (JavaScript) libraries have to be updated to a (minified) stable release prior to 8.0.0.

webchick’s picture

Ah, yep that makes sense. Thanks!

cilefen’s picture

As mentioned in #1341792-64: [meta] Ship minified versions of external JavaScript libraries and #1341792-40: [meta] Ship minified versions of external JavaScript libraries, source maps are the answer here. With them, you get to use a minified files, but still see un-minified when you debug. This is a little more work. For example, some 3rd party libraries may not provide maps and we would have to create them. But, it is the way things are going and it eliminates the need for a system-level switch to un-minified for debugging.

I agree with @webchick's comment in #65. This is a duplicate of #2203431: [meta] Various asset (JavaScript) libraries have to be updated to a (minified) stable release prior to 8.0.0 if we modify the requirements of that issue.

cilefen’s picture

@catch I am not necessarily disagreeing with you, but why don't we insist as part of the other issue that we provide maps or a library isn't "done"? If we have two issues like this we will be stepping on our own toes.

cilefen’s picture

Source maps are supported in IE 8.1+, Firefox, and Chrome and (I can vouch for Firefox and Chrome) the debugging tools look for them by default. One way to create them:

uglifyjs foo.js -o foo.min.js --source-map foo.min.js.map

webchick’s picture

The part of catch's point that I think justifies the two issues is even for old libraries that we *don't* end up updating to the latest versions (because there aren't any new versions since we put them into core), they still need to be minified. However, we could try and do this in one go, and close this out quickly, then work on the other one to only handle the libraries that need updates.

That map stuff looks very interesting; had no idea that was possible.

hass’s picture

I do understand that these maps are useful for development, but please do not forget the file size. We may add 500kb to a page! Under normal conditions you do not need the maps at all.

I have opened a case to remove the references from js/css files when aggregated. I still think the files shouldn't be there for uncompresses files. We better only add the map files if devel is installed as one example.

cilefen’s picture

@hass I for sure could be wrong about this, but as far as I know the map files are only asked-for by browser dev consoles, not at all in a normal page load.

hass’s picture

Chrome has this enabled by default... It is downloaded all times. See the network tab, please.

cilefen’s picture

@hass I know what you mean, but I don't know if that means the browser actually downloads the map files when the web console is not running.

cilefen’s picture

@hass

I performed a little experiment to be sure. All the files are here.

Here are logs from my web server in each case.

Without dev tools

::1 - - [10/Apr/2015:22:00:50 -0400] "GET /foo/ HTTP/1.1" 200 283
::1 - - [10/Apr/2015:22:00:51 -0400] "GET /foo/foo.min.js HTTP/1.1" 200 73

With dev tools open

::1 - - [10/Apr/2015:22:01:00 -0400] "GET /foo/ HTTP/1.1" 304 -
::1 - - [10/Apr/2015:22:01:00 -0400] "GET /foo/foo.min.js HTTP/1.1" 304 -
::1 - - [10/Apr/2015:22:01:01 -0400] "GET /foo/foo.min.js.map HTTP/1.1" 200 125

So it looks like I am correct. The full file and the map file are downloaded only when the dev tools are open. Also, this is the point of the map files - you get to use minified files for production but you also have the benefit of non-minified files when your browser's dev console is open.

cilefen’s picture

Map files also work for CSS by the way.

droplet’s picture

@cilefen is CORRECT. I told the same in other threads that requested to remove sourcemap too.

Aki Tendo’s picture

Crell: nod_ The use case for including the unminified versions is debugging and development.

Minified js files can be shipped with a sourcemap, and I **strongly** advise making those maps available if minified.js is in use. The same is true for less/sass files and css.

mfer: 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.

Wrong. Very, very wrong. Step back and think for a moment - the GPL originated with Richard Stallman as a means of sharing programs written in C.

Remember, compiled code is a bit of a bear to reverse engineer back to source, and has a much stronger obfuscating nature than anything that can be done in Java Script. Therefore the requirement of the GPL has always been that you provide the source code as part of your distribution upon request. You aren't required to include it in every distribution - look at any of the major Linux distros - they rarely have the source code in the main packages since doing so trebles or quadruples the download size. But that code is available on request.
You quite obviously aren't required to ask the computer to work from that source code package. If you were then the GPL would prevent distributions of the code to anyone who didn't know how to work a compiler. Java Script and PHP's ability to work directly from the source is hardly the rule among programming languages.

Crell’s picture

Aki: You are correct that, strictly speaking, GPLv2 only requires that the source be available upon request. Shipped in the same code download but not normally sent to the browser is fine. I said the same thing back in #6. :-) The catch is that we *do* need to retain the copyright statement, and can't strip that out as we do other comments.

I'd not heard of sourcemaps before. I'm not sure how new they are. If they work I've no problem with them.

(Disclaimer: I'm a member of the Drupal Licensing Working Group.)

Aki Tendo’s picture

I've not got a dog in the fight so it doesn't really matter to me, but I don't understand a need to draw a distinction between minified and compiled code. The latter can't be read at all, and the former isn't meant to be read so placing a copyright statement in there feels moot or overkill to me. But I'm no lawyer and I've seen sabers rattled over less I guess there's nothing wrong with being over cautious.

stefan.r’s picture

The devel patch in #2471919: Devel only supports uncompression of jQuery and not of any other core JavaScript libraries implements the toggling of uncompressed libraries suggested by @Crell and @catch (#62, #63).

stefan.r’s picture

Actually now that we commit source maps along with external libraries, dealing with this in Devel shouldn't be needed anymore...

nod_’s picture

Over time we got all our libs to used minified versions.

The only remaining lib is #2473837: Use minified jQuery once.

webchick’s picture

Status:Postponed» Fixed

Aaaand that one's now in too. :)

I couldn't get ahold of catch today, but in #66 he pointed out that the reason this one was separate was because we needed to do that for old libraries too, not just ones we're updating. And that is now done for all old libraries. He also didn't seem to disagree with #71 where I stated we could just get this one out of the way and update the issue summary of the other.

So since classmap stuff is covered in #2400287: Remove all occurences of sourceMappingURL and sourceURL when JS files are aggregated, I am going to "be bold" here and mark this fixed!!

Thanks SO much for everyone's hard work on this!!!

Status:Fixed» Closed (fixed)

Automatically closed - issue fixed for 2 weeks with no activity.