Reverse Router should use minified assets in production automatically

[huntc]

Serving assets is usually different between development and production mode. In dev mode, you want to serve the assets as they are, and in production they should be minified, compressed, versioned, and so on.

Using minified resources already works with RequireJS assets that go through r.js. Using versioned assets in prod mode works thanks to the Router's versioned method. Now it'd be nice to combine this, so Play serves versioned and minified LESS/CSS assets.

To put it in code:

@if(current.mode == Mode.Dev) {
  <link rel="stylesheet" media="screen" href="@routes.Assets.versioned("lib/bootstrap/css/bootstrap.css")" />
}
@if(current.mode == Mode.Prod) {
  <link rel="stylesheet" media="screen" href="@routes.Assets.versioned("lib/bootstrap/css/bootstrap.min.css")" />
}

should become

<link rel="stylesheet" media="screen" href="@routes.Assets.versionedAndModeAware("lib/bootstrap/css/bootstrap.css")" />

[jroper]

I think we can just have versioned handle this by default.

[gmethvin]

@jroper Do you mean doing the minification via the asset pipeline?

[huntc]

I think that @jroper means that the choice of using a .min or -min extension is one that can be made by the versioned method. The actual minification process is a build concern and not a runtime concern.

[gmethvin]

@huntc Right. I was just wondering as you'd need to make sure a minified version exists before you reference it. Creating the minified version via the asset pipeline would work for all assets consistently, but you could also use the minified versions already provided in some webjars.

[huntc]

Creating the minified version via the asset pipeline would work for all assets consistently, but you could also use the minified versions already provided in some webjars

That's already implemented i.e. if a WebJar contains a minified asset then that becomes available as per any other static asset. WebJars are extracted to disk and referenced via a lib/ folder.

BTW the asset controller already makes a similar sort of check for digest files in the versioned method. This processing is cached. We would do the same sort of thing for determining the correct extension to use.

[julienrf]

Isn’t it possible to have both at and versioned handle the min suffix?

[mariussoutier]

Or even merge at and versioned into one?

[jroper]

The question was whether we should break existing code or not... versioned requires a : Asset type in the routes file. We decided to not break source compatibility here, and introduced a new method.

[huntc]

I've attached a PR to this issue that has the asset controller check for the existence of .min.* and -min.* for all asset requests.

[huntc]

I've now updated this PR so that the checking for minified files is an option that may be disabled; just in case some transitive dependency within a WebJar (for example) appears and breaks things (doubtful). By default the option is enabled.

[mariussoutier]

Thanks Christopher!

[jroper]

I think we should document this in the assets documentation.

[huntc]

Should that preclude the merging of this? Also there are other options undocumented and in reality, I don't think this is going to be an issue.

[jroper]

Well... we've got to start somewhere... :) I'll merge it, but only if you add a comment to this PR with a picture of a kitten in it. I like kittens.

[jroper]

Backported to 2.3.x 1c9a5f2

[huntc]

As you requested:

[jroper]

Awww.... how cute!

[jroper]

I didn't notice this one before. Since it's on the Asset companion object, it never gets used unless you add it to the router imports (the assetPathBindable below does get used, because it provides a path bindable for Asset and is on the Asset companion object, whereas this provides a path bindable for String, and the compiler won't look at the Asset companion object for a path bindable for String). And if you did do that, any reverse route that accepted a String path parameter (eg /user/:username) will use it, and will throw an exception from the code above (with message "Asset path bindable must be used in combination with an action that accepts a path parameter"). I think this should therefore be deleted, I don't think there's any purpose for it.

[huntc]

I'm not sure, but the idea was to provide a string path bindable for situations where the reverse router is required in an assets.at scenario as opposed to assets.versioned. The tests should cover this scenario.

[jroper]

I can't see any tests that cover it - but the fact remains that an implicit PathBindable[String] in scope will apply to all string parameters, there's no way to make it differentiate between assets and non assets parameters, this is why we had to introduce the Asset type in the first place, otherwise we could have written a PathBindable[String] and have Assets.at use that. The first thing it does is call pathFromParams(rrc) which throws an exception if there's no fixed parameter in the route called path.

[jzahka]

@mariussoutier @jroper
I'm having an issue where Play is grabbing the minified asset in Dev mode.

<script src="@routes.Assets.versioned("lib/angularjs/angular.js")"></script>

resolves to (in Dev mode)

<script src="/assets/lib/angularjs/angular.min.js"></script>

I'm definitely in Dev mode, any intuition on what may be happening?
Let me know if you need more details.

[marcospereira]

Hello @jzahka,

The mailing list would be a better place to discuss this and get more information about your application, etc. If we decide that there is a bug, we can open an issue here. :-)

[mkurz]

@jzahka Is right, this is a bug in Play. I can confirm in dev mode also the .min file is served.
Pull request with the fix: #6291
@jzahka Please see #5173 for a workaround.