Update #2, Koz et al are on board with making the change assuming the community doesn't come up with any upgrade blockers. Cross your fingers, we might see the change in the near future.
I've always been slightly annoyed that ApplicationController does not follow convention and is defined in application.rb, but I never bothered to do anything about it, until now.
I was recently asked to look over a fairly new codebase that was being actively worked on by someone new to Rails. In one of the controllers I found code similar to the following snippet.
if RAILS_ENV == 'development'
caches_page :index
end
The code functionally works, but I prefer to see environment specific logic in the environment specific files (development.rb, test.rb, and production.rb). I suggested that the developer set the caching logic in the development.rb file. Unfortunately, referencing a controller in development.rb will raise an error because ApplicationController is not yet available. This stems from the fact that
require 'application'
has not yet been executed and due to the naming inconsistency the class can not be auto-loaded. The fix is easy enough, you can require 'application'
before you reference a controller, but I wouldn't expect any Rails novice to know that.Is the inconsistency a blocker for Rails adoption? Absolutely not, but is it necessary? Again, absolutely not.
I wanted to patch* Rails and remove the inconsistency, so I started by creating a sample app to see what the change will break. Using Rails 2.0.2 I created a sample application and froze the 2.0.2 gems. Next I created a simple controller that would allow me to see what breaks when I change application.rb to application_controller.rb.
render :text => "hello world"
end
end
At this point it's time to make the file change and see what breaks. After making the filename change I restart my server (using
script/server
) and to my surprise, nothing breaks. I refresh my browser and everything just works. Well, that was much better than expected, but I must have broken something or I expect this change would have already been made.So after a bit of looking around I found that application.rb was explicitly being referenced in the following places:
- activesupport/lib/active_support/dependencies.rb
- actionpack/lib/action_controller/dispatcher.rb
- railties/lib/console_with_helpers.rb
- railties/lib/test_help.rb
- railties/lib/commands/performance/request.rb
and actionpack/lib/action_controller/dispatcher.rb
The special cases defined in these files are actually bypassed (invisibly to the user) when ApplicationController follows naming conventions.
railties/lib/console_with_helpers.rb
Making the change does break
script/console
. On line 19 'application' is explicitly required and since this file no longer exists an error is raised. The fix here is simple, remove the explicit require and let auto-loading take care of things.railties/lib/test_help.rb
Line 1 explicitly requires application, again, the simple fix is to remove this line completely.
railties/lib/commands/performance/request.rb
Same as above, remove the explicit require on line 3 and everything just works.
Should you follow the instructions above? Maybe. If you never plan on changing your frozen version of rails then it might be a good idea. If you plan on changing your frozen version at some point but you think it's worth doing this work again at that point then it might also make sense. If you plan on changing your frozen version and you are worried that the change might cause issues in the future then you are probably better off living with the inconsistency for now.
I'm going to go ahead and make the change because I don't enjoy remembering the special case every time I want to Command+t and jump to the file.
Like I said before, I did the research so I could contribute back to the community by way of a Rails patch. I've put together a patch and submitted it, and it's gotten plenty of support, but it needs more, much more I suspect since Koz appears to have put it on hold. So, if you would also like to see the inconsistency removed please add a +1 to the ticket. (at http://dev.rubyonrails.org/ticket/10570)
* If you've ever wanted to submit a patch to Rails but not known where to start, I highly suggest starting at http://dev.rubyonrails.com.
No comments:
Post a Comment
Note: Only a member of this blog may post a comment.