Inform Drupal in *.libraries.yml via a new per-JS file "minified" flag whether a JS file has been minified

It seems strange to decide that themes are unable to use minification in their build process. Maybe D8 ships with a great minification system. What if something better becomes available in the next 3-5 years after D8 ships?

If core wants to ship with no unminified JS, that seems to make sense. But it should be possible for themes to add in minified JS. It's possible somebody downloads a library for use in a theme, and only a minified version is available.

There are others who know far more about this than me, but it seems like there could be use cases where it would be preferable to be able to declare that JS has already been minified.

Is this really a problem? What bad would it do to re-minify something already minified?

Most libraries provide unminified js, but in theory is it possible that one wouldn't? Probably.

I think there may be cases where one is linking to an external library hosted on a CDN, but that shouldn't be affected by aggregation I would think.

A workflow that's becoming more common for frontend work is to use Grunt or Gulp to do build work before moving to production. Those commonly have minification as part of the default build.

Setting up D8 in a way that you can't have minified assets in your themes seems pretty pushy on the part of Drupal. Maybe Drupal has an awesome minification system, or you can plug in a minification system that works wonders. That may even be the best way to do things.

But I get leery of anything that limits what front end techniques can be used when who knows what the heck might happen during the lifecycle of Drupal 8, which could be 5-10 years.

I have no problem with that.

If people are really concerned with minifying minified assets, I don't think it's the end of the world to tag an asset as already minified in libraries.yml. But probably would be bad if things broke if it wasn't tagged.

So maybe just minifying everything is the way to go.

We have a lot of minified code in core... Just one example ckeditor. There are more...

The flag needs to be at the file level, not library level and I'd go with a "minified: true/false" on the file properties.

If we look at a higher level that means asset plugins would define and use their own option on files properties to know when to process it (or not). Or at least we should make sure we're thinking it through making a precedent with this.

We can't disallow the use of minified file in core/contrib so 2) is not an option.

From the other issue: double escaping. I had problems years ago with yui compressor but i'm pretty sure it's not a problem anymore. I'd like to avoid having processing already minified files but it's probably not the end of the world.

This is a feature addition at worst, unless we're going to require this is set (which seems unlikely, it should just default one way or the other), so no need to be beta deadline.

Also we require that external libraries used in core are minified, i.e. #1341792: [meta] Ship minified versions of external JavaScript libraries. The first and only time we've had unminified external libraries in core was this release cycle to make development easier, all previous releases used the minified versions throughout. So it's not a policy change to allow it if we already require it in core.

EDIT: Forget it. Answered in the linked case I had not read yet.

How reliable are .min.js file suffixes in 2014?

(…as well as /min/ subdirectories?)

if (strpos($pathname, '.min.') || strpos($pathname, '/min/')) {
  // Already minified; aggregate as-is and move to next.
  $aggregate .= file_get_contents($pathname);
  continue;
}

re: #10: I think we should commit both original sources and minified versions, if both variants are normally part of the upstream dist archive/package. Unless we find a smart logic like the above (which must be reliable), we should think about declaring variants in libraries.yml files.

the problem with optimized is that it doesn't say what "optimized" means. So far optimized === minified but will that stay true? what if we have people putting asm'ed files? If we go with optimized we need to define it and say it's only about minification, in which case, "minified" is a better key to be using.

This looks nice and simple. +1 from me.

Pretty minor doc fix needed. Somebody with more expertise than me might want to review the tests. Rest looks fine. Any thoughts on who may need to review this so we can RTBC?

+++ b/core/lib/Drupal/Core/Asset/LibraryDiscoveryParser.php
@@ -177,6 +177,11 @@ public function buildByExtension($extension) {
+          // Set the 'minified' flag is on JS file assets, default to FALSE.

Change to: Set the 'minified' flag on JS file assets, default to FALSE.

Made the suggested changes.

Code and tests are good to go.

Afaics there is absolutely no documentation anywhere about the format of libraries.yml file. Can someone file an issue to create some documentation? Could go in core.api.php

Also we need an issue summary update to document which solution we've go for and why.

+++ b/core/lib/Drupal/Core/Asset/JsCollectionOptimizer.php
@@ -112,7 +112,13 @@ public function optimize(array $js_assets) {
+                // Optimize this JS file, but only if it's not yet minified.
+                if (isset($js_asset['minified']) && $js_asset['minified']) {
+                  $data .= file_get_contents($js_asset['data']);
+                }

The isset() here is unnecessary since we ensure it is present in LibraryDiscoveryParser

Committed dd4fd1a and pushed to 8.0.x. Thanks!