Skip to content

Loading…

Use Rails to Render Default Index Page #8468

Merged
merged 1 commit into from
@schneems
Ruby on Rails member

This is an alternative implementation to #7771 thanks to the advice of @spastorino

Rails is a dynamic framework that serves a static index.html by default. One of my first questions ever on IRC was solved by simply deleting my public/index.html file. This file is a source of confusion when starting as it over-rides any set "root" in the routes yet it itself is not listed in the routes. By making the page dynamic by default we can eliminate this confusion.

This PR moves the static index page to an internal controller/route/view similar to rails/info. When someone starts a rails server, if no root is defined, this route will take over and the "dynamic" index page from rails/welcome_controller will be rendered. These routes are only added in development. If a developer defines a root in their routes, it automatically takes precedence over this route and will be rendered, with no deleting of files required.

In addition to removing this source of confusion for new devs, we can now use Rails view helpers to build and render this page. While not the primary intent, the added value of "dogfooding" should not be under-estimated.

The prior PR #7771 had push-back since it introduced developer facing files. This PR solves all of the same problems, but does not have any new developer facing files (it actually removes one).

cc/ @wsouto, @dickeyxxx, @tyre, @ryanb, @josevalim, @maxim, @subdigital, @steveklabnik

ATP Railties and Actionpack.

@robin850
Ruby on Rails member

Just giving a big :+1: !

@sxua

:+1: * :100:

@steveklabnik
Ruby on Rails member

:shipit: as far as I'm concerned. This is great.

@guilleiguaran
Ruby on Rails member

This is a big improvement.

:+1: on my side

@nthj

:+1: yes!

@mgonto

:+1: finally!

@mgonto mgonto commented on an outdated diff
actionpack/lib/action_dispatch/routing/inspector.rb
@@ -51,7 +51,8 @@ def action
end
def internal?
- path =~ %r{/rails/info.*|^#{Rails.application.config.assets.prefix}}
+ return true if controller =~ %r{^rails/info|^rails/welcome}
@mgonto
mgonto added a note

Why return true if... instead of just controller=~.... OR path=~.... I think that's clearer and no return needed

@steveklabnik Ruby on Rails member

Only bad part is that it makes a damn long line.

@carlosantoniodasilva Ruby on Rails member

How about:

controller =~ %r{^rails/info|^rails/welcome} || 
  path =~ %r{^#{Rails.application.config.assets.prefix}}
@schneems Ruby on Rails member
schneems added a note

I still prefer explicit returns here, it also gives us a true return instead of a truthy return. If you prefer a different style I can change it up.

@spastorino Ruby on Rails member

:+1: to what @carlosantoniodasilva said

@carlosantoniodasilva Ruby on Rails member

Btw, for a true return you could use ===:

%r{^rails/info|^rails/welcome} === controller || 
  %r{^#{Rails.application.config.assets.prefix}} === path

Not that it'd make any big difference, just in case :).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
@agis-

:thumbsup: no doubt

@carlosantoniodasilva carlosantoniodasilva commented on an outdated diff
railties/lib/rails/welcome_controller.rb
@@ -0,0 +1,7 @@
+class Rails::WelcomeController < ActionController::Base
+ self.view_paths = File.join(File.dirname(__FILE__), 'templates')
@carlosantoniodasilva Ruby on Rails member

You could use File.expand_path here instead.

@schneems Ruby on Rails member
schneems added a note

Good idea, I can change the Rails::InfoController as well.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
@carlosantoniodasilva
Ruby on Rails member

Looks good to me as well. Just as reminder, we need to revert the flag to skip the index page.

@gaurish

:+1:

About serving the index page in production,
I think should enable this index page in production if no root route is provided because

  • there is no harm in doing so: yes, might expose information about an application,but if an app doesn't have a root path, it probably means developer is just testing anyway. In that that additional information might be helpful.
  • behavior in production & development environment should consistent:, unless there is a strong reason to do so. I can imagine someone new to rails generating new app, testing it locally & deploying it. only to encounter that dreaded "something went wrong" page & left wondering, why does it work in development & fails in production? The principle of least astonishment (POLA/PLA)
  • Hosting Providers & sysadmin can use this page to quickly troubleshoot errors with production apps. For example, if you are getting "something went wrong" page even without having a root path, means something is wrong with server configuration(missing ruby etc) but getting that default index.html page means everything is okay.

so I think we should display this page in production, if route path is not defined.

@schneems
You might want to edit that index.html & remove the lines instructing to delete index.html as that doesn't makes sense.

@phawk

Totally agree that this would take away a small pain point for intermediate / expert users of rails. I myself would benefit from it.

However, there is one thing we are taking away from users new to the framework, having this index.html file in the public folder teaches new users that static files override rails routes.

We need to decide if this lesson is less valuable than the pain it causes. Personally I think this lesson is more important that the pain it causes me.

@schneems
Ruby on Rails member

@phawk a better way to show that would be to include HTML files in public in the output of 'rake routes'.

@georgeclaghorn

Definitely support dynamically rendering the default index, but I don't agree that it should be rendered in production. The existing /rails/info routes are only visible in development. We also don't render app error details in production.

It's not necessary to have consistent behavior between the development and production environments. They are separate environments for a reason.

@carlosantoniodasilva
Ruby on Rails member

There's no possibility for this being added to environments other than development.

@schneems
Ruby on Rails member

Switched to expand_path and added a changelog.

@zires

:+1: thanks for saving our time

@rafaelfranca
Ruby on Rails member

@schneems would be great a test to ensure that a root route will take precedence.

@rafaelfranca
Ruby on Rails member

@phawk @schneems the best way to teach this is on the guides.

@schneems
Ruby on Rails member

@rafaelfranca added at test for the root route to take precedence in development

@rafaelfranca
Ruby on Rails member

:shipit:

@spastorino
Ruby on Rails member

This seems good to me, as @carlosantoniodasilva noted we will need to remove the flag to skip the index before merging. Thanks for working on this.

@frodsan

Could you update Setting the Application Home Page section? :+1:

@schneems
Ruby on Rails member

Updated the internal? method and modified the docs.

@gaurish

the page still says remove public/index.html. you might want to edit the index page

@schneems
Ruby on Rails member

Thanks @gaurish I totally wasn't accidentally leaving that in there for an easy PR later or anything :8ball:

@schneems
Ruby on Rails member

updated to remove that line

@georgeclaghorn

Maybe instead of removing that line, you could just change it to "Set up a root route to replace this default homepage" or something similar for clarity?

@schneems
Ruby on Rails member

bikeshed, merge the PR and we'll change it later. Also, I would :+1: that change.

@carlosantoniodasilva
Ruby on Rails member

Looks good to me :+1:, thanks @schneems.

@spastorino all yours :)

@carlosantoniodasilva
Ruby on Rails member

Actually, @schneems weren't you going to remove that skip_index_html flag from the generator? You should be able to revert 5c1109a.

@schneems
Ruby on Rails member

@carlosantoniodasilva yes, i can do that, didn't realize you wanted me to manually revert in same PR. I'll work on that.

@carlosantoniodasilva
Ruby on Rails member

@schneems I think we'll get rid of the option anyway, so it's probably better to do it together :)

@schneems
Ruby on Rails member

I updated pulled out the skip-index stuffs

@frodsan frodsan commented on the diff
railties/lib/rails/welcome_controller.rb
@@ -0,0 +1,7 @@
+class Rails::WelcomeController < ActionController::Base
@frodsan
frodsan added a note

Could you # :nodoc: this?

@frodsan
frodsan added a note

Also, please can you nodoc Rails::InfoController? :smile:

@schneems Ruby on Rails member

done

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
@schneems schneems Use Rails to Render Default Index Page
This is an alternative implementation to #7771 thanks to the advice of @spastorino

Rails is a dynamic framework that serves a static index.html by default. One of my first questions ever on IRC was solved by simply deleting my public/index.html file. This file is a source of confusion when starting as it over-rides any set "root" in the routes yet it itself is not listed in the routes. By making the page dynamic by default we can eliminate this confusion.

This PR moves the static index page to an internal controller/route/view similar to `rails/info`. When someone starts a rails server, if no root is defined, this route will take over and the "dynamic" index page from rails/welcome_controller will be rendered. These routes are only added in development. If a developer defines a root in their routes, it automatically takes precedence over this route and will be rendered, with no deleting of files required. 

In addition to removing this source of confusion for new devs, we can now use Rails view helpers to build and render this page. While not the primary intent, the added value of "dogfooding" should not be under-estimated.

The prior PR #7771 had push-back since it introduced developer facing files. This PR solves all of the same problems, but does not have any new developer facing files (it actually removes one). 

cc/ @wsouto, @dickeyxxx, @tyre, @ryanb, @josevalim, @maxim, @subdigital, @steveklabnik

ATP Railties and Actionpack.
baea5d6
@carlosantoniodasilva
Ruby on Rails member

:heart:

@schneems
Ruby on Rails member

anything else?

@carlosantoniodasilva
Ruby on Rails member

I guess not, lets just wait for @spastorino to do a final check. Thanks @schneems.

@spastorino spastorino merged commit 603e7f7 into rails:master
@rbq

Yay, finally. :)

@gregstallings

So we've now replaced having to delete a public/index.html file with having to delete a welcome_controller and a view?

@sxua

@gregstallings WelcomeController is inside of railties, you don't have to delete it, all you need is to point root in routes.rb to your controller. :shipit:

@rafaelfranca
Ruby on Rails member

@gregstallings no. You only need to define a root route and the default index action will be replaced.

@johnrlive

Awesome as a newbie this always bugged me. It's the little things that count!

@ravidsrk

Awesome!

@scalabl3

+2!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Commits on Dec 11, 2012
  1. @schneems

    Use Rails to Render Default Index Page

    schneems committed
    This is an alternative implementation to #7771 thanks to the advice of @spastorino
    
    Rails is a dynamic framework that serves a static index.html by default. One of my first questions ever on IRC was solved by simply deleting my public/index.html file. This file is a source of confusion when starting as it over-rides any set "root" in the routes yet it itself is not listed in the routes. By making the page dynamic by default we can eliminate this confusion.
    
    This PR moves the static index page to an internal controller/route/view similar to `rails/info`. When someone starts a rails server, if no root is defined, this route will take over and the "dynamic" index page from rails/welcome_controller will be rendered. These routes are only added in development. If a developer defines a root in their routes, it automatically takes precedence over this route and will be rendered, with no deleting of files required. 
    
    In addition to removing this source of confusion for new devs, we can now use Rails view helpers to build and render this page. While not the primary intent, the added value of "dogfooding" should not be under-estimated.
    
    The prior PR #7771 had push-back since it introduced developer facing files. This PR solves all of the same problems, but does not have any new developer facing files (it actually removes one). 
    
    cc/ @wsouto, @dickeyxxx, @tyre, @ryanb, @josevalim, @maxim, @subdigital, @steveklabnik
    
    ATP Railties and Actionpack.
View
2 actionpack/lib/action_dispatch/routing/inspector.rb
@@ -51,7 +51,7 @@ def action
end
def internal?
- path =~ %r{/rails/info.*|^#{Rails.application.config.assets.prefix}}
+ controller =~ %r{^rails/info|^rails/welcome} || path =~ %r{^#{Rails.application.config.assets.prefix}}
end
def engine?
View
9 guides/source/getting_started.md
@@ -215,11 +215,7 @@ Open the `app/views/welcome/index.html.erb` file in your text editor and edit it
### Setting the Application Home Page
-Now that we have made the controller and view, we need to tell Rails when we want Hello Rails! to show up. In our case, we want it to show up when we navigate to the root URL of our site, <http://localhost:3000>. At the moment, however, the "Welcome Aboard" smoke test is occupying that spot.
-
-To fix this, delete the `index.html` file located inside the `public` directory of the application.
-
-You need to do this because Rails will serve any static file in the `public` directory that matches a route in preference to any dynamic content you generate from the controllers. The `index.html` file is special: it will be served if a request comes in at the root route, e.g. <http://localhost:3000>. If another request such as <http://localhost:3000/welcome> happened, a static file at `public/welcome.html` would be served first, but only if it existed.
+Now that we have made the controller and view, we need to tell Rails when we want Hello Rails! to show up. In our case, we want it to show up when we navigate to the root URL of our site, <http://localhost:3000>. At the moment, "Welcome Aboard" is occupying that spot.
Next, you have to tell Rails where your actual home page is located.
@@ -233,7 +229,6 @@ Blog::Application.routes.draw do
# first created -> highest priority.
# ...
# You can have the root of your site routed with "root"
- # just remember to delete public/index.html.
# root to: "welcome#index"
```
@@ -558,7 +553,7 @@ parameter, which in our case will be the id of the post. Note that this
time we had to specify the actual mapping, `posts#show` because
otherwise Rails would not know which action to render.
-As we did before, we need to add the `show` action in
+As we did before, we need to add the `show` action in
`app/controllers/posts_controller.rb` and its respective view.
```ruby
View
6 railties/CHANGELOG.md
@@ -1,9 +1,13 @@
## Rails 4.0.0 (unreleased) ##
+* The public/index.html is no longer generated for new projects. Page is replaced by internal welcome_controller inside of railties
+
+ *Richard Schneeman*
+
* Add ENV['RACK_ENV'] support to `rails runner/console/server`.
*kennyj*
-
+
* Add `db` to list of folders included by `rake notes` and `rake notes:custom`. *Antonio Cangiano*
* Engines with a dummy app include the rake tasks of dependencies in the app namespace.
View
3 railties/lib/rails.rb
@@ -21,7 +21,8 @@
module Rails
autoload :Info, 'rails/info'
- autoload :InfoController, 'rails/info_controller'
+ autoload :InfoController, 'rails/info_controller'
+ autoload :WelcomeController, 'rails/welcome_controller'
class << self
attr_accessor :application, :cache, :logger
View
1 railties/lib/rails/application/finisher.rb
@@ -25,6 +25,7 @@ module Finisher
get '/rails/info/properties' => "rails/info#properties"
get '/rails/info/routes' => "rails/info#routes"
get '/rails/info' => "rails/info#index"
+ get '/' => "rails/welcome#index"
end
end
end
View
3 railties/lib/rails/generators/app_base.rb
@@ -52,9 +52,6 @@ def self.add_shared_options_for(name)
class_option :skip_javascript, type: :boolean, aliases: '-J', default: false,
desc: 'Skip JavaScript files'
- class_option :skip_index_html, type: :boolean, aliases: '-I', default: false,
- desc: 'Skip public/index.html and app/assets/images/rails.png files'
-
class_option :dev, type: :boolean, default: false,
desc: "Setup the #{name} with Gemfile pointing to your Rails checkout"
View
5 railties/lib/rails/generators/rails/app/app_generator.rb
@@ -97,11 +97,6 @@ def log
def public_directory
directory "public", "public", recursive: false
- if options[:skip_index_html]
- remove_file "public/index.html"
- remove_file 'app/assets/images/rails.png'
- keep_file 'app/assets/images'
- end
end
def script
View
2 railties/lib/rails/generators/rails/app/templates/config/routes.rb
@@ -2,7 +2,7 @@
# The priority is based upon order of creation: first created -> highest priority.
# See how all your routes lay out with "rake routes".
- # You can have the root of your site routed with "root" just remember to delete public/index.html.
+ # You can have the root of your site routed with "root"
# root to: 'welcome#index'
# Example of regular route:
View
1 ...ors/rails/app/templates/public/index.html → ...ls/templates/rails/welcome/index.html.erb
@@ -223,7 +223,6 @@
</li>
<li>
- <h2>Set up a default route and remove <span class="filename">public/index.html</span></h2>
<p>Routes are set up in <span class="filename">config/routes.rb</span>.</p>
</li>
View
7 railties/lib/rails/welcome_controller.rb
@@ -0,0 +1,7 @@
+class Rails::WelcomeController < ActionController::Base # :nodoc:
@frodsan
frodsan added a note

Could you # :nodoc: this?

@frodsan
frodsan added a note

Also, please can you nodoc Rails::InfoController? :smile:

@schneems Ruby on Rails member

done

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
+ self.view_paths = File.expand_path('../templates', __FILE__)
+ layout nil
+
+ def index
+ end
+end
View
36 railties/test/application/routing_test.rb
@@ -15,6 +15,12 @@ def teardown
teardown_app
end
+ test "rails/welcome in development" do
+ app("development")
+ get "/"
+ assert_equal 200, last_response.status
+ end
+
test "rails/info/routes in development" do
app("development")
get "/rails/info/routes"
@@ -27,6 +33,36 @@ def teardown
assert_equal 200, last_response.status
end
+ test "root takes precedence over internal welcome controller" do
+ app("development")
+
+ get '/'
+ assert_match %r{<h1>Getting started</h1>} , last_response.body
+
+ controller :foo, <<-RUBY
+ class FooController < ApplicationController
+ def index
+ render text: "foo"
+ end
+ end
+ RUBY
+
+ app_file 'config/routes.rb', <<-RUBY
+ AppTemplate::Application.routes.draw do
+ root to: "foo#index"
+ end
+ RUBY
+
+ get '/'
+ assert_equal 'foo', last_response.body
+ end
+
+ test "rails/welcome in production" do
+ app("production")
+ get "/"
+ assert_equal 404, last_response.status
+ end
+
test "rails/info/routes in production" do
app("production")
get "/rails/info/routes"
View
8 railties/test/generators/app_generator_test.rb
@@ -55,7 +55,6 @@ def test_assets
assert_file "app/views/layouts/application.html.erb", /javascript_include_tag\s+"application"/
assert_file "app/assets/stylesheets/application.css"
assert_file "config/application.rb", /config\.assets\.enabled = true/
- assert_file "public/index.html", /url\("assets\/rails.png"\);/
end
def test_invalid_application_name_raises_an_error
@@ -251,13 +250,6 @@ def test_inclusion_of_javascript_runtime
end
end
- def test_generator_if_skip_index_html_is_given
- run_generator [destination_root, '--skip-index-html']
- assert_no_file 'public/index.html'
- assert_no_file 'app/assets/images/rails.png'
- assert_file 'app/assets/images/.keep'
- end
-
def test_creation_of_a_test_directory
run_generator
assert_file 'test'
Something went wrong with that request. Please try again.