webjars / webjars-play

MIT License
80 stars 34 forks source link

Play nice with sbt-digest #47

Closed cvrabie closed 9 years ago

cvrabie commented 9 years ago

Currently serving files with the recommended @routes.WebJarAssets.at(WebJarAssets.locate("file.ext")) does not work together with the sbt-digest mechanism. We would need the equivalent of Assets.versioned method in WebJarAssets. I'm not sure if this is the same issues as https://github.com/webjars/webjars-play/issues/44 but I don't seem so.

Since versioned is now in AssetsBuilder I wonder if this is a simple of an issue of adding the below method to WebJarAssets.scala

def versioned(file: String): Action[AnyContent] = {
  assetsBuilder.versioned("/" + WebJarAssetLocator.WEBJARS_PATH_PREFIX, file)
}
jamesward commented 9 years ago

You shouldn't need to use sbt-digest on the WebJar files because they are already versioned.

cvrabie commented 9 years ago

That might be, but they're still being served with Cache-Control:public, max-age=3600 instead of Cache-Control:public, max-age=31536000.

cvrabie commented 9 years ago

I do see the If-None-Match header in the request, and the ETag in the response so the server will answer with 304 right away, but opposed to the versioned assets the client still does an unnecessary roundtrip every hour.

jamesward commented 9 years ago

You can set the Cache-Control via config:

assets.defaultCache="max-age=290304000"

Or add it to the response, like: https://github.com/webjars/webjars/blob/master/app/controllers/Application.scala#L89

You might want to read my blog about Optimizing Static Asset Loading with Play Framework.

cvrabie commented 9 years ago

Good article but it doesn't actually tell me anything new. It's incorrect to set the far-future expiry for ALL assets unless you're absolutely sure they won't change or you know for sure that you only use Assets.versioned for every html in your project. Sometimes that's not the case, for example if some assets are loaded by 3rd party modules/js/untouchable code.

That's why the Play max-age=3600 is a sensible default for Assets.at. On the other hand the Assets.versioned files are served with max-age=3153600 by Play, by default (see aggressiveCacheControl in AssetInfo ). My point was that the webjar assets should behave like the versioned ones, not like the normal assets.

jamesward commented 9 years ago

If you are using sbt-digest for your own assets and WebJars for your library assets then setting the global max-age=3153600 should be fine.

cvrabie commented 9 years ago

I disagree. You cannot make that assumption for everybody, in every situation. Assets loaded through JS is just one example where this might not be true. You can only assume that for the things you control, in this case the webjar assets. I think it would make sense to make the webjar static assets behave like aggressively cached assets by default, rather than ask everybody to up their cache expiry time for everything.

jamesward commented 9 years ago

Feel free to send a PR.