webjars / webjars-play

MIT License
80 stars 34 forks source link

"RequireJS.setup(...)" doesn't work in play 2.4.0-RC1 #52

Closed djx314 closed 9 years ago

djx314 commented 9 years ago

In play 2.4.0-RC1, reverse routers takes constructor params, so RequireJS.setup("javascripts/lib/archerui/") in template throws an exception like:

play.api.http.HttpErrorHandlerExceptions$$anon$1: Execution exception[[InstantiationException: controllers.ReverseWebJarAssets]]
...
Caused by: java.lang.NoSuchMethodException: controllers.ReverseWebJarAssets.<init>()

I suggest that just get the reverse route like this:

//RequireJS.scala
def nastyReflectedRoute(path: String, routerName: String): Call = {
  val clazz = Class.forName("controllers.routes", true, play.api.Play.current.classloader)
  val field = clazz.getDeclaredField(routerName)
  val routeInstance = field.get(null)
  routeInstance.asInstanceOf[{ def at(path: String): Call }].at(path)
}

and it works well in my project.

jamesward commented 9 years ago

I wonder if for 2.4 we can actually use DI instead of reflection? Seems like the Reverse Router should be able to be injected. /cc @jroper

jroper commented 9 years ago

Reverse routers can be injected, but no bindings are provided for them by default. Regardless, you would still have to use reflection - the reverse router classes simply do not exist at the time that this library is compiled, and therefore, there's nothing that they can be compiled against.

In Play 3, we're getting rid of global state, and that means we're going to have to rethink how assets are served, we're probably going to have to have some assets components, with a route that gets injected, and an injectable reverse router specifically for assets that is not generated but rather provided by Play. That will solve this issue.

Anyway, the suggested fix is what should be done to address this.

jamesward commented 9 years ago

Thanks @jroper. I need to figure out how to fix my tests so that I can actually test this.

jamesward commented 9 years ago

53 has the PR. Only thing I'm not sure about is why the paths are being escaped:

https://github.com/webjars/webjars-play/blob/fix-reflection/src/test/scala/org/webjars/requirejs/RequireJSSpec.scala#L15

Is that a problem?

jroper commented 9 years ago

In your routes file, you have:

GET     /webjars/:file       controllers.WebJarAssets.at(file)

:file only matches a single path segment, so when you put a value that contains slashes in there, they get escaped. If you want to match multiple path segments, you have to use *file.

jamesward commented 9 years ago

Oh man. Right. I'm on vacation so I have a good excuse for this. PR updated.

jamesward commented 9 years ago

I've released 2.4.0-RC2 with this fix.