playframework / playframework

The Community Maintained High Velocity Web Framework For Java and Scala.
http://www.playframework.com
Apache License 2.0
12.56k stars 4.1k forks source link

controllers package should be under play package #6555

Open gmethvin opened 8 years ago

gmethvin commented 8 years ago

I've never liked the idea of having a top-level package called controllers for controllers that are part of the framework like Assets and Default. Since they are Play-provided components, they should be within the play package. I think play.controllers would be reasonable. This also means our built-in controllers can use things that are private[play].

I think the top-level controllers makes the routes less clear when used in conjunction with other user-provided controllers in the controllers package (as opposed to namespacing to an organization like com.typesafe.controllers). It should be obvious which controllers are defined by your code and which are not.

wsargent commented 8 years ago

Agreed. I'd even go further and say Assets and Default should be in a different subproject from play, so they can be excluded as library dependencies for projects that want their own behavior.

gmethvin commented 8 years ago

@wsargent Yeah, that makes sense, though I think we should still include the dependency by default in the SBT plugin.

wsargent commented 8 years ago

Agreed.

gmethvin commented 8 years ago

Marking this for 2.6 since it seems like a simple migration, but we can always push it back to 3.0 if needed.

balagez commented 7 years ago

Having to do this when I need most things from play.api when wiring the application using compile time DI is just lame:

import play.api.{ controllers => _, _ }
import controllers.AssetsComponents
jxtps commented 7 years ago

To expand on @balagez comment as I also ran into this issue: IntelliJ likes to shorten multiple play.api.Xyz imports to play.api._. This then makes controllers.Xyz refer to play.api.controllers.Xyz which doesn't exist.

Currently the only thing in the play.api.controllers package is TrampolineContextProvider.

Since it's typical to import so much from play._ and play.api._ it might make sense to not re-use the exact same name (controllers) as it leads to this type of shadowing.

gmethvin commented 7 years ago

The main issue here is the way the reverse router works now. Let's say you have a routes file in the com.example package. The way it works now, the routes compiler will generate a com.example.play.controllers package if you reference the play.controllers package in your routes file. That will create a conflict anytime you need to reference the play package within the com.example package.

Maybe we should separate the pieces like AssetsComponents and other such classes into a play.assets package and keep the Assets controller in the root controllers package. Or perhaps we need to change the way the routes generator creates classes.

This requires a lot more thought than I thought it did originally, which is why it didn't make it into 2.6. If we can find a nice backwards compatible solution it could make it into 2.6.x sometime but it's not something we're working on right now.