Closed freekh closed 5 years ago
I believe we need a running app to get some config. Not sure how else we can do this...
I guess it would be to have an implicit on the defs that needs it? If that doesn't do it, how about making a plugin?
On Jul 19, 2013, at 7:03 PM, James Ward wrote:
I believe we need a running app to get some config. Not sure how else we can do this...
— Reply to this email directly or view it on GitHub.
But somehow the config has to get in there for that stuff to work. So we can move the problem to another place but will it fix it? cc @huntc to see if he has any ideas.
moving it somewhere so that it is an implicit at the user callsite, would provoke a nice compile error instead of the nasty runtime error. I can also have a closer look (if I remember). Found this bug during a training from which I am travelling from. My plane boards now so that is why I can't check it out now :)
I've not absorbed this deeply, but I'm pretty sure you need a running app to obtain config in Play @jroper?
Yep I think you need a running app - just sayin it might be a differentt way to get it then play.api.Current. Don't worry about it though :) I can have a look next week and put the money where my mouth is :) I haven't looked at it in detail yet so I might be wrong...
Something like the PR above would solve the issue. The cons is that the users will have to have an implicit app imported in scope, then again they do not have to be completely confused when it fails on runtime and spend a lot of time on thinking about it (like my client). I prefer a bit of extra code to spending time on debugging runtime issues anytime - WDYT?
Hey @freekh - did you know that it is possible to attach a PR to an existing issue in Github via the "hub" command? :-) I shall look at the PR now and get back to you...
How about lazily deferring the resolution of those properties using a lazy declaration? Those implicit declarations look ugly to me...
Ah, didn't know about hub actually - thanks for the tip. Looks like issue 14 is a PR now... BTW: the reproduction steps goes a bit like this: call one of the methods that uses the Play app in a template, then try to load that template in a test that does not have a running app. It will fail with a stacktrace that is coming from some code completely unrelated to webjars or anything, making it extremely hard to figure out.
I have changed it a bit, so that it uses less implicits parameters. Couldn't figure out how laziness can be used for this. Either way it still needs a Play Application each time the user request it - right?
I think this looks good but it would be nice to see how it impacts usage in one of @huntc's example apps. Also, we should see what the experience will be like for Java developers. Does this impact them at all?
I'm not sure if this is a good way to go about it. It means you need to either import an application into a template, or pass an application into a template, when really I don't think an application has any business being imported or passed into a template. Furthermore, it needs to be the Scala application object, so this will make it harder for Java users to use webjars, unless we provide an implicit conversion to convert a Java application to a Scala application, and I'm not keen on adding yet more magic like this to our templating. Either way, when testing, you'll still need to wrap your tests in Helpers.running(FakeApplication()).
Yes it's bad to rely on global state, but that's the only mechanism we currently provide in Play to access global configuration. In future we will be investigating attaching the application to the request object, and that will give us a better way to pass the configuration and remove global state from Play. But until then I think it's best not to fight Play's global state.
Sure, I think that is a valid argument. I am not going to fight for this, because i agree that easier is better. Nevertheless, the reason why I made this issue was because it was impossible to figure out the issue caused by the import. While it makes things easier it also makes it more complex. Also just to be pedantic on your java argument: this is an object so it will not look good for java users either way...
On Wed, Jul 24, 2013 at 1:43 AM, James Roper notifications@github.com wrote:
I'm not sure if this is a good way to go about it. It means you need to either import an application into a template, or pass an application into a template, when really I don't think an application has any business being imported or passed into a template. Furthermore, it needs to be the Scala application object, so this will make it harder for Java users to use webjars, unless we provide an implicit conversion to convert a Java application to a Scala application, and I'm not keen on adding yet more magic like this to our templating. Either way, when testing, you'll still need to wrap your tests in Helpers.running(FakeApplication()).
Yes it's bad to rely on global state, but that's the only mechanism we currently provide in Play to access global configuration. In future we will be investigating attaching the application to the request object, and that will give us a better way to pass the configuration and remove global state from Play. But until then I think it's best not to fight Play's global state.
Reply to this email directly or view it on GitHub: https://github.com/webjars/webjars-play/pull/14#issuecomment-21454471
So, perhaps there is a way to solve the issue without using implicit. @huntc Perhaps this is the way to lazy val? If it was all lazy vals, then it would perhaps not fail when the object is created but later when it is used. This will perhaps not swallow the error and you would get the error saying you need to import, instead of what my client was getting.
I ran into this issue as well when using a custom ApplicationLoader in play 2.4. When trying to pass an instance of WebJarAssets to my RouterConstructor I was getting RuntimeException: There is no started application
due to the the reference to current
in the initializer code of WebJarAssets.
My solution was to pass (inject) a Configuration
and Environment
instance into the constructor of the case class and have only the companion object depend on current
. This appears to solve the problem in both the case where I have a custom app load and I'm performing manual dependency injection as well as the case where Play is handling instantiation of controllers for me.
I've submitted a pr: #55
this is probably not relevant anymore. Last comment dates from 2015
import play.api.Play.current gives strange failures outside of a running app. Took me a bit of time to figure out during a training that a test that would not run used Webjars inside of a template.
The rule of thumb is to avoid using play.api.Play.current , especially in a static object. The reason is that if it fails it might give (as in my example) a very strange error message.